Merge pull request #36 from Im-Rises/develop

Develop

CMakeLists.txt

@@ -4,7 +4,7 @@ set(CMAKE_CXX_STANDARD 23)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 set(CMAKE_CXX_EXTENSIONS OFF)
 
-set(CHIP8TOPIA_VERSION "2.1.2")
+set(CHIP8TOPIA_VERSION "2.1.3")
 
 project("Chip8topia" VERSION ${CHIP8TOPIA_VERSION})
 add_subdirectory(${PROJECT_NAME})

Chip8topia/Chip8Emulator/Chip8CoreBase/Chip8CoreBase.h

@@ -26,9 +26,6 @@ enum class Chip8Frequency : uint32
     Freq24000000Hz = 24000000 // 24MHz for Alien-Inv8sion
 };
 
-// TODO: make clock method private and call it in a non public virtual function ClockCore which will call the clock method in a try catch block for safety.
-//  This way we will remove the dependency to the InputHandler in the Cpu and throw error.
-
 // class CpuBase;
 class Input;
 class Chip8CoreBase

Chip8topia/Chip8Emulator/Chip8CoreBase/Core/CpuBase.h

@@ -5,9 +5,16 @@
 #include <array>
 #include <RandomGenerator/RandomGenerator.h>
 #include <functional>
+#include <fmt/format.h>
 
 #include "../chip8Fonts.h"
 
+#if defined(BUILD_PARAM_SAFE)
+#define TRIGGER_EMULATION_ERROR(...) throw std::runtime_error(fmt::format(__VA_ARGS__))
+#else
+#define TRIGGER_EMULATION_ERROR(...) (void)(__VA_ARGS__)
+#endif
+
 class PpuBase;
 class Input;
 class CpuBase

Chip8topia/Chip8Emulator/Chip8Emulator.cpp

@@ -10,7 +10,7 @@
 #include "ChipCores/SchipCCore/SChipCCore.h"
 #include "ChipCores/XoChipCore/XoChipCore.h"
 
-Chip8Emulator::Chip8Emulator() : m_breakpoints{},
+Chip8Emulator::Chip8Emulator() : m_breakpointsStates{},
 #if defined(BUILD_DEBUG)
                                  // Chip8Emulator::Chip8Emulator() : m_core(std::make_unique<SChipCCore>(DEFAULT_FREQUENCY)),
                                  // Chip8Emulator::Chip8Emulator() : m_core(std::make_unique<SChip11Core>(DEFAULT_FREQUENCY)),
@@ -82,39 +82,53 @@ void Chip8Emulator::loadRom(const std::vector<uint8_t>& romData)
 
 void Chip8Emulator::update(const float deltaTime)
 {
-    if (!m_isRomLoaded)
+    try
     {
-        return;
-    }
+        if (!m_isRomLoaded)
+        {
+            return;
+        }
 
-    if (m_isBreak)
-    {
-        if (m_step)
+        if (m_isBreak)
         {
-            m_step = false;
-            m_core->clock();
+            if (m_step)
+            {
+                m_step = false;
+                m_core->clock();
+            }
         }
-    }
-    else
-    {
-        bool screenUpdated = false;
-        while (!screenUpdated && !m_isBreak && !m_errorTriggered)
+        else
         {
-            screenUpdated = m_core->clock();
-
-            if (m_breakpoints[m_core->getCpu()->getPc()])
+            bool screenUpdated = false;
+            while (!screenUpdated && !m_isBreak && !m_errorTriggered)
             {
-                m_isBreak = true;
-                ImGui::InsertNotification({ ImGuiToastType::Info, TOAST_DURATION_INFO,
-                    "Breakpoint hit", fmt::format("Breakpoint hit at 0x{:04X}", m_core->getCpu()->getPc()).c_str() });
+                screenUpdated = m_core->clock();
+
+                if (m_breakpointsStates[m_core->getCpu()->getPc()])
+                {
+                    m_isBreak = true;
+                    ImGui::InsertNotification({ ImGuiToastType::Info, TOAST_DURATION_INFO,
+                        "Breakpoint hit", fmt::format("Breakpoint hit at 0x{:04X}", m_core->getCpu()->getPc()).c_str() });
+                }
             }
         }
     }
+    catch (const std::exception& e)
+    {
+        TRIGGER_ERROR(true, "Emulation error: {}", e.what());
+    }
+    catch (const char* const e)
+    {
+        TRIGGER_ERROR(true, "Emulation error: {}", e);
+    }
+    catch (...)
+    {
+        TRIGGER_ERROR(true, "Emulation error: An unknown error occurred during emulation.");
+    }
 }
 
 void Chip8Emulator::emitSound()
 {
-    // TODO: Improve this condition...
     if (!m_isBreak && m_isRomLoaded && !m_errorTriggered)
     {
         m_soundEmulation.update(m_core);
@@ -196,7 +210,7 @@ void Chip8Emulator::switchCoreFrequency(const Chip8CoreType coreType, const Chip
 
 void Chip8Emulator::clearBreakpoints()
 {
-    m_breakpoints.fill(false);
+    m_breakpointsStates.fill(false);
     m_breakpointsList.clear();
 }
 
@@ -272,9 +286,9 @@ auto Chip8Emulator::getCanBreak() -> bool*
     return &m_canBreak;
 }
 
-auto Chip8Emulator::getBreakpoints() -> std::array<bool, CpuBase::MEMORY_SIZE>&
+auto Chip8Emulator::getBreakpointsStates() -> std::array<bool, CpuBase::MEMORY_SIZE>&
 {
-    return m_breakpoints;
+    return m_breakpointsStates;
 }
 
 auto Chip8Emulator::getBreakpointsList() -> std::set<uint16>&

Chip8topia/Chip8Emulator/Chip8Emulator.h

@@ -7,6 +7,24 @@
 #include "Chip8Emulator/Chip8VideoEmulation.h"
 #include "Chip8Emulator/Chip8SoundEmulation.h"
 
+#if defined(BUILD_PARAM_SAFE)
+#define TRIGGER_ERROR(condition, ...)                                                                            \
+    do                                                                                                           \
+    {                                                                                                            \
+        static_assert(std::is_same<decltype(condition), bool>::value, "Condition must be a boolean expression"); \
+        if ((condition))                                                                                         \
+        {                                                                                                        \
+            Chip8topiaInputHandler::getInstance().m_EmulationError.trigger(fmt::format(__VA_ARGS__));            \
+        }                                                                                                        \
+    } while (false)
+#else
+#define TRIGGER_ERROR(condition, message, ...) \
+    do                                         \
+    {                                          \
+        (void)(message);                       \
+    } while (false)
+#endif
+
 class Chip8Emulator
 {
 public:
@@ -65,7 +83,7 @@ class Chip8Emulator
     [[nodiscard]] auto getIsBreak() const -> bool;
     [[nodiscard]] auto getIsRomLoaded() const -> bool;
     [[nodiscard]] auto getCanBreak() -> bool*;
-    [[nodiscard]] auto getBreakpoints() -> std::array<bool, CpuBase::MEMORY_SIZE>&;
+    [[nodiscard]] auto getBreakpointsStates() -> std::array<bool, CpuBase::MEMORY_SIZE>&;
     [[nodiscard]] auto getBreakpointsList() -> std::set<uint16>&;
     [[nodiscard]] auto getCoreType() const -> Chip8CoreType;
     [[nodiscard]] auto getFrequency() const -> Chip8Frequency;
@@ -84,5 +102,5 @@ class Chip8Emulator
     bool m_errorTriggered;
 
     std::set<uint16> m_breakpointsList;
-    std::array<bool, CpuBase::MEMORY_SIZE> m_breakpoints;
+    std::array<bool, CpuBase::MEMORY_SIZE> m_breakpointsStates;
 };

Chip8topia/Chip8Emulator/Chip8Emulator/Chip8SoundEmulation.cpp

@@ -1,53 +1,51 @@
 #include "Chip8SoundEmulation.h"
 
 #include <SDL.h>
-#include <iostream>
-
 #include "../Chip8CoreBase/Chip8CoreBase.h"
 
 // Thanks to https://gist.github.com/jacobsebek/10867cb10cdfccf1d6cfdd24fa23ee96 !!!
 
-Sint16 format(double sample, double amplitude)
+auto format(double sample, double amplitude) -> uint8
 {
-    // 32567 is the maximum value of a 16 bit signed integer (2^15-1)
-    return static_cast<Sint16>(sample * 32567 * amplitude);
+    return static_cast<uint8>(sample * Chip8SoundEmulation::MAX_AMPLITUDE * amplitude);
 }
 
 // Generate a sine wave
-double tone(double hz, unsigned long time)
+auto tone(double hz, unsigned long time) -> double
 {
-    return sin(time * hz * M_PI * 2 / Chip8SoundEmulation::FREQUENCY_RESOLUTION);
+    return sin(time * hz * M_PI * 2 / Chip8SoundEmulation::FREQUENCY_RESOLUTION) * 0.5 + 0.5;
 }
 
 // Generate a sawtooth wave
-double saw(double hz, unsigned long time)
+auto saw(double hz, unsigned long time) -> double
 {
-    return fmod(time * hz / Chip8SoundEmulation::FREQUENCY_RESOLUTION, 1) * 2 - 1;
+    return 2 * (time * hz / Chip8SoundEmulation::FREQUENCY_RESOLUTION - floor(0.5 + time * hz / Chip8SoundEmulation::FREQUENCY_RESOLUTION));
 }
 
 // Generate a square wave
-double square(double hz, unsigned long time)
+auto square(double hz, unsigned long time) -> double
 {
     double sine = tone(hz, time);
-    return sine > 0.0 ? 1.0 : -1.0;
+    return sine > 0.5 ? 1.0 : 0.0;
 }
 
 Chip8SoundEmulation::Chip8SoundEmulation() : m_buffer{},
                                              m_spec{
                                                  .freq = FREQUENCY_RESOLUTION,
-                                                 .format = AUDIO_S16SYS,
+                                                 .format = AUDIO_U8,
                                                  .channels = 1,
-                                                 .samples = 4096,
+                                                 .samples = SAMPLE_SIZE,
                                                  .callback = Chip8SoundEmulation::soundPlayerCallback,
                                                  .userdata = this,
                                              },
                                              m_bufferPosition(0),
-                                             m_volume(0.5),
+                                             m_volume(DEFAULT_WAVE_AMPLITUDE),
                                              m_isPlaying(false),
-                                             m_squareSoundFrequency(440),
+                                             m_squareSoundFrequency(DEFAULT_WAVE_FREQUENCY),
                                              m_waveType(WaveType::Square)
 {
     m_dev = SDL_OpenAudioDevice(nullptr, 0, &m_spec, nullptr, 0);
+
     initSoundBuffer(square);
 }
 
@@ -61,19 +59,8 @@ void Chip8SoundEmulation::reset()
     stop();
 }
 
-void Chip8SoundEmulation::initSoundBuffer(std::function<double(double, unsigned long)> waveFunction)
-{
-    stop();
-
-    for (int i = 0; i < BUFFER_LEN; i++)
-    {
-        m_buffer[i] = format(waveFunction(m_squareSoundFrequency, i), m_volume);
-    }
-}
-
 void Chip8SoundEmulation::setWaveType(WaveType waveType)
 {
-    // TODO: Correct this code which may lead to crash in webassembly
     stop();
 
     m_waveType = waveType;
@@ -94,7 +81,6 @@ void Chip8SoundEmulation::setWaveType(WaveType waveType)
 
 void Chip8SoundEmulation::update(const std::unique_ptr<Chip8CoreBase>& chip8Core)
 {
-#if !defined(__EMSCRIPTEN__)
     const auto st = chip8Core->getCpu()->getST();
 
     if (st > 0)
@@ -105,7 +91,16 @@ void Chip8SoundEmulation::update(const std::unique_ptr<Chip8CoreBase>& chip8Core
     {
         stop();
     }
-#endif
+}
+
+void Chip8SoundEmulation::initSoundBuffer(std::function<double(double, unsigned long)> waveFunction)
+{
+    stop();
+
+    for (int i = 0; i < BUFFER_LEN; i++)
+    {
+        m_buffer[i] = format(waveFunction(m_squareSoundFrequency, i), m_volume);
+    }
 }
 
 void Chip8SoundEmulation::stop()
@@ -130,25 +125,16 @@ void Chip8SoundEmulation::soundPlayerCallback(void* userdata, unsigned char* str
 {
     auto* chip8SoundEmulation = static_cast<Chip8SoundEmulation*>(userdata);
 
-    chip8SoundEmulation->soundPlayer(stream, streamLength);
-}
-
-void Chip8SoundEmulation::soundPlayer(unsigned char* stream, int streamLength)
-{
-    static constexpr size_t SAMPLE_SIZE = sizeof(Sint16);
-    const int sampleLength = streamLength / SAMPLE_SIZE;
-
-    SDL_memset(stream, m_spec.silence, streamLength);
-
-    if (m_bufferPosition + sampleLength >= BUFFER_LEN)
+    if (chip8SoundEmulation->m_bufferPosition + streamLength >= BUFFER_LEN)
     {
-        m_bufferPosition = 0;
+        chip8SoundEmulation->m_bufferPosition = 0;
     }
 
-    // TODO: Correct the strange crash here in webassembly
-    SDL_memcpy(stream, &m_buffer[m_bufferPosition], streamLength);
-
-    m_bufferPosition += sampleLength;
+    for (int i = 0; i < streamLength; i++)
+    {
+        stream[i] = chip8SoundEmulation->m_buffer[chip8SoundEmulation->m_bufferPosition];
+        chip8SoundEmulation->m_bufferPosition++;
+    }
 }
 
 auto Chip8SoundEmulation::getIsPlaying() const -> bool
@@ -170,3 +156,8 @@ auto Chip8SoundEmulation::getWaveType() const -> WaveType
 {
     return m_waveType;
 }
+
+auto Chip8SoundEmulation::getBuffer() -> std::array<uint8, BUFFER_LEN>&
+{
+    return m_buffer;
+}

Chip8topia/Chip8Emulator/Chip8Emulator/Chip8SoundEmulation.h

@@ -6,9 +6,6 @@
 #include <binaryLib/binaryLib.h>
 #include <SDL.h>
 
-// TODO: Need format to move code of SDL that shouldn't be here elsewhere...
-// TODO: Check if a concurrency issue is present
-
 enum class WaveType : uint8
 {
     Sine,
@@ -21,10 +18,12 @@ class Chip8SoundEmulation
 {
 public:
     static constexpr int FREQUENCY_RESOLUTION = 48000;
-
-private:
+    static constexpr int SAMPLE_SIZE = 4096;
     static constexpr int BUFFER_DURATION = 1;
     static constexpr int BUFFER_LEN = BUFFER_DURATION * FREQUENCY_RESOLUTION;
+    static constexpr int MAX_AMPLITUDE = 255;
+    static constexpr int DEFAULT_WAVE_FREQUENCY = 440;
+    static constexpr double DEFAULT_WAVE_AMPLITUDE = 0.5;
 
 public:
     Chip8SoundEmulation();
@@ -36,27 +35,28 @@ class Chip8SoundEmulation
 
 public:
     void reset();
-    void initSoundBuffer(std::function<double(double, unsigned long)> waveFunction);
     void setWaveType(WaveType waveType);
     void update(const std::unique_ptr<Chip8CoreBase>& chip8Core);
 
 private:
+    void initSoundBuffer(std::function<double(double, unsigned long)> waveFunction);
+
     void stop();
     void play();
 
     static void soundPlayerCallback(void* userdata, unsigned char* stream, int streamLength);
-    void soundPlayer(unsigned char* stream, int streamLength);
 
 public:
     [[nodiscard]] auto getIsPlaying() const -> bool;
     [[nodiscard]] auto getFrequencyPtr() -> int*;
     [[nodiscard]] auto getVolumePtr() -> float*;
     [[nodiscard]] auto getWaveType() const -> WaveType;
+    [[nodiscard]] auto getBuffer() -> std::array<uint8, BUFFER_LEN>&;
 
 private:
     SDL_AudioSpec m_spec;
     SDL_AudioDeviceID m_dev;
-    std::array<Sint16, BUFFER_LEN> m_buffer;
+    std::array<uint8, BUFFER_LEN> m_buffer;
     int m_bufferPosition;
     int m_squareSoundFrequency;
     float m_volume;