From dd56addd5e31147316ad1ffcf01f2c784e2ca835 Mon Sep 17 00:00:00 2001 From: scawful Date: Mon, 6 Oct 2025 00:33:10 -0400 Subject: [PATCH] refactor: Improve Editor Management and UI Consistency - Removed PushID and PopID calls in EditorManager to prevent ID stack corruption, relying on window titles for uniqueness. - Updated ImGui window size and position settings to use FirstUseEver for maximizing on first open, enhancing user experience. - Replaced AgentUI::PopPanelStyle with ImGui::PopStyleColor in multiple locations for consistency in style management. - Ensured all EditorCard instances consistently call End after Begin, improving code clarity and preventing potential rendering issues. --- src/app/editor/agent/agent_chat_widget.cc | 18 +- src/app/editor/code/assembly_editor.cc | 4 +- src/app/editor/code/memory_editor.cc | 178 ++++++++++++++++--- src/app/editor/dungeon/dungeon_editor_v2.cc | 2 +- src/app/editor/editor_manager.cc | 15 +- src/app/editor/graphics/graphics_editor.cc | 40 +++-- src/app/editor/graphics/screen_editor.cc | 40 +++-- src/app/editor/overworld/overworld_editor.cc | 92 ++++++---- src/app/editor/sprite/sprite_editor.cc | 16 +- 9 files changed, 287 insertions(+), 118 deletions(-) diff --git a/src/app/editor/agent/agent_chat_widget.cc b/src/app/editor/agent/agent_chat_widget.cc index 83a0273d..d2962009 100644 --- a/src/app/editor/agent/agent_chat_widget.cc +++ b/src/app/editor/agent/agent_chat_widget.cc @@ -555,7 +555,7 @@ void AgentChatWidget::RenderHistory() { } } ImGui::EndChild(); - AgentUI::PopPanelStyle(); + ImGui::PopStyleColor(); // Pop the color we pushed at line 531 last_history_size_ = history.size(); } @@ -1342,8 +1342,8 @@ void AgentChatWidget::RenderCollaborationPanel() { } ImGui::EndChild(); - AgentUI::PopPanelStyle(); - ImGui::PopStyleVar(2); + ImGui::PopStyleColor(); // Pop the ChildBg color from line 1091 + ImGui::PopStyleVar(2); // Pop the 2 StyleVars from lines 1082-1083 ImGui::PopID(); // CollabPanel } @@ -1666,7 +1666,7 @@ void AgentChatWidget::RenderAgentConfigPanel() { } ImGui::EndChild(); - AgentUI::PopPanelStyle(); + ImGui::PopStyleColor(); // Pop the ChildBg color from line 1609 } void AgentChatWidget::RenderZ3EDCommandPanel() { @@ -1749,9 +1749,9 @@ void AgentChatWidget::RenderZ3EDCommandPanel() { } ImGui::EndChild(); - AgentUI::PopPanelStyle(); + ImGui::PopStyleColor(); // Pop the ChildBg color from line 1677 - ImGui::PopID(); // FIX: Pop the Z3EDCmdPanel ID + ImGui::PopID(); // Pop the Z3EDCmdPanel ID } void AgentChatWidget::RenderRomSyncPanel() { @@ -1848,7 +1848,7 @@ void AgentChatWidget::RenderRomSyncPanel() { } ImGui::EndChild(); - AgentUI::PopPanelStyle(); + ImGui::PopStyleColor(); // Pop the ChildBg color from line 1758 } void AgentChatWidget::RenderSnapshotPreviewPanel() { @@ -1898,7 +1898,7 @@ void AgentChatWidget::RenderSnapshotPreviewPanel() { } ImGui::EndChild(); - AgentUI::PopPanelStyle(); + ImGui::PopStyleColor(); // Pop the ChildBg color from line 1860 } void AgentChatWidget::RenderProposalManagerPanel() { @@ -2094,7 +2094,7 @@ void AgentChatWidget::RenderHarnessPanel() { } ImGui::EndChild(); - AgentUI::PopPanelStyle(); + ImGui::PopStyleColor(); // Pop the ChildBg color from line 1982 ImGui::PopID(); } diff --git a/src/app/editor/code/assembly_editor.cc b/src/app/editor/code/assembly_editor.cc index 401a5fc7..63ced8bf 100644 --- a/src/app/editor/code/assembly_editor.cc +++ b/src/app/editor/code/assembly_editor.cc @@ -228,8 +228,8 @@ void AssemblyEditor::UpdateCodeView() { current_folder_ = LoadFolder(FileDialogWrapper::ShowOpenFolderDialog()); } } - file_browser_card.End(); } + file_browser_card.End(); // ALWAYS call End after Begin // Draw open files as individual, dockable EditorCards for (int i = 0; i < active_files_.Size; i++) { @@ -253,8 +253,8 @@ void AssemblyEditor::UpdateCodeView() { active_file_id_ = file_id; } open_files_[file_id].Render(absl::StrCat("##", card_name).c_str()); - file_card.End(); } + file_card.End(); // ALWAYS call End after Begin if (!open) { active_files_.erase(active_files_.Data + i); diff --git a/src/app/editor/code/memory_editor.cc b/src/app/editor/code/memory_editor.cc index f64ab2ba..4361886f 100644 --- a/src/app/editor/code/memory_editor.cc +++ b/src/app/editor/code/memory_editor.cc @@ -1,23 +1,53 @@ #include "app/editor/code/memory_editor.h" + +#include "absl/strings/str_format.h" #include "app/gui/icons.h" +#include "app/gui/style.h" +#include "imgui/imgui.h" namespace yaze { namespace editor { void MemoryEditorWithDiffChecker::DrawToolbar() { + // Modern compact toolbar with icon-only buttons + ImGui::PushStyleVar(ImGuiStyleVar_FramePadding, ImVec2(6, 4)); + ImGui::PushStyleVar(ImGuiStyleVar_ItemSpacing, ImVec2(4, 4)); + if (ImGui::Button(ICON_MD_LOCATION_SEARCHING " Jump")) { ImGui::OpenPopup("JumpToAddress"); } + if (ImGui::IsItemHovered()) { + ImGui::SetTooltip("Jump to specific address"); + } ImGui::SameLine(); if (ImGui::Button(ICON_MD_SEARCH " Search")) { ImGui::OpenPopup("SearchPattern"); } + if (ImGui::IsItemHovered()) { + ImGui::SetTooltip("Search for hex pattern"); + } ImGui::SameLine(); if (ImGui::Button(ICON_MD_BOOKMARK " Bookmarks")) { ImGui::OpenPopup("Bookmarks"); } + if (ImGui::IsItemHovered()) { + ImGui::SetTooltip("Manage address bookmarks"); + } + + ImGui::SameLine(); + ImGui::Text(ICON_MD_MORE_VERT); + ImGui::SameLine(); + + // Show current address + if (current_address_ != 0) { + ImGui::TextColored(ImVec4(0.4f, 0.8f, 1.0f, 1.0f), + ICON_MD_LOCATION_ON " 0x%06X", current_address_); + } + + ImGui::PopStyleVar(2); + ImGui::Separator(); DrawJumpToAddressPopup(); DrawSearchPopup(); @@ -25,17 +55,37 @@ void MemoryEditorWithDiffChecker::DrawToolbar() { } void MemoryEditorWithDiffChecker::DrawJumpToAddressPopup() { - if (ImGui::BeginPopup("JumpToAddress")) { - ImGui::Text(ICON_MD_LOCATION_SEARCHING " Jump to Address"); + if (ImGui::BeginPopupModal("JumpToAddress", nullptr, ImGuiWindowFlags_AlwaysAutoResize)) { + ImGui::TextColored(ImVec4(0.4f, 0.8f, 1.0f, 1.0f), + ICON_MD_LOCATION_SEARCHING " Jump to Address"); ImGui::Separator(); - ImGui::InputText("Address (hex)", jump_address_, IM_ARRAYSIZE(jump_address_)); + ImGui::Spacing(); + + ImGui::SetNextItemWidth(200); + if (ImGui::InputText("##jump_addr", jump_address_, IM_ARRAYSIZE(jump_address_), + ImGuiInputTextFlags_CharsHexadecimal | ImGuiInputTextFlags_EnterReturnsTrue)) { + // Parse and jump on Enter key + unsigned int addr; + if (sscanf(jump_address_, "%X", &addr) == 1) { + current_address_ = addr; + ImGui::CloseCurrentPopup(); + } + } ImGui::TextDisabled("Format: 0x1C800 or 1C800"); - if (ImGui::Button("Go", ImVec2(120, 0))) { - // TODO: Parse address and scroll to it + + ImGui::Spacing(); + ImGui::Separator(); + ImGui::Spacing(); + + if (ImGui::Button(ICON_MD_CHECK " Go", ImVec2(120, 0))) { + unsigned int addr; + if (sscanf(jump_address_, "%X", &addr) == 1) { + current_address_ = addr; + } ImGui::CloseCurrentPopup(); } ImGui::SameLine(); - if (ImGui::Button("Cancel", ImVec2(120, 0))) { + if (ImGui::Button(ICON_MD_CANCEL " Cancel", ImVec2(120, 0))) { ImGui::CloseCurrentPopup(); } ImGui::EndPopup(); @@ -43,17 +93,45 @@ void MemoryEditorWithDiffChecker::DrawJumpToAddressPopup() { } void MemoryEditorWithDiffChecker::DrawSearchPopup() { - if (ImGui::BeginPopup("SearchPattern")) { - ImGui::Text(ICON_MD_SEARCH " Search Hex Pattern"); + if (ImGui::BeginPopupModal("SearchPattern", nullptr, ImGuiWindowFlags_AlwaysAutoResize)) { + ImGui::TextColored(ImVec4(0.4f, 0.8f, 0.4f, 1.0f), + ICON_MD_SEARCH " Search Hex Pattern"); ImGui::Separator(); - ImGui::InputText("Pattern", search_pattern_, IM_ARRAYSIZE(search_pattern_)); + ImGui::Spacing(); + + ImGui::SetNextItemWidth(300); + if (ImGui::InputText("##search_pattern", search_pattern_, IM_ARRAYSIZE(search_pattern_), + ImGuiInputTextFlags_EnterReturnsTrue)) { + // TODO: Implement search + ImGui::CloseCurrentPopup(); + } ImGui::TextDisabled("Use ?? for wildcard (e.g. FF 00 ?? 12)"); - if (ImGui::Button("Search", ImVec2(120, 0))) { + ImGui::Spacing(); + + // Quick preset patterns + ImGui::Text(ICON_MD_LIST " Quick Patterns:"); + if (ImGui::SmallButton("LDA")) { + snprintf(search_pattern_, sizeof(search_pattern_), "A9 ??"); + } + ImGui::SameLine(); + if (ImGui::SmallButton("STA")) { + snprintf(search_pattern_, sizeof(search_pattern_), "8D ?? ??"); + } + ImGui::SameLine(); + if (ImGui::SmallButton("JSR")) { + snprintf(search_pattern_, sizeof(search_pattern_), "20 ?? ??"); + } + + ImGui::Spacing(); + ImGui::Separator(); + ImGui::Spacing(); + + if (ImGui::Button(ICON_MD_SEARCH " Search", ImVec2(120, 0))) { // TODO: Implement search using hex-search handler ImGui::CloseCurrentPopup(); } ImGui::SameLine(); - if (ImGui::Button("Cancel", ImVec2(120, 0))) { + if (ImGui::Button(ICON_MD_CANCEL " Cancel", ImVec2(120, 0))) { ImGui::CloseCurrentPopup(); } ImGui::EndPopup(); @@ -61,27 +139,79 @@ void MemoryEditorWithDiffChecker::DrawSearchPopup() { } void MemoryEditorWithDiffChecker::DrawBookmarksPopup() { - if (ImGui::BeginPopup("Bookmarks")) { - ImGui::Text(ICON_MD_BOOKMARK " Memory Bookmarks"); + if (ImGui::BeginPopupModal("Bookmarks", nullptr, ImGuiWindowFlags_AlwaysAutoResize)) { + ImGui::TextColored(ImVec4(1.0f, 0.843f, 0.0f, 1.0f), + ICON_MD_BOOKMARK " Memory Bookmarks"); ImGui::Separator(); + ImGui::Spacing(); if (bookmarks_.empty()) { - ImGui::TextDisabled("No bookmarks yet"); + ImGui::TextDisabled(ICON_MD_INFO " No bookmarks yet"); + ImGui::Spacing(); ImGui::Separator(); - if (ImGui::Button("Add Current Address")) { - // TODO: Add bookmark at current address + ImGui::Spacing(); + + if (ImGui::Button(ICON_MD_ADD " Add Current Address", ImVec2(250, 0))) { + Bookmark new_bookmark; + new_bookmark.address = current_address_; + new_bookmark.name = absl::StrFormat("Bookmark %zu", bookmarks_.size() + 1); + new_bookmark.description = "User-defined bookmark"; + bookmarks_.push_back(new_bookmark); } } else { - for (size_t i = 0; i < bookmarks_.size(); ++i) { - const auto& bm = bookmarks_[i]; - ImGui::PushID(static_cast(i)); - if (ImGui::Selectable(bm.name.c_str())) { - current_address_ = bm.address; - // TODO: Jump to this address + // Bookmarks table + ImGui::BeginChild("##bookmarks_list", ImVec2(500, 300), true); + if (ImGui::BeginTable("##bookmarks_table", 3, + ImGuiTableFlags_Borders | ImGuiTableFlags_RowBg | + ImGuiTableFlags_Resizable)) { + ImGui::TableSetupColumn("Name", ImGuiTableColumnFlags_WidthFixed, 150); + ImGui::TableSetupColumn("Address", ImGuiTableColumnFlags_WidthFixed, 100); + ImGui::TableSetupColumn("Description", ImGuiTableColumnFlags_WidthStretch); + ImGui::TableHeadersRow(); + + for (size_t i = 0; i < bookmarks_.size(); ++i) { + const auto& bm = bookmarks_[i]; + ImGui::PushID(static_cast(i)); + + ImGui::TableNextRow(); + ImGui::TableNextColumn(); + if (ImGui::Selectable(bm.name.c_str(), false, ImGuiSelectableFlags_SpanAllColumns)) { + current_address_ = bm.address; + ImGui::CloseCurrentPopup(); + } + + ImGui::TableNextColumn(); + ImGui::TextColored(ImVec4(0.4f, 0.8f, 1.0f, 1.0f), "0x%06X", bm.address); + + ImGui::TableNextColumn(); + ImGui::TextDisabled("%s", bm.description.c_str()); + + ImGui::PopID(); } - ImGui::TextDisabled(" 0x%06X - %s", bm.address, bm.description.c_str()); - ImGui::PopID(); + ImGui::EndTable(); } + ImGui::EndChild(); + + ImGui::Spacing(); + if (ImGui::Button(ICON_MD_ADD " Add Bookmark", ImVec2(150, 0))) { + Bookmark new_bookmark; + new_bookmark.address = current_address_; + new_bookmark.name = absl::StrFormat("Bookmark %zu", bookmarks_.size() + 1); + new_bookmark.description = "User-defined bookmark"; + bookmarks_.push_back(new_bookmark); + } + ImGui::SameLine(); + if (ImGui::Button(ICON_MD_CLEAR_ALL " Clear All", ImVec2(150, 0))) { + bookmarks_.clear(); + } + } + + ImGui::Spacing(); + ImGui::Separator(); + ImGui::Spacing(); + + if (ImGui::Button(ICON_MD_CLOSE " Close", ImVec2(250, 0))) { + ImGui::CloseCurrentPopup(); } ImGui::EndPopup(); diff --git a/src/app/editor/dungeon/dungeon_editor_v2.cc b/src/app/editor/dungeon/dungeon_editor_v2.cc index 4251c518..495eca48 100644 --- a/src/app/editor/dungeon/dungeon_editor_v2.cc +++ b/src/app/editor/dungeon/dungeon_editor_v2.cc @@ -140,8 +140,8 @@ void DungeonEditorV2::DrawLayout() { gui::EditorCard room_card(card_name, ICON_MD_GRID_ON, &open); if (room_card.Begin()) { DrawRoomTab(room_id); - room_card.End(); } + room_card.End(); // ALWAYS call End after Begin if (!open) { active_rooms_.erase(active_rooms_.Data + i); diff --git a/src/app/editor/editor_manager.cc b/src/app/editor/editor_manager.cc index e4aff3c7..1d47c5d2 100644 --- a/src/app/editor/editor_manager.cc +++ b/src/app/editor/editor_manager.cc @@ -747,15 +747,12 @@ absl::Status EditorManager::Update() { std::string window_title = GenerateUniqueEditorTitle(editor->type(), session_idx); - // Create truly unique ImGui ID combining session index and editor type - // This ensures ImGui treats them as completely separate windows - ImGui::PushID(static_cast(session_idx * 100 + static_cast(editor->type()))); + // Note: PushID removed - window title provides sufficient uniqueness + // and PushID was causing ID stack corruption issues - // Set window to maximize on first open - if (ImGui::IsWindowAppearing()) { - ImGui::SetNextWindowSize(ImGui::GetMainViewport()->WorkSize, ImGuiCond_Appearing); - ImGui::SetNextWindowPos(ImGui::GetMainViewport()->WorkPos, ImGuiCond_Appearing); - } + // Set window to maximize on first open (use FirstUseEver instead of IsWindowAppearing check) + ImGui::SetNextWindowSize(ImGui::GetMainViewport()->WorkSize, ImGuiCond_FirstUseEver); + ImGui::SetNextWindowPos(ImGui::GetMainViewport()->WorkPos, ImGuiCond_FirstUseEver); if (ImGui::Begin(window_title.c_str(), editor->active(), ImGuiWindowFlags_None)) { // Allow full docking @@ -801,7 +798,7 @@ absl::Status EditorManager::Update() { context_.session_id = prev_session_id; // Restore previous session ID } ImGui::End(); - ImGui::PopID(); // Pop the unique ID for this session+editor combination + // PopID removed to match PushID removal above } } } diff --git a/src/app/editor/graphics/graphics_editor.cc b/src/app/editor/graphics/graphics_editor.cc index 20eba53d..2b774d1a 100644 --- a/src/app/editor/graphics/graphics_editor.cc +++ b/src/app/editor/graphics/graphics_editor.cc @@ -56,27 +56,35 @@ absl::Status GraphicsEditor::Update() { static gui::EditorCard player_anims_card("Player Animations", ICON_MD_PERSON); static gui::EditorCard prototype_card("Prototype Viewer", ICON_MD_CONSTRUCTION); - if (show_sheet_editor_ && sheet_editor_card.Begin(&show_sheet_editor_)) { - status_ = UpdateGfxEdit(); - sheet_editor_card.End(); - } - - if (show_sheet_browser_ && sheet_browser_card.Begin(&show_sheet_browser_)) { - if (asset_browser_.Initialized == false) { - asset_browser_.Initialize(gfx::Arena::Get().gfx_sheets()); + if (show_sheet_editor_) { + if (sheet_editor_card.Begin(&show_sheet_editor_)) { + status_ = UpdateGfxEdit(); } - asset_browser_.Draw(gfx::Arena::Get().gfx_sheets()); - sheet_browser_card.End(); + sheet_editor_card.End(); // ALWAYS call End after Begin } - if (show_player_animations_ && player_anims_card.Begin(&show_player_animations_)) { - status_ = UpdateLinkGfxView(); - player_anims_card.End(); + if (show_sheet_browser_) { + if (sheet_browser_card.Begin(&show_sheet_browser_)) { + if (asset_browser_.Initialized == false) { + asset_browser_.Initialize(gfx::Arena::Get().gfx_sheets()); + } + asset_browser_.Draw(gfx::Arena::Get().gfx_sheets()); + } + sheet_browser_card.End(); // ALWAYS call End after Begin } - if (show_prototype_viewer_ && prototype_card.Begin(&show_prototype_viewer_)) { - status_ = UpdateScadView(); - prototype_card.End(); + if (show_player_animations_) { + if (player_anims_card.Begin(&show_player_animations_)) { + status_ = UpdateLinkGfxView(); + } + player_anims_card.End(); // ALWAYS call End after Begin + } + + if (show_prototype_viewer_) { + if (prototype_card.Begin(&show_prototype_viewer_)) { + status_ = UpdateScadView(); + } + prototype_card.End(); // ALWAYS call End after Begin } CLEAR_AND_RETURN_STATUS(status_) diff --git a/src/app/editor/graphics/screen_editor.cc b/src/app/editor/graphics/screen_editor.cc index 3dcf4bed..51d28dd2 100644 --- a/src/app/editor/graphics/screen_editor.cc +++ b/src/app/editor/graphics/screen_editor.cc @@ -74,25 +74,35 @@ absl::Status ScreenEditor::Update() { static gui::EditorCard title_screen_card("Title Screen", ICON_MD_TITLE); static gui::EditorCard naming_screen_card("Naming Screen", ICON_MD_EDIT_ATTRIBUTES); - if (show_dungeon_maps_ && dungeon_maps_card.Begin(&show_dungeon_maps_)) { - DrawDungeonMapsEditor(); - dungeon_maps_card.End(); + if (show_dungeon_maps_) { + if (dungeon_maps_card.Begin(&show_dungeon_maps_)) { + DrawDungeonMapsEditor(); + } + dungeon_maps_card.End(); // ALWAYS call End after Begin } - if (show_inventory_menu_ && inventory_menu_card.Begin(&show_inventory_menu_)) { - DrawInventoryMenuEditor(); - inventory_menu_card.End(); + if (show_inventory_menu_) { + if (inventory_menu_card.Begin(&show_inventory_menu_)) { + DrawInventoryMenuEditor(); + } + inventory_menu_card.End(); // ALWAYS call End after Begin } - if (show_overworld_map_ && overworld_map_card.Begin(&show_overworld_map_)) { - DrawOverworldMapEditor(); - overworld_map_card.End(); + if (show_overworld_map_) { + if (overworld_map_card.Begin(&show_overworld_map_)) { + DrawOverworldMapEditor(); + } + overworld_map_card.End(); // ALWAYS call End after Begin } - if (show_title_screen_ && title_screen_card.Begin(&show_title_screen_)) { - DrawTitleScreenEditor(); - title_screen_card.End(); + if (show_title_screen_) { + if (title_screen_card.Begin(&show_title_screen_)) { + DrawTitleScreenEditor(); + } + title_screen_card.End(); // ALWAYS call End after Begin } - if (show_naming_screen_ && naming_screen_card.Begin(&show_naming_screen_)) { - DrawNamingScreenEditor(); - naming_screen_card.End(); + if (show_naming_screen_) { + if (naming_screen_card.Begin(&show_naming_screen_)) { + DrawNamingScreenEditor(); + } + naming_screen_card.End(); // ALWAYS call End after Begin } return status_; diff --git a/src/app/editor/overworld/overworld_editor.cc b/src/app/editor/overworld/overworld_editor.cc index bef4e829..c1679f65 100644 --- a/src/app/editor/overworld/overworld_editor.cc +++ b/src/app/editor/overworld/overworld_editor.cc @@ -168,58 +168,72 @@ absl::Status OverworldEditor::Update() { DrawOverworldCanvas(); // Floating tile selector cards (4 tabs converted to separate cards) - if (show_tile16_selector_ && tile16_card.Begin(&show_tile16_selector_)) { - status_ = DrawTile16Selector(); - tile16_card.End(); + if (show_tile16_selector_) { + if (tile16_card.Begin(&show_tile16_selector_)) { + status_ = DrawTile16Selector(); + } + tile16_card.End(); // ALWAYS call End after Begin } - if (show_tile8_selector_ && tile8_card.Begin(&show_tile8_selector_)) { - gui::BeginPadding(3); - gui::BeginChildWithScrollbar("##Tile8SelectorScrollRegion"); - DrawTile8Selector(); - ImGui::EndChild(); - gui::EndNoPadding(); - tile8_card.End(); + if (show_tile8_selector_) { + if (tile8_card.Begin(&show_tile8_selector_)) { + gui::BeginPadding(3); + gui::BeginChildWithScrollbar("##Tile8SelectorScrollRegion"); + DrawTile8Selector(); + ImGui::EndChild(); + gui::EndNoPadding(); + } + tile8_card.End(); // ALWAYS call End after Begin } - if (show_area_gfx_ && area_gfx_card.Begin(&show_area_gfx_)) { - status_ = DrawAreaGraphics(); - area_gfx_card.End(); + if (show_area_gfx_) { + if (area_gfx_card.Begin(&show_area_gfx_)) { + status_ = DrawAreaGraphics(); + } + area_gfx_card.End(); // ALWAYS call End after Begin } - if (show_scratch_ && scratch_card.Begin(&show_scratch_)) { - status_ = DrawScratchSpace(); - scratch_card.End(); + if (show_scratch_) { + if (scratch_card.Begin(&show_scratch_)) { + status_ = DrawScratchSpace(); + } + scratch_card.End(); // ALWAYS call End after Begin } // Tile16 Editor popup-only (no tab) - if (show_tile16_editor_ && tile16_editor_card.Begin(&show_tile16_editor_)) { - if (rom_->is_loaded()) { - status_ = tile16_editor_.Update(); - } else { - gui::CenterText("No ROM loaded"); + if (show_tile16_editor_) { + if (tile16_editor_card.Begin(&show_tile16_editor_)) { + if (rom_->is_loaded()) { + status_ = tile16_editor_.Update(); + } else { + gui::CenterText("No ROM loaded"); + } } - tile16_editor_card.End(); + tile16_editor_card.End(); // ALWAYS call End after Begin } // Graphics Groups popup - if (show_gfx_groups_ && gfx_groups_card.Begin(&show_gfx_groups_)) { - if (rom_->is_loaded()) { - status_ = gfx_group_editor_.Update(); - } else { - gui::CenterText("No ROM loaded"); + if (show_gfx_groups_) { + if (gfx_groups_card.Begin(&show_gfx_groups_)) { + if (rom_->is_loaded()) { + status_ = gfx_group_editor_.Update(); + } else { + gui::CenterText("No ROM loaded"); + } } - gfx_groups_card.End(); + gfx_groups_card.End(); // ALWAYS call End after Begin } // Usage Statistics popup - if (show_usage_stats_ && usage_stats_card.Begin(&show_usage_stats_)) { - if (rom_->is_loaded()) { - status_ = UpdateUsageStats(); - } else { - gui::CenterText("No ROM loaded"); + if (show_usage_stats_) { + if (usage_stats_card.Begin(&show_usage_stats_)) { + if (rom_->is_loaded()) { + status_ = UpdateUsageStats(); + } else { + gui::CenterText("No ROM loaded"); + } } - usage_stats_card.End(); + usage_stats_card.End(); // ALWAYS call End after Begin } // Area Configuration Panel (detailed editing) @@ -1254,7 +1268,6 @@ void OverworldEditor::DrawOverworldCanvas() { gui::BeginNoPadding(); gui::BeginChildBothScrollbars(7); ow_map_canvas_.DrawBackground(); - gui::EndNoPadding(); // Setup dynamic context menu based on current map state (Phase 3B) if (rom_->is_loaded() && overworld_.is_loaded() && map_properties_system_) { @@ -1314,6 +1327,7 @@ void OverworldEditor::DrawOverworldCanvas() { ow_map_canvas_.DrawGrid(); ow_map_canvas_.DrawOverlay(); EndChild(); + gui::EndNoPadding(); // End the no-padding style that was started at line 1254 } absl::Status OverworldEditor::DrawTile16Selector() { @@ -1344,7 +1358,13 @@ absl::Status OverworldEditor::DrawTile16Selector() { if (result.selection_changed) { current_tile16_ = result.selected_tile; - RETURN_IF_ERROR(tile16_editor_.SetCurrentTile(current_tile16_)); + auto status = tile16_editor_.SetCurrentTile(current_tile16_); + if (!status.ok()) { + // Store error but ensure we close the child before returning + EndChild(); + ImGui::EndGroup(); + return status; + } // Note: We do NOT auto-scroll here because it breaks user interaction. // The canvas should only scroll when explicitly requested (e.g., when // selecting a tile from the overworld canvas via ScrollBlocksetCanvasToCurrentTile). diff --git a/src/app/editor/sprite/sprite_editor.cc b/src/app/editor/sprite/sprite_editor.cc index d2314289..0a83c5ac 100644 --- a/src/app/editor/sprite/sprite_editor.cc +++ b/src/app/editor/sprite/sprite_editor.cc @@ -43,14 +43,18 @@ absl::Status SpriteEditor::Update() { static gui::EditorCard vanilla_card("Vanilla Sprites", ICON_MD_PEST_CONTROL_RODENT); static gui::EditorCard custom_card("Custom Sprites", ICON_MD_ADD_MODERATOR); - if (show_vanilla_editor_ && vanilla_card.Begin(&show_vanilla_editor_)) { - DrawVanillaSpriteEditor(); - vanilla_card.End(); + if (show_vanilla_editor_) { + if (vanilla_card.Begin(&show_vanilla_editor_)) { + DrawVanillaSpriteEditor(); + } + vanilla_card.End(); // ALWAYS call End after Begin } - if (show_custom_editor_ && custom_card.Begin(&show_custom_editor_)) { - DrawCustomSprites(); - custom_card.End(); + if (show_custom_editor_) { + if (custom_card.Begin(&show_custom_editor_)) { + DrawCustomSprites(); + } + custom_card.End(); // ALWAYS call End after Begin } return status_.ok() ? absl::OkStatus() : status_;