From 1e39df88a3fa57f2c93a648054bfcd595128d4b6 Mon Sep 17 00:00:00 2001 From: scawful Date: Sat, 18 Oct 2025 00:09:09 -0400 Subject: [PATCH] refactor: enhance overworld entity properties and version handling - Updated `UpdateMapProperties` methods across various entities (entrances, exits, items, sprites) to include an optional context parameter for improved area size detection. - Introduced `OverworldVersionHelper` for centralized ROM version detection and feature gating, replacing scattered inline checks. - Refactored coordinate calculations to utilize normalized map IDs, ensuring consistency and preventing data corruption. - Enhanced exit properties to sync player positions and calculate scroll/camera values based on the overworld context. Benefits: - Streamlines entity property updates and improves compatibility with different ROM versions. - Reduces code duplication and enhances maintainability by centralizing version checks. - Ensures accurate coordinate calculations for overworld entities, improving overall functionality. --- src/app/editor/overworld/entity.cc | 41 +++- src/app/editor/overworld/entity_operations.cc | 56 ++--- src/app/editor/overworld/map_properties.cc | 76 +++---- src/app/editor/overworld/overworld_editor.cc | 33 +-- .../overworld/overworld_entity_renderer.cc | 10 +- src/cli/handlers/game/overworld_inspect.cc | 4 +- src/zelda3/common.h | 37 ++- src/zelda3/overworld/overworld_entrance.h | 37 +-- src/zelda3/overworld/overworld_exit.cc | 138 ++++++++++- src/zelda3/overworld/overworld_exit.h | 214 +++++++----------- src/zelda3/overworld/overworld_item.h | 22 +- src/zelda3/overworld/overworld_map.h | 8 +- .../overworld/overworld_version_helper.h | 147 ++++++++++++ src/zelda3/sprite/sprite.cc | 3 +- src/zelda3/sprite/sprite.h | 2 +- 15 files changed, 546 insertions(+), 282 deletions(-) create mode 100644 src/zelda3/overworld/overworld_version_helper.h diff --git a/src/app/editor/overworld/entity.cc b/src/app/editor/overworld/entity.cc index ebe4f6b2..5a07677c 100644 --- a/src/app/editor/overworld/entity.cc +++ b/src/app/editor/overworld/entity.cc @@ -3,7 +3,10 @@ #include "app/gui/core/icons.h" #include "app/gui/core/input.h" #include "app/gui/core/style.h" +#include "imgui.h" #include "util/hex.h" +#include "zelda3/common.h" +#include "zelda3/overworld/overworld_item.h" namespace yaze { namespace editor { @@ -26,11 +29,8 @@ bool IsMouseHoveringOverEntity(const zelda3::GameEntity &entity, const ImVec2 mouse_pos(io.MousePos.x - origin.x, io.MousePos.y - origin.y); // Check if the mouse is hovering over the entity - if (mouse_pos.x >= entity.x_ && mouse_pos.x <= entity.x_ + 16 && - mouse_pos.y >= entity.y_ && mouse_pos.y <= entity.y_ + 16) { - return true; - } - return false; + return mouse_pos.x >= entity.x_ && mouse_pos.x <= entity.x_ + 16 && + mouse_pos.y >= entity.y_ && mouse_pos.y <= entity.y_ + 16; } void MoveEntityOnGrid(zelda3::GameEntity *entity, ImVec2 canvas_p0, @@ -154,6 +154,7 @@ bool DrawExitEditorPopup(zelda3::OverworldExit &exit) { static bool set_done = false; if (set_done) { set_done = false; + return true; } if (ImGui::BeginPopupModal("Exit editor", NULL, ImGuiWindowFlags_AlwaysAutoResize)) { @@ -207,7 +208,9 @@ bool DrawExitEditorPopup(zelda3::OverworldExit &exit) { if (show_properties) { Text("Deleted? %s", exit.deleted_ ? "true" : "false"); Text("Hole? %s", exit.is_hole_ ? "true" : "false"); - Text("Large Map? %s", exit.large_map_ ? "true" : "false"); + Text("Auto-calc scroll/camera? %s", exit.is_automatic_ ? "true" : "false"); + Text("Map ID: 0x%02X", exit.map_id_); + Text("Game coords: (%d, %d)", exit.game_x_, exit.game_y_); } gui::TextWithSeparators("Unimplemented below"); @@ -260,19 +263,21 @@ bool DrawExitEditorPopup(zelda3::OverworldExit &exit) { } if (Button(ICON_MD_DONE)) { + set_done = true; // FIX: Save changes when Done is clicked ImGui::CloseCurrentPopup(); } SameLine(); if (Button(ICON_MD_CANCEL)) { - set_done = true; + // FIX: Discard changes - don't set set_done ImGui::CloseCurrentPopup(); } SameLine(); if (Button(ICON_MD_DELETE)) { exit.deleted_ = true; + set_done = true; // FIX: Save deletion when Delete is clicked ImGui::CloseCurrentPopup(); } @@ -316,6 +321,7 @@ bool DrawItemEditorPopup(zelda3::OverworldItem &item) { static bool set_done = false; if (set_done) { set_done = false; + return true; } if (ImGui::BeginPopupModal("Item editor", NULL, ImGuiWindowFlags_AlwaysAutoResize)) { @@ -325,20 +331,26 @@ bool DrawItemEditorPopup(zelda3::OverworldItem &item) { for (size_t i = 0; i < zelda3::kSecretItemNames.size(); i++) { if (Selectable(zelda3::kSecretItemNames[i].c_str(), item.id_ == i)) { item.id_ = i; + item.entity_id_ = i; + item.UpdateMapProperties(item.map_id_, nullptr); } } ImGui::EndGroup(); EndChild(); - if (Button(ICON_MD_DONE)) ImGui::CloseCurrentPopup(); + if (Button(ICON_MD_DONE)) { + set_done = true; // FIX: Save changes when Done is clicked + ImGui::CloseCurrentPopup(); + } SameLine(); if (Button(ICON_MD_CLOSE)) { - set_done = true; + // FIX: Discard changes - don't set set_done ImGui::CloseCurrentPopup(); } SameLine(); if (Button(ICON_MD_DELETE)) { item.deleted = true; + set_done = true; // FIX: Save deletion when Delete is clicked ImGui::CloseCurrentPopup(); } @@ -437,6 +449,7 @@ bool DrawSpriteEditorPopup(zelda3::Sprite &sprite) { static bool set_done = false; if (set_done) { set_done = false; + return true; } if (ImGui::BeginPopupModal("Sprite editor", NULL, ImGuiWindowFlags_AlwaysAutoResize)) { @@ -447,20 +460,24 @@ bool DrawSpriteEditorPopup(zelda3::Sprite &sprite) { DrawSpriteTable([&sprite](int selected_id) { sprite.set_id(selected_id); - sprite.UpdateMapProperties(sprite.map_id()); + sprite.UpdateMapProperties(sprite.map_id(), nullptr); }); ImGui::EndGroup(); EndChild(); - if (Button(ICON_MD_DONE)) ImGui::CloseCurrentPopup(); + if (Button(ICON_MD_DONE)) { + set_done = true; // FIX: Save changes when Done is clicked + ImGui::CloseCurrentPopup(); + } SameLine(); if (Button(ICON_MD_CLOSE)) { - set_done = true; + // FIX: Discard changes - don't set set_done ImGui::CloseCurrentPopup(); } SameLine(); if (Button(ICON_MD_DELETE)) { sprite.set_deleted(true); + set_done = true; // FIX: Save deletion when Delete is clicked ImGui::CloseCurrentPopup(); } diff --git a/src/app/editor/overworld/entity_operations.cc b/src/app/editor/overworld/entity_operations.cc index c7dc2cc1..8d289ce6 100644 --- a/src/app/editor/overworld/entity_operations.cc +++ b/src/app/editor/overworld/entity_operations.cc @@ -34,33 +34,33 @@ absl::StatusOr InsertEntrance( holes[i].entrance_id_ = 0; // Default, user configures in popup holes[i].is_hole_ = true; - // Update map properties (ZScream: EntranceMode.cs:90) - holes[i].UpdateMapProperties(map_id); - - LOG_DEBUG("EntityOps", "Inserted hole at slot %zu: pos=(%d,%d) map=0x%02X", - i, holes[i].x_, holes[i].y_, map_id); - - return &holes[i]; - } + // Update map properties (ZScream: EntranceMode.cs:90) + holes[i].UpdateMapProperties(map_id, overworld); + + LOG_DEBUG("EntityOps", "Inserted hole at slot %zu: pos=(%d,%d) map=0x%02X", + i, holes[i].x_, holes[i].y_, map_id); + + return &holes[i]; } - return absl::ResourceExhaustedError( - "No space available for new hole. Delete one first."); - - } else { - // Search for first deleted entrance slot (ZScream: EntranceMode.cs:104-130) - auto* entrances = overworld->mutable_entrances(); - for (size_t i = 0; i < entrances->size(); ++i) { - if (entrances->at(i).deleted) { - // Reuse deleted slot - entrances->at(i).deleted = false; - entrances->at(i).map_id_ = map_id; - entrances->at(i).x_ = static_cast(snapped_pos.x); - entrances->at(i).y_ = static_cast(snapped_pos.y); - entrances->at(i).entrance_id_ = 0; // Default, user configures in popup - entrances->at(i).is_hole_ = false; - - // Update map properties (ZScream: EntranceMode.cs:120) - entrances->at(i).UpdateMapProperties(map_id); + } + return absl::ResourceExhaustedError( + "No space available for new hole. Delete one first."); + +} else { + // Search for first deleted entrance slot (ZScream: EntranceMode.cs:104-130) + auto* entrances = overworld->mutable_entrances(); + for (size_t i = 0; i < entrances->size(); ++i) { + if (entrances->at(i).deleted) { + // Reuse deleted slot + entrances->at(i).deleted = false; + entrances->at(i).map_id_ = map_id; + entrances->at(i).x_ = static_cast(snapped_pos.x); + entrances->at(i).y_ = static_cast(snapped_pos.y); + entrances->at(i).entrance_id_ = 0; // Default, user configures in popup + entrances->at(i).is_hole_ = false; + + // Update map properties (ZScream: EntranceMode.cs:120) + entrances->at(i).UpdateMapProperties(map_id, overworld); LOG_DEBUG("EntityOps", "Inserted entrance at slot %zu: pos=(%d,%d) map=0x%02X", i, entrances->at(i).x_, entrances->at(i).y_, map_id); @@ -111,8 +111,8 @@ absl::StatusOr InsertExit( exits[i].door_type_1_ = 0; exits[i].door_type_2_ = 0; - // Update map properties - exits[i].UpdateMapProperties(map_id); + // Update map properties with overworld context for area size detection + exits[i].UpdateMapProperties(map_id, overworld); LOG_DEBUG("EntityOps", "Inserted exit at slot %zu: pos=(%d,%d) map=0x%02X", i, exits[i].x_, exits[i].y_, map_id); diff --git a/src/app/editor/overworld/map_properties.cc b/src/app/editor/overworld/map_properties.cc index 57c67246..627de14b 100644 --- a/src/app/editor/overworld/map_properties.cc +++ b/src/app/editor/overworld/map_properties.cc @@ -9,6 +9,7 @@ #include "app/gui/core/input.h" #include "app/gui/core/layout_helpers.h" #include "zelda3/overworld/overworld_map.h" +#include "zelda3/overworld/overworld_version_helper.h" #include "imgui/imgui.h" namespace yaze { @@ -60,15 +61,15 @@ void MapPropertiesSystem::DrawSimplifiedMapSettings( ImGui::Text("%d (0x%02X)", current_map, current_map); TableNextColumn(); - // IMPORTANT: Don't cache - read fresh to reflect ROM upgrades - uint8_t asm_version = (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; + // Use centralized version detection + auto rom_version = zelda3::OverworldVersionHelper::GetVersion(*rom_); // ALL ROMs support Small/Large. Only v3+ supports Wide/Tall. int current_area_size = static_cast(overworld_->overworld_map(current_map)->area_size()); ImGui::SetNextItemWidth(kComboAreaSizeWidth); - if (asm_version >= 3 && asm_version != 0xFF) { + if (zelda3::OverworldVersionHelper::SupportsAreaEnum(rom_version)) { // v3+ ROM: Show all 4 area size options if (ImGui::Combo("##AreaSize", ¤t_area_size, kAreaSizeNames, 4)) { auto status = overworld_->ConfigureMultiAreaMap( @@ -95,7 +96,8 @@ void MapPropertiesSystem::DrawSimplifiedMapSettings( } } - if (asm_version == 0xFF || asm_version < 3) { + if (rom_version == zelda3::OverworldVersion::kVanilla || + !zelda3::OverworldVersionHelper::SupportsAreaEnum(rom_version)) { HOVER_HINT("Small (1x1) and Large (2x2) maps. Wide/Tall require v3+"); } } @@ -223,9 +225,9 @@ void MapPropertiesSystem::DrawMapPropertiesPanel( } // Custom Overworld Features Tab - uint8_t asm_version = - (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; - if (asm_version != 0xFF && ImGui::BeginTabItem("Custom Features")) { + auto rom_version = zelda3::OverworldVersionHelper::GetVersion(*rom_); + if (rom_version != zelda3::OverworldVersion::kVanilla && + ImGui::BeginTabItem("Custom Features")) { DrawCustomFeaturesTab(current_map); ImGui::EndTabItem(); } @@ -254,9 +256,8 @@ void MapPropertiesSystem::DrawCustomBackgroundColorEditor( return; } - uint8_t asm_version = - (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; - if (asm_version < 2) { + auto rom_version = zelda3::OverworldVersionHelper::GetVersion(*rom_); + if (!zelda3::OverworldVersionHelper::SupportsCustomBGColors(rom_version)) { Text("Custom background colors require ZSCustomOverworld v2+"); return; } @@ -309,15 +310,14 @@ void MapPropertiesSystem::DrawOverlayEditor(int current_map, return; } - uint8_t asm_version = - (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; + auto rom_version = zelda3::OverworldVersionHelper::GetVersion(*rom_); ImGui::TextColored(ImVec4(0.4f, 0.8f, 1.0f, 1.0f), ICON_MD_LAYERS " Visual Effects Configuration"); ImGui::Text("Map: 0x%02X", current_map); Separator(); - if (asm_version < 1) { + if (rom_version == zelda3::OverworldVersion::kVanilla) { ImGui::TextColored(ImVec4(1.0f, 0.6f, 0.4f, 1.0f), ICON_MD_WARNING " Subscreen overlays require ZSCustomOverworld v1+"); ImGui::Separator(); @@ -491,9 +491,8 @@ void MapPropertiesSystem::SetupCanvasContextMenu( canvas.AddContextMenuItem(properties_item); // Custom overworld features (only show if v3+) - uint8_t asm_version = - (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; - if (asm_version >= 3 && asm_version != 0xFF) { + auto rom_version = zelda3::OverworldVersionHelper::GetVersion(*rom_); + if (zelda3::OverworldVersionHelper::SupportsAreaEnum(rom_version)) { // Custom Background Color gui::CanvasMenuItem bg_color_item; bg_color_item.label = ICON_MD_FORMAT_COLOR_FILL " Custom Background Color"; @@ -589,9 +588,8 @@ void MapPropertiesSystem::DrawGraphicsPopup(int current_map, int game_state) { } HOVER_HINT("Sprite graphics sheet for current game state"); - uint8_t asm_version = - (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; - if (asm_version >= 3) { + auto rom_version_gfx = zelda3::OverworldVersionHelper::GetVersion(*rom_); + if (zelda3::OverworldVersionHelper::SupportsAnimatedGFX(rom_version_gfx)) { if (gui::InputHexByte(ICON_MD_ANIMATION " Animated GFX", overworld_->mutable_overworld_map(current_map) ->mutable_animated_gfx(), @@ -605,7 +603,7 @@ void MapPropertiesSystem::DrawGraphicsPopup(int current_map, int game_state) { } // Custom Tile Graphics - Only available for v1+ ROMs - if (asm_version >= 1 && asm_version != 0xFF) { + if (zelda3::OverworldVersionHelper::SupportsExpandedSpace(rom_version_gfx)) { ImGui::Separator(); ImGui::Text(ICON_MD_GRID_VIEW " Custom Tile Graphics"); ImGui::Separator(); @@ -631,7 +629,7 @@ void MapPropertiesSystem::DrawGraphicsPopup(int current_map, int game_state) { } ImGui::EndTable(); } - } else if (asm_version == 0xFF) { + } else if (rom_version_gfx == zelda3::OverworldVersion::kVanilla) { ImGui::Separator(); ImGui::TextColored(ImVec4(0.6f, 0.6f, 0.6f, 1.0f), ICON_MD_INFO " Custom Tile Graphics"); @@ -672,8 +670,8 @@ void MapPropertiesSystem::DrawPalettesPopup(int current_map, int game_state, HOVER_HINT("Main color palette for background tiles"); // Read fresh to reflect ROM upgrades - uint8_t asm_version = (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; - if (asm_version >= 2) { + auto rom_version_pal = zelda3::OverworldVersionHelper::GetVersion(*rom_); + if (zelda3::OverworldVersionHelper::SupportsCustomBGColors(rom_version_pal)) { if (gui::InputHexByte(ICON_MD_COLOR_LENS " Main Palette", overworld_->mutable_overworld_map(current_map) ->mutable_main_palette(), @@ -1037,14 +1035,13 @@ void MapPropertiesSystem::DrawCustomFeaturesTab(int current_map) { ImGui::Text(ICON_MD_PHOTO_SIZE_SELECT_LARGE " Area Size"); TableNextColumn(); // ALL ROMs support Small/Large. Only v3+ supports Wide/Tall. - uint8_t asm_version = - (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; + auto rom_version_basic = zelda3::OverworldVersionHelper::GetVersion(*rom_); int current_area_size = static_cast(overworld_->overworld_map(current_map)->area_size()); ImGui::SetNextItemWidth(130.f); - if (asm_version >= 3 && asm_version != 0xFF) { + if (zelda3::OverworldVersionHelper::SupportsAreaEnum(rom_version_basic)) { // v3+ ROM: Show all 4 area size options static const char* all_sizes[] = {"Small (1x1)", "Large (2x2)", "Wide (2x1)", "Tall (1x2)"}; @@ -1079,7 +1076,7 @@ void MapPropertiesSystem::DrawCustomFeaturesTab(int current_map) { } } - if (asm_version >= 2) { + if (zelda3::OverworldVersionHelper::SupportsCustomBGColors(rom_version_basic)) { TableNextColumn(); ImGui::Text(ICON_MD_COLOR_LENS " Main Palette"); TableNextColumn(); @@ -1096,7 +1093,7 @@ void MapPropertiesSystem::DrawCustomFeaturesTab(int current_map) { } } - if (asm_version >= 3) { + if (zelda3::OverworldVersionHelper::SupportsAnimatedGFX(rom_version_basic)) { TableNextColumn(); ImGui::Text(ICON_MD_ANIMATION " Animated GFX"); TableNextColumn(); @@ -1131,10 +1128,10 @@ void MapPropertiesSystem::DrawCustomFeaturesTab(int current_map) { } void MapPropertiesSystem::DrawTileGraphicsTab(int current_map) { - uint8_t asm_version = (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; + auto rom_version = zelda3::OverworldVersionHelper::GetVersion(*rom_); // Only show custom tile graphics for v1+ ROMs - if (asm_version >= 1 && asm_version != 0xFF) { + if (zelda3::OverworldVersionHelper::SupportsExpandedSpace(rom_version)) { ImGui::Text(ICON_MD_GRID_VIEW " Custom Tile Graphics (8 sheets)"); Separator(); @@ -1350,9 +1347,8 @@ void MapPropertiesSystem::RefreshSiblingMapGraphics(int map_index, bool include_ } void MapPropertiesSystem::DrawMosaicControls(int current_map) { - uint8_t asm_version = - (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; - if (asm_version >= 2) { + auto rom_version = zelda3::OverworldVersionHelper::GetVersion(*rom_); + if (zelda3::OverworldVersionHelper::SupportsCustomBGColors(rom_version)) { ImGui::Separator(); ImGui::Text("Mosaic Effects (per direction):"); @@ -1379,8 +1375,7 @@ void MapPropertiesSystem::DrawMosaicControls(int current_map) { void MapPropertiesSystem::DrawOverlayControls(int current_map, bool& show_overlay_preview) { - uint8_t asm_version = - (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; + auto rom_version = zelda3::OverworldVersionHelper::GetVersion(*rom_); // Determine if this is a special overworld map (0x80-0x9F) bool is_special_overworld_map = (current_map >= 0x80 && current_map < 0xA0); @@ -1491,7 +1486,7 @@ void MapPropertiesSystem::DrawOverlayControls(int current_map, ImGui::Separator(); // Interactive/Dynamic Map Overlay Section (for vanilla ROMs) - if (asm_version == 0xFF) { + if (rom_version == zelda3::OverworldVersion::kVanilla) { ImGui::TextColored(ImVec4(1.0f, 0.8f, 0.4f, 1.0f), ICON_MD_EDIT_NOTE " Map Overlay (Interactive)"); ImGui::SameLine(); @@ -1537,16 +1532,17 @@ void MapPropertiesSystem::DrawOverlayControls(int current_map, // Show version and capability info ImGui::Separator(); - if (asm_version == 0xFF) { + if (rom_version == zelda3::OverworldVersion::kVanilla) { ImGui::TextColored(ImVec4(0.7f, 0.7f, 0.7f, 1.0f), ICON_MD_INFO " Vanilla ROM"); ImGui::BulletText("Visual effects use maps 0x80-0x9F"); ImGui::BulletText("Map overlays are read-only"); } else { + const char* version_name = zelda3::OverworldVersionHelper::GetVersionName(rom_version); ImGui::TextColored(ImVec4(0.4f, 1.0f, 0.4f, 1.0f), - ICON_MD_UPGRADE " ZSCustomOverworld v%d", asm_version); + ICON_MD_UPGRADE " %s", version_name); ImGui::BulletText("Enhanced visual effect control"); - if (asm_version >= 3) { + if (zelda3::OverworldVersionHelper::SupportsAreaEnum(rom_version)) { ImGui::BulletText("Extended overlay system"); ImGui::BulletText("Custom area sizes support"); } @@ -1592,8 +1588,6 @@ void MapPropertiesSystem::DrawOverlayPreviewOnMap(int current_map, uint16_t overlay_id = 0x00FF; bool has_subscreen_overlay = false; - uint8_t asm_version = - (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; bool is_special_overworld_map = (current_map >= 0x80 && current_map < 0xA0); if (is_special_overworld_map) { diff --git a/src/app/editor/overworld/overworld_editor.cc b/src/app/editor/overworld/overworld_editor.cc index f2afc811..4aef67d1 100644 --- a/src/app/editor/overworld/overworld_editor.cc +++ b/src/app/editor/overworld/overworld_editor.cc @@ -56,6 +56,7 @@ #include "zelda3/overworld/overworld_exit.h" #include "zelda3/overworld/overworld_item.h" #include "zelda3/overworld/overworld_map.h" +#include "zelda3/overworld/overworld_version_helper.h" #include "zelda3/sprite/sprite.h" namespace yaze::editor { @@ -491,8 +492,9 @@ void OverworldEditor::DrawToolset() { // Modern adaptive toolbar with inline mode switching and properties static gui::Toolset toolbar; - // IMPORTANT: Don't make asm_version static - it needs to update after ROM upgrade - uint8_t asm_version = (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; + // IMPORTANT: Don't cache version - it needs to update after ROM upgrade + auto rom_version = zelda3::OverworldVersionHelper::GetVersion(*rom_); + uint8_t asm_version = (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; // Still needed for badge display // Don't use WidgetIdScope here - it conflicts with ImGui::Begin/End ID stack in cards // Widgets register themselves individually instead @@ -1256,9 +1258,9 @@ absl::Status OverworldEditor::CheckForCurrentMap() { const int current_highlighted_map = current_map_; - // Check if ZSCustomOverworld v3 is present - uint8_t asm_version = (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; - bool use_v3_area_sizes = (asm_version >= 3); + // Use centralized version detection + auto rom_version = zelda3::OverworldVersionHelper::GetVersion(*rom_); + bool use_v3_area_sizes = zelda3::OverworldVersionHelper::SupportsAreaEnum(rom_version); // Get area size for v3+ ROMs, otherwise use legacy logic if (use_v3_area_sizes) { @@ -1622,7 +1624,8 @@ void OverworldEditor::DrawOverworldCanvas() { MoveEntityOnGrid(dragged_entity_, ow_map_canvas_.zero_point(), ow_map_canvas_.scrolling(), dragged_entity_free_movement_); - dragged_entity_->UpdateMapProperties(dragged_entity_->map_id_); + // Pass overworld context for proper area size detection + dragged_entity_->UpdateMapProperties(dragged_entity_->map_id_, &overworld_); rom_->set_dirty(true); } is_dragging_entity_ = false; @@ -2118,9 +2121,9 @@ void OverworldEditor::RefreshChildMapOnDemand(int map_index) { } // Handle multi-area maps (large, wide, tall) with safe coordination - // Check if ZSCustomOverworld v3 is present - uint8_t asm_version = (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; - bool use_v3_area_sizes = (asm_version >= 3 && asm_version != 0xFF); + // Use centralized version detection + auto rom_version = zelda3::OverworldVersionHelper::GetVersion(*rom_); + bool use_v3_area_sizes = zelda3::OverworldVersionHelper::SupportsAreaEnum(rom_version); if (use_v3_area_sizes) { // Use v3 multi-area coordination @@ -2302,9 +2305,9 @@ absl::Status OverworldEditor::RefreshMapPalette() { overworld_.mutable_overworld_map(current_map_)->LoadPalette()); const auto current_map_palette = overworld_.current_area_palette(); - // Check if ZSCustomOverworld v3 is present - uint8_t asm_version = (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; - bool use_v3_area_sizes = (asm_version >= 3 && asm_version != 0xFF); + // Use centralized version detection + auto rom_version = zelda3::OverworldVersionHelper::GetVersion(*rom_); + bool use_v3_area_sizes = zelda3::OverworldVersionHelper::SupportsAreaEnum(rom_version); if (use_v3_area_sizes) { // Use v3 area size system @@ -2444,9 +2447,9 @@ void OverworldEditor::RefreshSiblingMapGraphics(int map_index, void OverworldEditor::RefreshMapProperties() { const auto& current_ow_map = *overworld_.mutable_overworld_map(current_map_); - // Check if ZSCustomOverworld v3 is present - uint8_t asm_version = (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; - bool use_v3_area_sizes = (asm_version >= 3); + // Use centralized version detection + auto rom_version = zelda3::OverworldVersionHelper::GetVersion(*rom_); + bool use_v3_area_sizes = zelda3::OverworldVersionHelper::SupportsAreaEnum(rom_version); if (use_v3_area_sizes) { // Use v3 area size system diff --git a/src/app/editor/overworld/overworld_entity_renderer.cc b/src/app/editor/overworld/overworld_entity_renderer.cc index 93e1ae81..d1dae2a4 100644 --- a/src/app/editor/overworld/overworld_entity_renderer.cc +++ b/src/app/editor/overworld/overworld_entity_renderer.cc @@ -1,4 +1,5 @@ #include "overworld_entity_renderer.h" +#include #include "absl/strings/str_format.h" #include "core/features.h" @@ -7,6 +8,7 @@ #include "zelda3/common.h" #include "util/hex.h" #include "imgui/imgui.h" +#include "zelda3/overworld/overworld_item.h" namespace yaze { namespace editor { @@ -24,7 +26,7 @@ ImVec4 GetSpriteColor() { return ImVec4{1.0f, 0.0f, 1.0f, 1.0f}; } // So void OverworldEntityRenderer::DrawEntrances(ImVec2 canvas_p0, ImVec2 scrolling, int current_world, int current_mode) { - hovered_entity_ = nullptr; + // Don't reset hovered_entity_ here - DrawExits resets it (called first) int i = 0; for (auto& each : overworld_->entrances()) { if (each.map_id_ < 0x40 + (current_world * 0x40) && @@ -56,11 +58,15 @@ void OverworldEntityRenderer::DrawEntrances(ImVec2 canvas_p0, ImVec2 scrolling, void OverworldEntityRenderer::DrawExits(ImVec2 canvas_p0, ImVec2 scrolling, int current_world, int current_mode) { + // Reset hover state at the start of entity rendering (DrawExits is called first) + hovered_entity_ = nullptr; + int i = 0; for (auto& each : *overworld_->mutable_exits()) { if (each.map_id_ < 0x40 + (current_world * 0x40) && each.map_id_ >= (current_world * 0x40) && !each.deleted_) { canvas_->DrawRect(each.x_, each.y_, 16, 16, GetExitColor()); + if (IsMouseHoveringOverEntity(each, canvas_p0, scrolling)) { hovered_entity_ = &each; } @@ -91,7 +97,7 @@ void OverworldEntityRenderer::DrawItems(int current_world, int current_mode) { } - std::string item_name = ""; + std::string item_name = ""; if (item.id_ < zelda3::kSecretItemNames.size()) { item_name = zelda3::kSecretItemNames[item.id_]; } else { diff --git a/src/cli/handlers/game/overworld_inspect.cc b/src/cli/handlers/game/overworld_inspect.cc index 7a29a038..93b5f9b8 100644 --- a/src/cli/handlers/game/overworld_inspect.cc +++ b/src/cli/handlers/game/overworld_inspect.cc @@ -451,8 +451,8 @@ absl::StatusOr GetEntranceDetails( details.x = entrance.x_; details.y = entrance.y_; - details.area_x = entrance.area_x_; - details.area_y = entrance.area_y_; + details.area_x = entrance.game_x_; + details.area_y = entrance.game_y_; details.map_pos = entrance.map_pos_; details.is_hole = entrance.is_hole_; diff --git a/src/zelda3/common.h b/src/zelda3/common.h index 734ecdda..e0791090 100644 --- a/src/zelda3/common.h +++ b/src/zelda3/common.h @@ -13,6 +13,18 @@ namespace yaze::zelda3{ /** * @class GameEntity * @brief Base class for all overworld and dungeon entities. + * + * Coordinate System (matches ZScream naming conventions): + * - x_, y_: World coordinates in pixels (0-4095 for overworld) + * ZScream equivalent: PlayerX/PlayerY (ExitOW.cs), GlobalX/GlobalY (EntranceOW.cs) + * + * - game_x_, game_y_: Map-local tile coordinates (0-63 for normal, 0-31 for small areas) + * ZScream equivalent: AreaX/AreaY (ExitOW.cs), GameX/GameY (items/sprites) + * + * - map_id_: Parent map ID accounting for large/wide/tall multi-area maps + * ZScream equivalent: MapID property + * + * - entity_id_: Index in entity array (for display/debugging) */ class GameEntity { public: @@ -27,11 +39,22 @@ class GameEntity { kProperties = 7, kDungeonSprite = 8, } entity_type_; + + // World coordinates (0-4095 for overworld) + // ZScream: PlayerX/PlayerY (exits), GlobalX/GlobalY (entrances) int x_ = 0; int y_ = 0; + + // Map-local game coordinates (0-63 tiles, or 0-31 for small areas) + // ZScream: AreaX/AreaY (exits), GameX/GameY (items/sprites) int game_x_ = 0; int game_y_ = 0; + + // Entity index in array (for display/debugging) int entity_id_ = 0; + + // Parent map ID (accounting for large/wide/tall areas) + // ZScream: MapID property uint16_t map_id_ = 0; auto set_x(int x) { x_ = x; } @@ -40,7 +63,19 @@ class GameEntity { GameEntity() = default; virtual ~GameEntity() {} - virtual void UpdateMapProperties(uint16_t map_id) = 0; + /** + * @brief Update entity properties based on map position + * @param map_id Parent map ID to update to + * @param context Optional context (typically const Overworld* for coordinate calculations) + * + * ZScream equivalent: UpdateMapStuff() / UpdateMapProperties() + * + * This method recalculates derived properties like: + * - game_x_/game_y_ from world x_/y_ coordinates + * - Scroll/camera values for exits (if is_automatic_ = true) + * - Map position encoding for saving + */ + virtual void UpdateMapProperties(uint16_t map_id, const void* context = nullptr) = 0; }; constexpr int kNumOverworldMaps = 160; diff --git a/src/zelda3/overworld/overworld_entrance.h b/src/zelda3/overworld/overworld_entrance.h index ad31476a..d59ee55f 100644 --- a/src/zelda3/overworld/overworld_entrance.h +++ b/src/zelda3/overworld/overworld_entrance.h @@ -84,8 +84,6 @@ class OverworldEntrance : public GameEntity { public: uint16_t map_pos_; uint8_t entrance_id_; - uint8_t area_x_; - uint8_t area_y_; bool is_hole_ = false; bool deleted = false; @@ -95,30 +93,35 @@ class OverworldEntrance : public GameEntity { : map_pos_(map_pos), entrance_id_(entrance_id), is_hole_(hole) { x_ = x; y_ = y; - map_id_ = map_id; + map_id_ = map_id; // Store original map_id (no corruption) entity_id_ = entrance_id; entity_type_ = kEntrance; - int mapX = (map_id_ - ((map_id_ / 8) * 8)); - int mapY = (map_id_ / 8); - area_x_ = (uint8_t)((std::abs(x - (mapX * 512)) / 16)); - area_y_ = (uint8_t)((std::abs(y - (mapY * 512)) / 16)); + // Use normalized map_id for coordinate calculations + uint8_t normalized_map_id = map_id % 0x40; + int mapX = normalized_map_id % 8; + int mapY = normalized_map_id / 8; + + // Use base game_x_/game_y_ instead of duplicated area_x_/area_y_ + game_x_ = static_cast((std::abs(x - (mapX * 512)) / 16)); + game_y_ = static_cast((std::abs(y - (mapY * 512)) / 16)); } - void UpdateMapProperties(uint16_t map_id) override { + void UpdateMapProperties(uint16_t map_id, const void* context = nullptr) override { + (void)context; // Not used by entrances currently map_id_ = map_id; - if (map_id_ >= 64) { - map_id_ -= 64; - } + // Use normalized map_id for calculations (don't corrupt stored value) + uint8_t normalized_map_id = map_id % 0x40; + int mapX = normalized_map_id % 8; + int mapY = normalized_map_id / 8; - int mapX = (map_id_ - ((map_id_ / 8) * 8)); - int mapY = (map_id_ / 8); + // Update game coordinates from world coordinates + game_x_ = static_cast((std::abs(x_ - (mapX * 512)) / 16)); + game_y_ = static_cast((std::abs(y_ - (mapY * 512)) / 16)); - area_x_ = (uint8_t)((std::abs(x_ - (mapX * 512)) / 16)); - area_y_ = (uint8_t)((std::abs(y_ - (mapY * 512)) / 16)); - - map_pos_ = (uint16_t)((((area_y_) << 6) | (area_x_ & 0x3F)) << 1); + // Calculate map position encoding for ROM save + map_pos_ = (uint16_t)((((game_y_) << 6) | (game_x_ & 0x3F)) << 1); } }; constexpr int kEntranceTileTypePtrLow = 0xDB8BF; diff --git a/src/zelda3/overworld/overworld_exit.cc b/src/zelda3/overworld/overworld_exit.cc index 0176650e..5220c40a 100644 --- a/src/zelda3/overworld/overworld_exit.cc +++ b/src/zelda3/overworld/overworld_exit.cc @@ -1,11 +1,17 @@ #include "zelda3/overworld/overworld_exit.h" + +#include +#include +#include +#include + #include "absl/status/status.h" #include "absl/status/statusor.h" #include "app/rom.h" #include "util/macro.h" #include "zelda3/common.h" -#include -#include +#include "zelda3/overworld/overworld.h" +#include "zelda3/overworld/overworld_version_helper.h" namespace yaze::zelda3 { @@ -13,7 +19,7 @@ absl::StatusOr> LoadExits(Rom* rom) { const int NumberOfOverworldExits = 0x4F; std::vector exits; for (int i = 0; i < NumberOfOverworldExits; i++) { - auto rom_data = rom->data(); + const auto* rom_data = rom->data(); uint16_t exit_room_id; uint16_t exit_map_id; @@ -39,19 +45,128 @@ absl::StatusOr> LoadExits(Rom* rom) { OWExitDoorType1 + (i * 2), exit_door_type_2, OWExitDoorType2 + (i * 2))); - uint16_t py = (uint16_t)((rom_data[OWExitYPlayer + (i * 2) + 1] << 8) + - rom_data[OWExitYPlayer + (i * 2)]); - uint16_t px = (uint16_t)((rom_data[OWExitXPlayer + (i * 2) + 1] << 8) + - rom_data[OWExitXPlayer + (i * 2)]); + uint16_t player_y = static_cast((rom_data[OWExitYPlayer + (i * 2) + 1] << 8) + + rom_data[OWExitYPlayer + (i * 2)]); + uint16_t player_x = static_cast((rom_data[OWExitXPlayer + (i * 2) + 1] << 8) + + rom_data[OWExitXPlayer + (i * 2)]); exits.emplace_back(exit_room_id, exit_map_id, exit_vram, exit_y_scroll, - exit_x_scroll, py, px, exit_y_camera, exit_x_camera, + exit_x_scroll, player_y, player_x, exit_y_camera, exit_x_camera, exit_scroll_mod_y, exit_scroll_mod_x, exit_door_type_1, - exit_door_type_2, (px & py) == 0xFFFF); + exit_door_type_2, (player_x & player_y) == 0xFFFF); } return exits; } +void OverworldExit::UpdateMapProperties(uint16_t map_id, const void* context) { + // Sync player position from drag system + // ZScream: ExitMode.cs:229-244 updates PlayerX/PlayerY, then calls UpdateMapStuff + x_player_ = static_cast(x_); + y_player_ = static_cast(y_); + map_id_ = map_id; + + // FIX Bug 3: Query actual area size from overworld + // ZScream: ExitOW.cs:210-212 + int area_size_x = 256; + int area_size_y = 256; + + if (context != nullptr) { + const auto* overworld = static_cast(context); + auto area_size = overworld->overworld_map(map_id)->area_size(); + + // Calculate area dimensions based on size enum + if (area_size == AreaSizeEnum::LargeArea) { + area_size_x = area_size_y = 768; + } else if (area_size == AreaSizeEnum::WideArea) { + area_size_x = 768; + area_size_y = 256; + } else if (area_size == AreaSizeEnum::TallArea) { + area_size_x = 256; + area_size_y = 768; + } + } + + // FIX Bug 5: Normalize map_id FIRST before using for calculations + // ZScream: ExitOW.cs:214 + uint8_t normalized_map_id = map_id % 0x40; + + // Calculate map grid position + // ZScream: ExitOW.cs:216-217 + int mapX = normalized_map_id % 8; + int mapY = normalized_map_id / 8; + + // Calculate game coordinates (map-local tile position) + // ZScream: ExitOW.cs:219-220 + game_x_ = static_cast((std::abs(x_ - (mapX * 512)) / 16)); + game_y_ = static_cast((std::abs(y_ - (mapY * 512)) / 16)); + + // Clamp to valid range based on area size + // ZScream: ExitOW.cs:222-234 + int max_game_x = (area_size_x == 256) ? 31 : 63; + int max_game_y = (area_size_y == 256) ? 31 : 63; + game_x_ = std::clamp(game_x_, 0, max_game_x); + game_y_ = std::clamp(game_y_, 0, max_game_y); + + // Map base coordinates in world space + // ZScream: ExitOW.cs:237-238 (mapx, mapy) + int mapx = (normalized_map_id & 7) << 9; // * 512 + int mapy = (normalized_map_id & 56) << 6; // (map_id / 8) * 512 + + if (is_automatic_) { + // Auto-calculate scroll and camera from player position + // ZScream: ExitOW.cs:256-309 + + // Base scroll calculation (player centered in screen) + x_scroll_ = x_player_ - 120; + y_scroll_ = y_player_ - 80; + + // Clamp scroll to map bounds using actual area size + if (x_scroll_ < mapx) { + x_scroll_ = mapx; + } + + if (y_scroll_ < mapy) { + y_scroll_ = mapy; + } + + if (x_scroll_ > mapx + area_size_x) { + x_scroll_ = mapx + area_size_x; + } + + if (y_scroll_ > mapy + area_size_y + 32) { + y_scroll_ = mapy + area_size_y + 32; + } + + // Camera position (offset from player) + x_camera_ = x_player_ + 0x07; + y_camera_ = y_player_ + 0x1F; + + // Clamp camera to valid range + if (x_camera_ < mapx + 127) { + x_camera_ = mapx + 127; + } + + if (y_camera_ < mapy + 111) { + y_camera_ = mapy + 111; + } + + if (x_camera_ > mapx + 127 + area_size_x) { + x_camera_ = mapx + 127 + area_size_x; + } + + if (y_camera_ > mapy + 143 + area_size_y) { + y_camera_ = mapy + 143 + area_size_y; + } + } + + // Calculate VRAM location from scroll values + // ZScream: ExitOW.cs:312-315 + int16_t vram_x_scroll = static_cast(x_scroll_ - mapx); + int16_t vram_y_scroll = static_cast(y_scroll_ - mapy); + + map_pos_ = static_cast(((vram_y_scroll & 0xFFF0) << 3) | + ((vram_x_scroll & 0xFFF0) >> 3)); +} absl::Status SaveExits(Rom* rom, const std::vector& exits) { @@ -62,7 +177,8 @@ absl::Status SaveExits(Rom* rom, const std::vector& exits) { uint8_t asm_version = (*rom)[OverworldCustomASMHasBeenApplied]; if (asm_version == 0x00) { // Apply special fix for Zora's Domain exit (index 0x4D) - // TODO: Implement SpecialUpdatePosition for OverworldExit + // TODO(scawful): Implement SpecialUpdatePosition for OverworldExit + // Similar to ZScream Save.cs:1034-1039 // if (all_exits_.size() > 0x4D) { // all_exits_[0x4D].SpecialUpdatePosition(); // } @@ -110,4 +226,6 @@ absl::Status SaveExits(Rom* rom, const std::vector& exits) { return absl::OkStatus(); } + + } // namespace yaze::zelda3 \ No newline at end of file diff --git a/src/zelda3/overworld/overworld_exit.h b/src/zelda3/overworld/overworld_exit.h index 19caeacd..2450f55f 100644 --- a/src/zelda3/overworld/overworld_exit.h +++ b/src/zelda3/overworld/overworld_exit.h @@ -2,13 +2,20 @@ #define YAZE_APP_ZELDA3_OVERWORLD_EXIT_H #include -#include +#include +#include +#include "absl/status/status.h" +#include "absl/status/statusor.h" #include "app/rom.h" #include "zelda3/common.h" +#include "zelda3/overworld/overworld_version_helper.h" namespace yaze::zelda3 { +// Forward declaration to avoid circular dependency +class Overworld; + constexpr int kNumOverworldExits = 0x4F; constexpr int OWExitRoomId = 0x15D8A; // 0x15E07 Credits sequences // 105C2 Ending maps @@ -39,173 +46,108 @@ constexpr int OWExitUnk1Whirlpool = 0x16BF5; // JP = ;016E91 constexpr int OWExitUnk2Whirlpool = 0x16C17; // JP = ;016EB3 constexpr int OWWhirlpoolPosition = 0x16CF8; // JP = ;016F94 +/** + * @class OverworldExit + * @brief Represents an overworld exit that transitions from dungeon to overworld + * + * Coordinate System (inherited from GameEntity): + * - x_, y_: World pixel coordinates (ZScream: PlayerX/PlayerY) + * - game_x_, game_y_: Map-local tile coordinates (ZScream: AreaX/AreaY) + * - map_id_: Parent map ID (ZScream: MapID) + * + * Exit-Specific Properties: + * - x_player_, y_player_: Player spawn position in world (saved to ROM) + * - x_scroll_, y_scroll_: Camera scroll position + * - x_camera_, y_camera_: Camera center position + * - room_id_: Target dungeon room ID (ZScream: RoomID) + * - is_automatic_: If true, scroll/camera auto-calculated from player position (ZScream: IsAutomatic) + * + * ZScream Reference: ExitOW.cs, ExitMode.cs + */ class OverworldExit : public GameEntity { public: uint16_t y_scroll_; uint16_t x_scroll_; - uint16_t y_player_; // CRITICAL: Changed from uint8_t to uint16_t (0-4088 range) - uint16_t x_player_; // CRITICAL: Changed from uint8_t to uint16_t (0-4088 range) - uint16_t y_camera_; // Changed from uint8_t to uint16_t for consistency - uint16_t x_camera_; // Changed from uint8_t to uint16_t for consistency + uint16_t y_player_; // Player spawn Y (0-4088 range, ZScream: PlayerY) + uint16_t x_player_; // Player spawn X (0-4088 range, ZScream: PlayerX) + uint16_t y_camera_; // Camera Y position + uint16_t x_camera_; // Camera X position uint8_t scroll_mod_y_; uint8_t scroll_mod_x_; uint16_t door_type_1_; uint16_t door_type_2_; - uint16_t room_id_; - uint16_t map_pos_; // Position in the vram - uint8_t entrance_id_; - uint8_t area_x_; - uint8_t area_y_; + uint16_t room_id_; // ZScream: RoomID + uint16_t map_pos_; // VRAM location (ZScream: VRAMLocation) bool is_hole_ = false; bool deleted_ = false; - bool is_automatic_ = false; - bool large_map_ = false; + bool is_automatic_ = true; // FIX: Default to true (matches ZScream ExitOW.cs:101) OverworldExit() = default; + + /** + * @brief Constructor for loading exits from ROM + * + * Matches ZScream ExitOW.cs constructor (lines 129-167) + * + * CRITICAL: Does NOT modify map_id parameter (Bug 1 fix) + * Uses temporary normalized_map_id for calculations only. + */ OverworldExit(uint16_t room_id, uint8_t map_id, uint16_t vram_location, uint16_t y_scroll, uint16_t x_scroll, uint16_t player_y, uint16_t player_x, uint16_t camera_y, uint16_t camera_x, uint8_t scroll_mod_y, uint8_t scroll_mod_x, uint16_t door_type_1, uint16_t door_type_2, bool deleted = false) - : map_pos_(vram_location), - entrance_id_(0), - area_x_(0), - area_y_(0), - room_id_(room_id), - y_scroll_(y_scroll), + : y_scroll_(y_scroll), x_scroll_(x_scroll), - y_player_(player_y), // No cast - preserve full 16-bit value - x_player_(player_x), // No cast - preserve full 16-bit value - y_camera_(camera_y), // No cast - preserve full 16-bit value - x_camera_(camera_x), // No cast - preserve full 16-bit value + y_player_(player_y), + x_player_(player_x), + y_camera_(camera_y), + x_camera_(camera_x), scroll_mod_y_(scroll_mod_y), scroll_mod_x_(scroll_mod_x), door_type_1_(door_type_1), door_type_2_(door_type_2), + room_id_(room_id), + map_pos_(vram_location), is_hole_(false), deleted_(deleted) { - // Initialize entity variables with full 16-bit coordinates + // Initialize base entity fields (ZScream: PlayerX/PlayerY → x_/y_) x_ = player_x; y_ = player_y; - map_id_ = map_id; + map_id_ = map_id; // FIX Bug 1: Store original map_id WITHOUT modification entity_type_ = kExit; - int mapX = (map_id_ - ((map_id_ / 8) * 8)); - int mapY = (map_id_ / 8); + // Calculate game coordinates using normalized map_id (0-63 range) + // ZScream: ExitOW.cs:159-163 + uint8_t normalized_map_id = map_id % 0x40; + int mapX = normalized_map_id % 8; + int mapY = normalized_map_id / 8; - area_x_ = (uint8_t)((std::abs(x_ - (mapX * 512)) / 16)); - area_y_ = (uint8_t)((std::abs(y_ - (mapY * 512)) / 16)); - - if (door_type_1 != 0) { - int p = (door_type_1 & 0x7FFF) >> 1; - entrance_id_ = (uint8_t)(p % 64); - area_y_ = (uint8_t)(p >> 6); - } - - if (door_type_2 != 0) { - int p = (door_type_2 & 0x7FFF) >> 1; - entrance_id_ = (uint8_t)(p % 64); - area_y_ = (uint8_t)(p >> 6); - } - - if (map_id_ >= 64) { - map_id_ -= 64; - } - - mapX = (map_id_ - ((map_id_ / 8) * 8)); - mapY = (map_id_ / 8); - - area_x_ = (uint8_t)((std::abs(x_ - (mapX * 512)) / 16)); - area_y_ = (uint8_t)((std::abs(y_ - (mapY * 512)) / 16)); - - map_pos_ = (uint16_t)((((area_y_) << 6) | (area_x_ & 0x3F)) << 1); - } - - // Overworld overworld - void UpdateMapProperties(uint16_t map_id) override { - // CRITICAL FIX: Sync player position from base entity coordinates - // The drag system in overworld_editor.cc updates x_/y_ (base GameEntity fields), - // but exit auto-calculation uses x_player_/y_player_ for scroll/camera computation. - // Without this sync, dragged exits retain old scroll values, causing save corruption. - // Matches ZScream ExitMode.cs:229-244 where PlayerX/PlayerY are updated during drag, - // then UpdateMapStuff recalculates scroll/camera from those values. - x_player_ = static_cast(x_); - y_player_ = static_cast(y_); + // FIX Bug 4: Calculate game coords ONCE using correct formula + // ZScream: ExitOW.cs:162-163 (AreaX/AreaY) + game_x_ = static_cast((std::abs(x_ - (mapX * 512)) / 16)); + game_y_ = static_cast((std::abs(y_ - (mapY * 512)) / 16)); - map_id_ = map_id; - - int large = 256; - int mapid = map_id; - - if (map_id < 128) { - large = large_map_ ? 768 : 256; - // if (overworld.overworld_map(map_id)->Parent() != map_id) { - // mapid = overworld.overworld_map(map_id)->Parent(); - // } - } - - int mapX = map_id - ((map_id / 8) * 8); - int mapY = map_id / 8; - - area_x_ = (uint8_t)((std::abs(x_ - (mapX * 512)) / 16)); - area_y_ = (uint8_t)((std::abs(y_ - (mapY * 512)) / 16)); - - if (map_id >= 64) { - map_id -= 64; - } - - int mapx = (map_id & 7) << 9; - int mapy = (map_id & 56) << 6; - - if (is_automatic_) { - // Auto-calculate scroll and camera from player position - // Matches ZScream ExitOW.cs:256-309 - x_scroll_ = x_player_ - 120; - y_scroll_ = y_player_ - 80; - - if (x_scroll_ < mapx) { - x_scroll_ = mapx; - } - - if (y_scroll_ < mapy) { - y_scroll_ = mapy; - } - - if (x_scroll_ > mapx + large) { - x_scroll_ = mapx + large; - } - - if (y_scroll_ > mapy + large + 32) { - y_scroll_ = mapy + large + 32; - } - - x_camera_ = x_player_ + 0x07; - y_camera_ = y_player_ + 0x1F; - - if (x_camera_ < mapx + 127) { - x_camera_ = mapx + 127; - } - - if (y_camera_ < mapy + 111) { - y_camera_ = mapy + 111; - } - - if (x_camera_ > mapx + 127 + large) { - x_camera_ = mapx + 127 + large; - } - - if (y_camera_ > mapy + 143 + large) { - y_camera_ = mapy + 143 + large; - } - } - - short vram_x_scroll = (short)(x_scroll_ - mapx); - short vram_y_scroll = (short)(y_scroll_ - mapy); - - map_pos_ = (uint16_t)(((vram_y_scroll & 0xFFF0) << 3) | - ((vram_x_scroll & 0xFFF0) >> 3)); + // Door position calculations (used by door editor, not for coordinates) + // ZScream: ExitOW.cs:145-157 + // These update DoorXEditor/DoorYEditor, NOT AreaX/AreaY } + + /** + * @brief Update exit properties when moved or map changes + * @param map_id Parent map ID to update to + * @param context Pointer to const Overworld for area size queries (can be nullptr for vanilla) + * + * Recalculates: + * - game_x_/game_y_ (map-local tile coords) + * - x_scroll_/y_scroll_ (if is_automatic_ = true) + * - x_camera_/y_camera_ (if is_automatic_ = true) + * - map_pos_ (VRAM location) + * + * ZScream equivalent: ExitOW.cs:206-318 (UpdateMapStuff) + */ + void UpdateMapProperties(uint16_t map_id, const void* context) override; }; absl::StatusOr> LoadExits(Rom* rom); diff --git a/src/zelda3/overworld/overworld_item.h b/src/zelda3/overworld/overworld_item.h index 4911031f..0c5b1992 100644 --- a/src/zelda3/overworld/overworld_item.h +++ b/src/zelda3/overworld/overworld_item.h @@ -45,27 +45,29 @@ class OverworldItem : public GameEntity { : bg2_(bg2), id_(id), room_map_id_(room_map_id) { x_ = x; y_ = y; - map_id_ = room_map_id; + map_id_ = room_map_id; // Store original map_id entity_id_ = id; entity_type_ = kItem; - int map_x = room_map_id - ((room_map_id / 8) * 8); - int map_y = room_map_id / 8; + // Use normalized map_id for coordinate calculations + uint8_t normalized_map_id = room_map_id % 0x40; + int map_x = normalized_map_id % 8; + int map_y = normalized_map_id / 8; game_x_ = static_cast(std::abs(x - (map_x * 512)) / 16); game_y_ = static_cast(std::abs(y - (map_y * 512)) / 16); } - void UpdateMapProperties(uint16_t room_map_id) override { + void UpdateMapProperties(uint16_t room_map_id, const void* context = nullptr) override { + (void)context; // Not used by items currently room_map_id_ = room_map_id; - if (room_map_id_ >= 64) { - room_map_id_ -= 64; - } - - int map_x = room_map_id_ - ((room_map_id_ / 8) * 8); - int map_y = room_map_id_ / 8; + // Use normalized map_id for calculations (don't corrupt stored value) + uint8_t normalized_map_id = room_map_id % 0x40; + int map_x = normalized_map_id % 8; + int map_y = normalized_map_id / 8; + // Update game coordinates from world coordinates game_x_ = static_cast(std::abs(x_ - (map_x * 512)) / 16); game_y_ = static_cast(std::abs(y_ - (map_y * 512)) / 16); } diff --git a/src/zelda3/overworld/overworld_map.h b/src/zelda3/overworld/overworld_map.h index eec8102b..f5cfdb71 100644 --- a/src/zelda3/overworld/overworld_map.h +++ b/src/zelda3/overworld/overworld_map.h @@ -9,6 +9,7 @@ #include "absl/status/status.h" #include "app/gfx/types/snes_palette.h" #include "app/gfx/types/snes_tile.h" +#include "zelda3/overworld/overworld_version_helper.h" #include "app/rom.h" namespace yaze { @@ -83,12 +84,7 @@ typedef struct OverworldMapTiles { OverworldBlockset special_world; // 32 maps } OverworldMapTiles; -enum class AreaSizeEnum { - SmallArea = 0, - LargeArea = 1, - WideArea = 2, - TallArea = 3, -}; + /** * @brief Represents a single Overworld map screen. diff --git a/src/zelda3/overworld/overworld_version_helper.h b/src/zelda3/overworld/overworld_version_helper.h new file mode 100644 index 00000000..ba9224ff --- /dev/null +++ b/src/zelda3/overworld/overworld_version_helper.h @@ -0,0 +1,147 @@ +#ifndef YAZE_ZELDA3_OVERWORLD_VERSION_HELPER_H +#define YAZE_ZELDA3_OVERWORLD_VERSION_HELPER_H + +#include +#include "app/rom.h" +#include "zelda3/common.h" + +namespace yaze::zelda3 { + +enum class AreaSizeEnum { + SmallArea = 0, + LargeArea = 1, + WideArea = 2, + TallArea = 3, +}; + +/** + * @brief ROM version detection for overworld features + * + * Centralizes version checks to distinguish between: + * - Vanilla: No ZScream patches (uses parent system for large maps only) + * - v1: Basic custom overworld features + * - v2: Parent system + BG colors + main palettes + * - v3: Area enum system + wide/tall areas + all features + */ +enum class OverworldVersion { + kVanilla = 0, // 0xFF in ROM, no ZScream ASM applied + kZSCustomV1 = 1, // Basic features, expanded pointers + kZSCustomV2 = 2, // Parent system, BG colors, main palettes + kZSCustomV3 = 3 // Area enum, wide/tall areas, all features +}; + +/** + * @brief Helper for ROM version detection and feature gating + * + * Provides consistent version checking across the codebase to replace + * scattered inline checks like: + * if (asm_version >= 3 && asm_version != 0xFF) { ... } + * + * With semantic helpers: + * if (OverworldVersionHelper::SupportsAreaEnum(version)) { ... } + */ +class OverworldVersionHelper { + public: + /** + * @brief Detect ROM version from ASM marker + * @param rom ROM to check + * @return Detected overworld version + */ + static OverworldVersion GetVersion(const Rom& rom) { + uint8_t asm_version = rom.data()[OverworldCustomASMHasBeenApplied]; + + // 0xFF = vanilla ROM (no ZScream ASM applied) + if (asm_version == 0xFF || asm_version == 0x00) { + return OverworldVersion::kVanilla; + } + + // Otherwise return version number directly + if (asm_version == 1) { + return OverworldVersion::kZSCustomV1; + } else if (asm_version == 2) { + return OverworldVersion::kZSCustomV2; + } else if (asm_version >= 3) { + return OverworldVersion::kZSCustomV3; + } + + // Fallback for unknown values + return OverworldVersion::kVanilla; + } + + /** + * @brief Check if ROM supports area enum system (v3+ only) + * + * Area enum system allows: + * - Wide areas (2x1 screens) + * - Tall areas (1x2 screens) + * - Direct area size queries + * + * Vanilla/v1/v2 use parent system with large_map_ flag only. + */ + static bool SupportsAreaEnum(OverworldVersion version) { + return version == OverworldVersion::kZSCustomV3; + } + + /** + * @brief Check if ROM uses expanded space for overworld data + * + * v1+ ROMs use expanded pointers for: + * - Map data (0x130000+) + * - Sprite data (0x141438+) + * - Message IDs (0x1417F8+) + */ + static bool SupportsExpandedSpace(OverworldVersion version) { + return version != OverworldVersion::kVanilla; + } + + /** + * @brief Check if ROM supports custom background colors (v2+) + */ + static bool SupportsCustomBGColors(OverworldVersion version) { + return version == OverworldVersion::kZSCustomV2 || + version == OverworldVersion::kZSCustomV3; + } + + /** + * @brief Check if ROM supports custom tile GFX groups (v3+) + */ + static bool SupportsCustomTileGFX(OverworldVersion version) { + return version == OverworldVersion::kZSCustomV3; + } + + /** + * @brief Check if ROM supports animated GFX (v3+) + */ + static bool SupportsAnimatedGFX(OverworldVersion version) { + return version == OverworldVersion::kZSCustomV3; + } + + /** + * @brief Check if ROM supports subscreen overlays (v3+) + */ + static bool SupportsSubscreenOverlay(OverworldVersion version) { + return version == OverworldVersion::kZSCustomV3; + } + + /** + * @brief Get human-readable version name + */ + static const char* GetVersionName(OverworldVersion version) { + switch (version) { + case OverworldVersion::kVanilla: + return "Vanilla"; + case OverworldVersion::kZSCustomV1: + return "ZSCustomOverworld v1"; + case OverworldVersion::kZSCustomV2: + return "ZSCustomOverworld v2"; + case OverworldVersion::kZSCustomV3: + return "ZSCustomOverworld v3"; + default: + return "Unknown"; + } + } +}; + +} // namespace yaze::zelda3 + +#endif // YAZE_ZELDA3_OVERWORLD_VERSION_HELPER_H diff --git a/src/zelda3/sprite/sprite.cc b/src/zelda3/sprite/sprite.cc index bbdf5bc2..271eeed1 100644 --- a/src/zelda3/sprite/sprite.cc +++ b/src/zelda3/sprite/sprite.cc @@ -6,7 +6,8 @@ namespace yaze { namespace zelda3 { -void Sprite::UpdateMapProperties(uint16_t map_id) { +void Sprite::UpdateMapProperties(uint16_t map_id, const void* context) { + (void)context; // Not used by sprites currently map_x_ = x_; map_y_ = y_; name_ = kSpriteDefaultNames[id_]; diff --git a/src/zelda3/sprite/sprite.h b/src/zelda3/sprite/sprite.h index c987d743..e2fd9f13 100644 --- a/src/zelda3/sprite/sprite.h +++ b/src/zelda3/sprite/sprite.h @@ -331,7 +331,7 @@ class Sprite : public GameEntity { bool mirror_x = false, bool mirror_y = false, int sizex = 2, int sizey = 2); - void UpdateMapProperties(uint16_t map_id) override; + void UpdateMapProperties(uint16_t map_id, const void* context = nullptr) override; void UpdateCoordinates(int map_x, int map_y); auto preview_graphics() const { return &preview_gfx_; }