From 41fd19f9bfc8b4784213bfa2614c573329b9955b Mon Sep 17 00:00:00 2001 From: scawful Date: Sun, 5 Oct 2025 19:44:37 -0400 Subject: [PATCH] refactor: Enhance Map Properties System and Overworld Configuration - Updated MapPropertiesSystem to support area size selection for all ROM versions, introducing logic for Wide and Tall options in v3+. - Improved UI elements with tooltips and organized layouts for better user experience when configuring map sizes. - Added ConfigureMultiAreaMap method in Overworld class to manage sibling relationships and update ROM data based on area size. - Implemented safety checks for palette setting in OverworldEditor to ensure valid bitmap surfaces before applying changes. - Cleaned up related code and comments for clarity and maintainability. --- src/app/editor/overworld/map_properties.cc | 135 +++++++++++----- src/app/editor/overworld/overworld_editor.cc | 19 ++- src/app/editor/overworld/overworld_editor.h | 3 - .../overworld/overworld_entity_renderer.cc | 10 +- src/app/zelda3/overworld/overworld.cc | 150 +++++++++++++++++- src/app/zelda3/overworld/overworld.h | 10 ++ src/app/zelda3/overworld/overworld_map.h | 4 + 7 files changed, 280 insertions(+), 51 deletions(-) diff --git a/src/app/editor/overworld/map_properties.cc b/src/app/editor/overworld/map_properties.cc index a9c48a32..c7f19110 100644 --- a/src/app/editor/overworld/map_properties.cc +++ b/src/app/editor/overworld/map_properties.cc @@ -61,17 +61,42 @@ void MapPropertiesSystem::DrawSimplifiedMapSettings( TableNextColumn(); static uint8_t asm_version = (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; - if (asm_version != 0xFF) { - int current_area_size = - static_cast(overworld_->overworld_map(current_map)->area_size()); - ImGui::SetNextItemWidth(kComboAreaSizeWidth); + + // 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) { + // v3+ ROM: Show all 4 area size options if (ImGui::Combo("##AreaSize", ¤t_area_size, kAreaSizeNames, 4)) { - overworld_->mutable_overworld_map(current_map) - ->SetAreaSize(static_cast(current_area_size)); - RefreshOverworldMap(); + auto status = overworld_->ConfigureMultiAreaMap( + current_map, + static_cast(current_area_size)); + if (status.ok()) { + RefreshSiblingMapGraphics(current_map, true); + RefreshOverworldMap(); + } } } else { - ImGui::Text("N/A"); + // Vanilla/v1/v2 ROM: Show only Small/Large (first 2 options) + const char* limited_names[] = {"Small (1x1)", "Large (2x2)"}; + int limited_size = (current_area_size == 0 || current_area_size == 1) ? current_area_size : 0; + + if (ImGui::Combo("##AreaSize", &limited_size, limited_names, 2)) { + // limited_size is 0 (Small) or 1 (Large) + auto size = (limited_size == 1) ? zelda3::AreaSizeEnum::LargeArea + : zelda3::AreaSizeEnum::SmallArea; + auto status = overworld_->ConfigureMultiAreaMap(current_map, size); + if (status.ok()) { + RefreshSiblingMapGraphics(current_map, true); + RefreshOverworldMap(); + } + } + + if (asm_version == 0xFF || asm_version < 3) { + HOVER_HINT("Small (1x1) and Large (2x2) maps. Wide/Tall require v3+"); + } } TableNextColumn(); @@ -660,32 +685,41 @@ void MapPropertiesSystem::DrawPropertiesPopup(int current_map, ImGui::Text(ICON_MD_ASPECT_RATIO " Area Configuration"); ImGui::Separator(); + // ALL ROMs support Small/Large. Only v3+ supports Wide/Tall. static uint8_t asm_version = (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; - if (asm_version != 0xFF) { - int current_area_size = - static_cast(overworld_->overworld_map(current_map)->area_size()); - ImGui::SetNextItemWidth(kComboAreaSizeWidth); + + int current_area_size = + static_cast(overworld_->overworld_map(current_map)->area_size()); + ImGui::SetNextItemWidth(kComboAreaSizeWidth); + + if (asm_version >= 3 && asm_version != 0xFF) { + // v3+ ROM: Show all 4 area size options if (ImGui::Combo(ICON_MD_PHOTO_SIZE_SELECT_LARGE " Size", ¤t_area_size, kAreaSizeNames, 4)) { - overworld_->mutable_overworld_map(current_map) - ->SetAreaSize(static_cast(current_area_size)); - RefreshOverworldMap(); + auto status = overworld_->ConfigureMultiAreaMap( + current_map, + static_cast(current_area_size)); + if (status.ok()) { + RefreshSiblingMapGraphics(current_map, true); + RefreshOverworldMap(); + } } HOVER_HINT("Map area size (1x1, 2x2, 2x1, 1x2 screens)"); } else { - // Vanilla ROM - show small/large map controls - auto* map = overworld_->mutable_overworld_map(current_map); - bool is_small = !map->is_large_map(); - if (ImGui::Checkbox(ICON_MD_CROP_SQUARE " Small Map", &is_small)) { - if (is_small) { - map->SetAsSmallMap(); - } else { - // For vanilla, use default parent and quadrant values - map->SetAsLargeMap(0, 0); + // Vanilla/v1/v2 ROM: Show only Small/Large + const char* limited_names[] = {"Small (1x1)", "Large (2x2)"}; + int limited_size = (current_area_size == 0 || current_area_size == 1) ? current_area_size : 0; + + if (ImGui::Combo(ICON_MD_PHOTO_SIZE_SELECT_LARGE " Size", &limited_size, limited_names, 2)) { + auto size = (limited_size == 1) ? zelda3::AreaSizeEnum::LargeArea + : zelda3::AreaSizeEnum::SmallArea; + auto status = overworld_->ConfigureMultiAreaMap(current_map, size); + if (status.ok()) { + RefreshSiblingMapGraphics(current_map, true); + RefreshOverworldMap(); } - RefreshOverworldMap(); } - HOVER_HINT("Small (1x1) vs Large (2x2) map size"); + HOVER_HINT("Small (1x1) and Large (2x2) maps. Wide/Tall require v3+"); } // Visual Effects Section @@ -922,22 +956,49 @@ void MapPropertiesSystem::DrawCustomFeaturesTab(int current_map) { TableNextColumn(); ImGui::Text(ICON_MD_PHOTO_SIZE_SELECT_LARGE " Area Size"); TableNextColumn(); - static const char* area_size_names[] = {"Small (1x1)", "Large (2x2)", - "Wide (2x1)", "Tall (1x2)"}; + // ALL ROMs support Small/Large. Only v3+ supports Wide/Tall. + static uint8_t asm_version = + (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; + int current_area_size = static_cast(overworld_->overworld_map(current_map)->area_size()); ImGui::SetNextItemWidth(130.f); - if (ImGui::Combo("##AreaSize", ¤t_area_size, area_size_names, 4)) { - overworld_->mutable_overworld_map(current_map) - ->SetAreaSize(static_cast(current_area_size)); - RefreshOverworldMap(); - } - if (ImGui::IsItemHovered()) { - ImGui::SetTooltip("Map size in screens (ZSCustomOverworld feature)"); + + if (asm_version >= 3 && asm_version != 0xFF) { + // v3+ ROM: Show all 4 area size options + static const char* all_sizes[] = {"Small (1x1)", "Large (2x2)", + "Wide (2x1)", "Tall (1x2)"}; + if (ImGui::Combo("##AreaSize", ¤t_area_size, all_sizes, 4)) { + auto status = overworld_->ConfigureMultiAreaMap( + current_map, + static_cast(current_area_size)); + if (status.ok()) { + RefreshSiblingMapGraphics(current_map, true); + RefreshOverworldMap(); + } + } + if (ImGui::IsItemHovered()) { + ImGui::SetTooltip("Map size: Small (1x1), Large (2x2), Wide (2x1), Tall (1x2)"); + } + } else { + // Vanilla/v1/v2 ROM: Show only Small/Large + static const char* limited_sizes[] = {"Small (1x1)", "Large (2x2)"}; + int limited_size = (current_area_size == 0 || current_area_size == 1) ? current_area_size : 0; + + if (ImGui::Combo("##AreaSize", &limited_size, limited_sizes, 2)) { + auto size = (limited_size == 1) ? zelda3::AreaSizeEnum::LargeArea + : zelda3::AreaSizeEnum::SmallArea; + auto status = overworld_->ConfigureMultiAreaMap(current_map, size); + if (status.ok()) { + RefreshSiblingMapGraphics(current_map, true); + RefreshOverworldMap(); + } + } + if (ImGui::IsItemHovered()) { + ImGui::SetTooltip("Map size: Small (1x1), Large (2x2). Wide/Tall require v3+"); + } } - static uint8_t asm_version = - (*rom_)[zelda3::OverworldCustomASMHasBeenApplied]; if (asm_version >= 2) { TableNextColumn(); ImGui::Text(ICON_MD_COLOR_LENS " Main Palette"); diff --git a/src/app/editor/overworld/overworld_editor.cc b/src/app/editor/overworld/overworld_editor.cc index 4ed467f3..c3e13286 100644 --- a/src/app/editor/overworld/overworld_editor.cc +++ b/src/app/editor/overworld/overworld_editor.cc @@ -1741,8 +1741,11 @@ void OverworldEditor::RefreshMultiAreaMapsSafely(int map_index, overworld_.GetMapTiles(current_world_)); if (status.ok()) { maps_bmp_[sibling].set_data(sibling_map->bitmap_data()); - maps_bmp_[sibling].SetPalette( - overworld_.current_area_palette()); + + // SAFETY: Only set palette if bitmap has a valid surface + if (maps_bmp_[sibling].is_active() && maps_bmp_[sibling].surface()) { + maps_bmp_[sibling].SetPalette(overworld_.current_area_palette()); + } maps_bmp_[sibling].set_modified(false); // Update texture if it exists @@ -1836,10 +1839,18 @@ absl::Status OverworldEditor::RefreshMapPalette() { sibling_index += 6; RETURN_IF_ERROR( overworld_.mutable_overworld_map(sibling_index)->LoadPalette()); - maps_bmp_[sibling_index].SetPalette(current_map_palette); + + // SAFETY: Only set palette if bitmap has a valid surface + if (maps_bmp_[sibling_index].is_active() && maps_bmp_[sibling_index].surface()) { + maps_bmp_[sibling_index].SetPalette(current_map_palette); + } } } - maps_bmp_[current_map_].SetPalette(current_map_palette); + + // SAFETY: Only set palette if bitmap has a valid surface + if (maps_bmp_[current_map_].is_active() && maps_bmp_[current_map_].surface()) { + maps_bmp_[current_map_].SetPalette(current_map_palette); + } } return absl::OkStatus(); diff --git a/src/app/editor/overworld/overworld_editor.h b/src/app/editor/overworld/overworld_editor.h index bd709f42..1957f428 100644 --- a/src/app/editor/overworld/overworld_editor.h +++ b/src/app/editor/overworld/overworld_editor.h @@ -337,9 +337,6 @@ class OverworldEditor : public Editor, public gfx::GfxContext { gui::Canvas properties_canvas_; gui::Canvas scratch_canvas_{"ScratchSpace", ImVec2(320, 480), gui::CanvasGridSize::k32x32}; - gui::Table map_settings_table_{kOWMapTable.data(), 8, kOWMapFlags, - ImVec2(0, 0)}; - absl::Status status_; }; } // namespace editor diff --git a/src/app/editor/overworld/overworld_entity_renderer.cc b/src/app/editor/overworld/overworld_entity_renderer.cc index 05919e67..da19c11b 100644 --- a/src/app/editor/overworld/overworld_entity_renderer.cc +++ b/src/app/editor/overworld/overworld_entity_renderer.cc @@ -13,12 +13,12 @@ namespace editor { using namespace ImGui; -// Entity colors - these could be theme-dependent in the future +// Entity colors - solid with good visibility namespace { -ImVec4 GetEntranceColor() { return ImVec4(0.0f, 1.0f, 0.0f, 0.5f); } // Green -ImVec4 GetExitColor() { return ImVec4(1.0f, 0.0f, 0.0f, 0.5f); } // Red -ImVec4 GetItemColor() { return ImVec4(1.0f, 1.0f, 0.0f, 0.5f); } // Yellow -ImVec4 GetSpriteColor() { return ImVec4(1.0f, 0.5f, 0.0f, 0.5f); } // Orange +ImVec4 GetEntranceColor() { return ImVec4(0.0f, 255.0f, 0.0f, 255.0f); } // Solid green +ImVec4 GetExitColor() { return ImVec4(255.0f, 0.0f, 0.0f, 255.0f); } // Solid red +ImVec4 GetItemColor() { return ImVec4(255.0f, 255.0f, 0.0f, 255.0f); } // Solid yellow +ImVec4 GetSpriteColor() { return ImVec4(255.0f, 128.0f, 0.0f, 255.0f); } // Solid orange } // namespace void OverworldEntityRenderer::DrawEntrances(ImVec2 canvas_p0, ImVec2 scrolling, diff --git a/src/app/zelda3/overworld/overworld.cc b/src/app/zelda3/overworld/overworld.cc index 324e4561..19d9d1e1 100644 --- a/src/app/zelda3/overworld/overworld.cc +++ b/src/app/zelda3/overworld/overworld.cc @@ -2,11 +2,11 @@ #include #include +#include #include +#include #include #include -#include -#include #include "absl/status/status.h" #include "app/core/features.h" @@ -211,10 +211,14 @@ void Overworld::AssignMapSizes(std::vector& maps) { case AreaSizeEnum::WideArea: map_checked[i] = true; + // CRITICAL FIX: Set parent for wide area maps + // Map i is parent (left), map i+1 is child (right) + maps[i].SetParent(i); // Parent points to itself maps[i].SetAreaSize(AreaSizeEnum::WideArea); if (i + 1 < static_cast(maps.size())) { map_checked[i + 1] = true; + maps[i + 1].SetParent(i); // Child points to parent maps[i + 1].SetAreaSize(AreaSizeEnum::WideArea); } @@ -223,10 +227,14 @@ void Overworld::AssignMapSizes(std::vector& maps) { case AreaSizeEnum::TallArea: map_checked[i] = true; + // CRITICAL FIX: Set parent for tall area maps + // Map i is parent (top), map i+8 is child (bottom) + maps[i].SetParent(i); // Parent points to itself maps[i].SetAreaSize(AreaSizeEnum::TallArea); if (i + 8 < static_cast(maps.size())) { map_checked[i + 8] = true; + maps[i + 8].SetParent(i); // Child points to parent maps[i + 8].SetAreaSize(AreaSizeEnum::TallArea); } break; @@ -246,6 +254,144 @@ void Overworld::AssignMapSizes(std::vector& maps) { } } +absl::Status Overworld::ConfigureMultiAreaMap(int parent_index, AreaSizeEnum size) { + if (parent_index < 0 || parent_index >= kNumOverworldMaps) { + return absl::InvalidArgumentError( + absl::StrFormat("Invalid parent index: %d", parent_index)); + } + + // Check ROM version + uint8_t asm_version = (*rom_)[OverworldCustomASMHasBeenApplied]; + + // Version requirements: + // - Vanilla (0xFF): Supports Small and Large only + // - v1-v2: Supports Small and Large only + // - v3+: Supports all 4 sizes (Small, Large, Wide, Tall) + if ((size == AreaSizeEnum::WideArea || size == AreaSizeEnum::TallArea) && + (asm_version < 3 || asm_version == 0xFF)) { + return absl::FailedPreconditionError( + "Wide and Tall areas require ZSCustomOverworld v3+"); + } + + LOG_INFO("Overworld", "ConfigureMultiAreaMap: parent=%d, current_size=%d, new_size=%d, version=%d", + parent_index, static_cast(overworld_maps_[parent_index].area_size()), + static_cast(size), asm_version); + + // CRITICAL: First, get OLD siblings (before changing) so we can reset them + std::vector old_siblings; + auto old_size = overworld_maps_[parent_index].area_size(); + int old_parent = overworld_maps_[parent_index].parent(); + + switch (old_size) { + case AreaSizeEnum::LargeArea: + old_siblings = {old_parent, old_parent + 1, old_parent + 8, old_parent + 9}; + break; + case AreaSizeEnum::WideArea: + old_siblings = {old_parent, old_parent + 1}; + break; + case AreaSizeEnum::TallArea: + old_siblings = {old_parent, old_parent + 8}; + break; + default: + old_siblings = {parent_index}; // Was small, just this map + break; + } + + // Reset all old siblings to SmallArea first (clean slate) + for (int old_sibling : old_siblings) { + if (old_sibling >= 0 && old_sibling < kNumOverworldMaps) { + overworld_maps_[old_sibling].SetAsSmallMap(old_sibling); + } + } + + // Now configure NEW siblings based on requested size + std::vector new_siblings; + + switch (size) { + case AreaSizeEnum::SmallArea: + // Just configure this single map as small + overworld_maps_[parent_index].SetParent(parent_index); + overworld_maps_[parent_index].SetAreaSize(AreaSizeEnum::SmallArea); + new_siblings = {parent_index}; + break; + + case AreaSizeEnum::LargeArea: + new_siblings = {parent_index, parent_index + 1, parent_index + 8, parent_index + 9}; + for (size_t i = 0; i < new_siblings.size(); ++i) { + int sibling = new_siblings[i]; + if (sibling < 0 || sibling >= kNumOverworldMaps) continue; + overworld_maps_[sibling].SetAsLargeMap(parent_index, i); + } + break; + + case AreaSizeEnum::WideArea: + new_siblings = {parent_index, parent_index + 1}; + for (int sibling : new_siblings) { + if (sibling < 0 || sibling >= kNumOverworldMaps) continue; + overworld_maps_[sibling].SetParent(parent_index); + overworld_maps_[sibling].SetAreaSize(AreaSizeEnum::WideArea); + } + break; + + case AreaSizeEnum::TallArea: + new_siblings = {parent_index, parent_index + 8}; + for (int sibling : new_siblings) { + if (sibling < 0 || sibling >= kNumOverworldMaps) continue; + overworld_maps_[sibling].SetParent(parent_index); + overworld_maps_[sibling].SetAreaSize(AreaSizeEnum::TallArea); + } + break; + } + + // Update ROM data for ALL affected siblings (old + new) + std::set all_affected; + for (int s : old_siblings) all_affected.insert(s); + for (int s : new_siblings) all_affected.insert(s); + + if (asm_version >= 3 && asm_version != 0xFF) { + // v3+: Update expanded tables + for (int sibling : all_affected) { + if (sibling < 0 || sibling >= kNumOverworldMaps) continue; + + RETURN_IF_ERROR(rom()->WriteByte(kOverworldMapParentIdExpanded + sibling, + overworld_maps_[sibling].parent())); + RETURN_IF_ERROR(rom()->WriteByte( + kOverworldScreenSize + sibling, + static_cast(overworld_maps_[sibling].area_size()))); + } + } else if (asm_version < 3 && asm_version != 0xFF) { + // v1/v2: Update basic parent table + for (int sibling : all_affected) { + if (sibling < 0 || sibling >= kNumOverworldMaps) continue; + + RETURN_IF_ERROR(rom()->WriteByte(kOverworldMapParentId + sibling, + overworld_maps_[sibling].parent())); + RETURN_IF_ERROR(rom()->WriteByte( + kOverworldScreenSize + (sibling & 0x3F), + static_cast(overworld_maps_[sibling].area_size()))); + } + } else { + // Vanilla: Update parent and screen size tables + for (int sibling : all_affected) { + if (sibling < 0 || sibling >= kNumOverworldMaps) continue; + + RETURN_IF_ERROR(rom()->WriteByte(kOverworldMapParentId + sibling, + overworld_maps_[sibling].parent())); + RETURN_IF_ERROR(rom()->WriteByte( + kOverworldScreenSize + (sibling & 0x3F), + (overworld_maps_[sibling].area_size() == AreaSizeEnum::LargeArea) ? 0x00 : 0x01)); + } + } + + LOG_INFO("Overworld", "Configured %s area: parent=%d, old_siblings=%zu, new_siblings=%zu", + (size == AreaSizeEnum::LargeArea) ? "Large" : + (size == AreaSizeEnum::WideArea) ? "Wide" : + (size == AreaSizeEnum::TallArea) ? "Tall" : "Small", + parent_index, old_siblings.size(), new_siblings.size()); + + return absl::OkStatus(); +} + absl::StatusOr Overworld::GetTile16ForTile32( int index, int quadrant, int dimension, const uint32_t* map32address) { ASSIGN_OR_RETURN( diff --git a/src/app/zelda3/overworld/overworld.h b/src/app/zelda3/overworld/overworld.h index e3048d80..462e5aef 100644 --- a/src/app/zelda3/overworld/overworld.h +++ b/src/app/zelda3/overworld/overworld.h @@ -200,6 +200,16 @@ class Overworld { absl::Status SaveMusic(); absl::Status SaveAreaSizes(); void AssignMapSizes(std::vector& maps); + + /** + * @brief Configure a multi-area map structure (Large/Wide/Tall) + * @param parent_index The parent map index + * @param size The area size to configure + * @return Status of the configuration + * + * Properly sets up sibling relationships and updates ROM data for v3+. + */ + absl::Status ConfigureMultiAreaMap(int parent_index, AreaSizeEnum size); auto rom() const { return rom_; } auto mutable_rom() { return rom_; } diff --git a/src/app/zelda3/overworld/overworld_map.h b/src/app/zelda3/overworld/overworld_map.h index 144b4fbb..e2a4f4da 100644 --- a/src/app/zelda3/overworld/overworld_map.h +++ b/src/app/zelda3/overworld/overworld_map.h @@ -208,6 +208,10 @@ class OverworldMap : public gfx::GfxContext { area_size_ = size; large_map_ = (size == AreaSizeEnum::LargeArea); } + + void SetParent(int parent_index) { + parent_ = parent_index; + } void Destroy() { current_blockset_.clear();