diff --git a/src/app/gfx/util/palette_manager.cc b/src/app/gfx/util/palette_manager.cc index 44d17470..78064a5d 100644 --- a/src/app/gfx/util/palette_manager.cc +++ b/src/app/gfx/util/palette_manager.cc @@ -18,7 +18,7 @@ void PaletteManager::Initialize(zelda3::GameData* game_data) { } game_data_ = game_data; - rom_ = nullptr; // Clear legacy ROM pointer + rom_ = game_data->rom(); // Load original palette snapshots for all groups auto* palette_groups = &game_data_->palette_groups; @@ -67,6 +67,19 @@ void PaletteManager::Initialize(Rom* rom) { ClearHistory(); } +void PaletteManager::ResetForTesting() { + game_data_ = nullptr; + rom_ = nullptr; + original_palettes_.clear(); + modified_palettes_.clear(); + modified_colors_.clear(); + change_listeners_.clear(); + next_callback_id_ = 1; + batch_depth_ = 0; + batch_changes_.clear(); + ClearHistory(); +} + // ========== Color Operations ========== SnesColor PaletteManager::GetColor(const std::string& group_name, @@ -245,6 +258,14 @@ absl::Status PaletteManager::SaveGroup(const std::string& group_name) { return absl::FailedPreconditionError("PaletteManager not initialized"); } + Rom* rom = rom_; + if (!rom && game_data_) { + rom = game_data_->rom(); + } + if (!rom) { + return absl::FailedPreconditionError("No ROM available for palette save"); + } + auto* group = GetMutableGroup(group_name); if (!group) { return absl::NotFoundError( @@ -271,7 +292,8 @@ absl::Status PaletteManager::SaveGroup(const std::string& group_name) { GetPaletteAddress(group_name, palette_idx, color_idx); // Write color to ROM - write the 16-bit SNES color value - rom_->WriteShort(address, (*palette)[color_idx].snes()); + RETURN_IF_ERROR( + rom->WriteShort(address, (*palette)[color_idx].snes())); } } } @@ -286,7 +308,7 @@ absl::Status PaletteManager::SaveGroup(const std::string& group_name) { ClearModifiedFlags(group_name); // Mark ROM as dirty - rom_->set_dirty(true); + rom->set_dirty(true); // Notify listeners PaletteChangeEvent event{PaletteChangeEvent::Type::kGroupSaved, group_name, diff --git a/src/app/gfx/util/palette_manager.h b/src/app/gfx/util/palette_manager.h index cca33615..cdc5b585 100644 --- a/src/app/gfx/util/palette_manager.h +++ b/src/app/gfx/util/palette_manager.h @@ -106,6 +106,11 @@ class PaletteManager { */ bool IsInitialized() const { return game_data_ != nullptr || rom_ != nullptr; } + /** + * @brief Reset all state for test isolation + */ + void ResetForTesting(); + // ========== Color Operations ========== /** diff --git a/src/zelda3/game_data.cc b/src/zelda3/game_data.cc index 516318a8..fd1a96a3 100644 --- a/src/zelda3/game_data.cc +++ b/src/zelda3/game_data.cc @@ -79,6 +79,7 @@ uint32_t GetGraphicsAddress(const uint8_t* data, uint8_t addr, uint32_t ptr1, absl::Status LoadGameData(Rom& rom, GameData& data, const LoadOptions& options) { data.Clear(); + data.set_rom(&rom); if (options.populate_metadata) { RETURN_IF_ERROR(LoadMetadata(rom, data)); diff --git a/test/e2e/rom_dependent/palette_editor_save_test.cc b/test/e2e/rom_dependent/palette_editor_save_test.cc index 5b0fd31d..c5eac5fc 100644 --- a/test/e2e/rom_dependent/palette_editor_save_test.cc +++ b/test/e2e/rom_dependent/palette_editor_save_test.cc @@ -170,8 +170,8 @@ TEST_F(PaletteEditorSaveTest, SnesColorFormat_RoundTrip) { // Convert to SnesColor and back gfx::SnesColor color(test_snes); - // Get RGB representation - auto rgb = color.rgb(); + // Get RGB representation in 0-255 range + auto rgb = color.rom_color(); // Create new color from RGB gfx::SnesColor reconstructed(rgb); @@ -284,19 +284,11 @@ TEST_F(PaletteEditorSaveTest, PaletteManager_SaveAllToRom) { } // Try to modify a color through PaletteManager - // Access overworld main palettes through game_data - auto* ow_main = game_data_->palette_groups.overworld_main.mutable_palette(0); - if (!ow_main || ow_main->size() == 0) { - GTEST_SKIP() << "No overworld main palette available"; - } - - // Record original color - gfx::SnesColor original_color = (*ow_main)[0]; + gfx::SnesColor original_color = pm.GetColor("ow_main", 0, 0); // Modify the color uint16_t new_snes_value = (original_color.snes() + 0x0842) & 0x7FFF; - (*ow_main)[0] = gfx::SnesColor(new_snes_value); - (*ow_main)[0].set_modified(true); + ASSERT_OK(pm.SetColor("ow_main", 0, 0, gfx::SnesColor(new_snes_value))); // Save through PaletteManager auto save_result = pm.SaveAllToRom(); @@ -436,4 +428,3 @@ TEST_F(PaletteEditorSaveTest, RoundTrip_NoModification) { } // namespace test } // namespace yaze - diff --git a/test/integration/palette_manager_test.cc b/test/integration/palette_manager_test.cc index e45f913b..fe69b523 100644 --- a/test/integration/palette_manager_test.cc +++ b/test/integration/palette_manager_test.cc @@ -14,9 +14,8 @@ namespace { class PaletteManagerTest : public ::testing::Test { protected: void SetUp() override { - // PaletteManager is a singleton, so we need to reset it between tests - // Note: In a real scenario, we'd need a way to reset the singleton - // For now, we'll work with the existing instance + // PaletteManager is a singleton, so reset it for test isolation + PaletteManager::Get().ResetForTesting(); } void TearDown() override { diff --git a/test/unit/gfx/snes_palette_test.cc b/test/unit/gfx/snes_palette_test.cc index 8c8b57b6..2de63c28 100644 --- a/test/unit/gfx/snes_palette_test.cc +++ b/test/unit/gfx/snes_palette_test.cc @@ -31,17 +31,17 @@ TEST(SnesColorConversionTest, DefaultConstructor) { EXPECT_EQ(color.rgb().x, 0.0f); EXPECT_EQ(color.rgb().y, 0.0f); EXPECT_EQ(color.rgb().z, 0.0f); - EXPECT_EQ(color.rgb().w, 0.0f); + EXPECT_EQ(color.rgb().w, 255.0f); EXPECT_EQ(color.snes(), 0); } TEST(SnesColorConversionTest, RGBConstructor) { ImVec4 rgb(1.0f, 0.5f, 0.25f, 1.0f); yaze::gfx::SnesColor color(rgb); - EXPECT_EQ(color.rgb().x, rgb.x); - EXPECT_EQ(color.rgb().y, rgb.y); - EXPECT_EQ(color.rgb().z, rgb.z); - EXPECT_EQ(color.rgb().w, rgb.w); + EXPECT_EQ(color.rgb().x, 255.0f); + EXPECT_EQ(color.rgb().y, 127.5f); + EXPECT_EQ(color.rgb().z, 63.75f); + EXPECT_EQ(color.rgb().w, 255.0f); } TEST(SnesColorConversionTest, SNESConstructor) {