feat: Finalize Core Architecture, CI Pipeline, and Modernization#102
Merged
Conversation
… missing unit tests
There was a problem hiding this comment.
Pull request overview
This PR expands the emulator’s correctness and API-safety story by adding new GoogleTest coverage for core hardware components (Timer, Interrupt Controller, Cartridge) and by modernizing several public interfaces (e.g., [[nodiscard]] usage and std::string_view parameters in PPU).
Changes:
- Added new GoogleTest suites for Timer, InterruptController, and Cartridge behavior.
- Hardened Cartridge ROM loading by padding undersized ROM inputs to a minimum 32KB.
- Modernized several public APIs with
[[nodiscard]]andstd::string_view(PPU + core headers).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/timer_tests.cpp | Adds unit tests for DIV/TIMA behavior and timer interrupt generation. |
| tests/interrupt_controller_tests.cpp | Adds unit tests for IF/IE behavior, pending interrupts, and IO register semantics. |
| tests/cartridge_tests.cpp | Adds unit tests for ROM loading, undersized ROM padding, and basic ROM reads. |
| src/core/ppu.cpp | Implements std::string_view-based signatures and adapts file/GUI text construction accordingly. |
| src/core/cartridge.cpp | Pads ROMs to 32KB during load to avoid out-of-bounds behavior on malformed inputs. |
| include/timer.hpp | Adds [[nodiscard]] to read/getters to improve API correctness. |
| include/ppu.hpp | Introduces std::string_view APIs and adds [[nodiscard]] to several query methods. |
| include/mmu.hpp | Adds [[nodiscard]] to read/getters for safer callsites. |
| include/interrupt_controller.hpp | Adds [[nodiscard]] to read/getters and pending-interrupt query. |
| include/cartridge.hpp | Adds [[nodiscard]], introduces std::string_view in get_title(), and updates includes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+25
to
+29
| // A hardware compliant Game Boy ROM must at least contain the base 32KB banking area. | ||
| // If a testing array or malformed ROM is passed, we securely zero-pad it to prevent segfaults. | ||
| if (rom.size() < 0x8000) { | ||
| rom.resize(0x8000, 0); | ||
| } |
| u8 get_rom_size_code() const { return header.rom_size; } | ||
| u8 get_ram_size_code() const { return header.ram_size; } | ||
| const std::vector<u8>& get_rom_bytes() const { return rom; } | ||
| [[nodiscard]] std::string_view get_title() const { return title; } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR completes all production-grade emulator roadmap features. It introduces a modernized C++17 subsystem (Timer, MMU, Cartridge), complete GoogleTest unit coverage, native Doxygen+Mermaid documentation rendering, and a robust CI pipeline.