Refactor DungeonEditor to streamline room loading and size calculation; replace individual room loading methods with a unified LoadRoomFromRom function, and update room size management to utilize a dedicated RoomSize struct for improved clarity and efficiency.

src/app/editor/dungeon/dungeon_editor.cc

@@ -47,15 +47,17 @@ absl::Status DungeonEditor::Load() {
 
   for (int i = 0; i < 0x100 + 40; i++) {
     rooms_.emplace_back(zelda3::Room(/*room_id=*/i));
-    rooms_[i].LoadHeader();
+    rooms_[i] = zelda3::LoadRoomFromRom(rom(), i);
-    rooms_[i].LoadRoomFromROM();
+
-    if (core::FeatureFlags::get().kDrawDungeonRoomGraphics) {
+    auto room_size = zelda3::CalculateRoomSize(rom(), i);
-      rooms_[i].LoadRoomGraphics();
+    room_size_pointers_.push_back(room_size.room_size_pointer);
+    room_sizes_.push_back(room_size.room_size);
+    if (room_size.room_size_pointer != 0x0A8000) {
+      room_size_addresses_[i] = room_size.room_size_pointer;
     }
 
-    room_size_pointers_.push_back(rooms_[i].room_size_ptr());
+    if (core::FeatureFlags::get().kDrawDungeonRoomGraphics) {
-    if (rooms_[i].room_size_ptr() != 0x0A8000) {
+      rooms_[i].LoadRoomGraphics();
-      room_size_addresses_[i] = rooms_[i].room_size_ptr();
     }
 
     auto dungeon_palette_ptr = rom()->paletteset_ids[rooms_[i].palette][0];
@@ -100,14 +102,16 @@ absl::Status DungeonEditor::Update() {
   }
 
   if (ImGui::BeginTabBar("##DungeonEditorTabBar")) {
-    TAB_ITEM("Room Editor")
+    if (ImGui::BeginTabItem("Room Editor")) {
-    status_ = UpdateDungeonRoomView();
+      status_ = UpdateDungeonRoomView();
-    END_TAB_ITEM()
+      ImGui::EndTabItem();
-    TAB_ITEM("Usage Statistics")
+    }
-    if (is_loaded_) {
+    if (ImGui::BeginTabItem("Usage Statistics")) {
-      DrawUsageStats();
+      if (is_loaded_) {
+        DrawUsageStats();
+      }
+      ImGui::EndTabItem();
     }
-    END_TAB_ITEM()
     ImGui::EndTabBar();
   }
 
@@ -161,16 +165,16 @@ void DungeonEditor::LoadDungeonRoomSize() {
         if (i < bank_rooms.second.size() - 1) {
           // Calculate size as difference between current room and next room
           // in the same bank
-          rooms_[room_id].set_room_size(bank_rooms.second[i + 1] - room_ptr);
+          room_sizes_[room_id] = bank_rooms.second[i + 1] - room_ptr;
         } else {
           // Calculate size for the last room in this bank
           int bank_end_address = (bank_rooms.first << 16) | 0xFFFF;
-          rooms_[room_id].set_room_size(bank_end_address - room_ptr + 1);
+          room_sizes_[room_id] = bank_end_address - room_ptr + 1;
         }
-        total_room_size_ += rooms_[room_id].room_size();
+        total_room_size_ += room_sizes_[room_id];
       } else {
         // Room with address 0x0A8000
-        rooms_[room_id].set_room_size(0x00);
+        room_sizes_[room_id] = 0x00;
       }
     }
   }
@@ -762,10 +766,10 @@ void DungeonEditor::DrawUsageGrid() {
       color.y = color.y / 255;
       color.z = color.z / 255;
       color.w = 1.0f;
-      if (rooms_[row * squaresWide + col].room_size() > 0xFFFF) {
+      if (room_sizes_[row * squaresWide + col] > 0xFFFF) {
         color = ImVec4(1.0f, 0.0f, 0.0f, 1.0f);  // Or any highlight color
       }
-      if (rooms_[row * squaresWide + col].room_size() == 0) {
+      if (room_sizes_[row * squaresWide + col] == 0) {
         color = ImVec4(0.0f, 0.0f, 0.0f, 1.0f);  // Or any highlight color
       }
       ImGui::PushStyleColor(ImGuiCol_Button, color);
@@ -782,8 +786,7 @@ void DungeonEditor::DrawUsageGrid() {
             ImGuiCol_Button,
             ImVec4(1.0f, 0.5f, 0.0f, 1.0f));  // Or any highlight color
       }
-      if (Button(absl::StrFormat("%#x",
+      if (Button(absl::StrFormat("%#x", room_sizes_[row * squaresWide + col])
-                                 rooms_[row * squaresWide + col].room_size())
                      .c_str(),
                  ImVec2(55, 30))) {
         // Switch over to the room editor tab
@@ -814,8 +817,9 @@ void DungeonEditor::DrawUsageGrid() {
         Text("Floor1: %#02x", room.floor1);
         Text("Floor2: %#02x", room.floor2);
         Text("Message ID: %#04x", room.message_id_);
-        Text("Size: %#016llx", room.room_size());
+        Text("Size: %#016llx", room_sizes_[row * squaresWide + col]);
-        Text("Size Pointer: %#016llx", room.room_size_ptr());
+        Text("Size Pointer: %#016llx",
+             room_size_pointers_[row * squaresWide + col]);
         ImGui::EndTooltip();
       }
 

src/app/editor/dungeon/dungeon_editor.h

@@ -135,6 +135,7 @@ class DungeonEditor : public Editor {
   absl::flat_hash_map<uint16_t, int> palette_usage_;
 
   std::vector<int64_t> room_size_pointers_;
+  std::vector<int64_t> room_sizes_;
 
   uint16_t selected_blockset_ = 0xFFFF;  // 0xFFFF indicates no selection
   uint16_t selected_spriteset_ = 0xFFFF;

src/app/zelda3/dungeon/room.cc

@@ -10,53 +10,11 @@
 #include "app/zelda3/dungeon/room_object.h"
 #include "app/zelda3/sprite/sprite.h"
 #include "util/log.h"
-#include "util/macro.h"
 
 namespace yaze {
 namespace zelda3 {
 
-void Room::LoadHeader() {
+RoomSize CalculateRoomSize(Rom *rom, int room_id) {
-  int header_pointer = (rom()->data()[kRoomHeaderPointer + 2] << 16) +
-                       (rom()->data()[kRoomHeaderPointer + 1] << 8) +
-                       (rom()->data()[kRoomHeaderPointer]);
-  header_pointer = SnesToPc(header_pointer);
-
-  int address = (rom()->data()[kRoomHeaderPointerBank] << 16) +
-                (rom()->data()[(header_pointer + 1) + (room_id_ * 2)] << 8) +
-                rom()->data()[(header_pointer) + (room_id_ * 2)];
-
-  auto header_location = SnesToPc(address);
-
-  bg2_ = (background2)((rom()->data()[header_location] >> 5) & 0x07);
-  collision_ = (CollisionKey)((rom()->data()[header_location] >> 2) & 0x07);
-  is_light_ = ((rom()->data()[header_location]) & 0x01) == 1;
-
-  if (is_light_) {
-    bg2_ = background2::DarkRoom;
-  }
-
-  palette = ((rom()->data()[header_location + 1] & 0x3F));
-  blockset = (rom()->data()[header_location + 2]);
-  spriteset = (rom()->data()[header_location + 3]);
-  effect_ = (EffectKey)((rom()->data()[header_location + 4]));
-  tag1_ = (TagKey)((rom()->data()[header_location + 5]));
-  tag2_ = (TagKey)((rom()->data()[header_location + 6]));
-
-  staircase_plane_[0] = ((rom()->data()[header_location + 7] >> 2) & 0x03);
-  staircase_plane_[1] = ((rom()->data()[header_location + 7] >> 4) & 0x03);
-  staircase_plane_[2] = ((rom()->data()[header_location + 7] >> 6) & 0x03);
-  staircase_plane_[3] = ((rom()->data()[header_location + 8]) & 0x03);
-
-  holewarp = (rom()->data()[header_location + 9]);
-  staircase_rooms_[0] = (rom()->data()[header_location + 10]);
-  staircase_rooms_[1] = (rom()->data()[header_location + 11]);
-  staircase_rooms_[2] = (rom()->data()[header_location + 12]);
-  staircase_rooms_[3] = (rom()->data()[header_location + 13]);
-
-  CalculateRoomSize();
-}
-
-void Room::CalculateRoomSize() {
   // Calculate the size of the room based on how many objects are used per room
   // Some notes from hacker Zarby89
   // vanilla rooms are using in average ~0x80 bytes
@@ -68,120 +26,165 @@ void Room::CalculateRoomSize() {
   // So we want to search the rom() object at this addressed based on the room
   // ID since it's the roomid * 3 we will by pulling 3 bytes at a time We can do
   // this with the rom()->ReadByteVector(addr, size)
-  try {
+  // Existing room size address calculation...
-    // Existing room size address calculation...
+  RoomSize room_size;
-    auto room_size_address = 0xF8000 + (room_id_ * 3);
+  room_size.room_size_pointer = 0;
-    util::logf("Room #%#03X Addresss: %#06X", room_id_, room_size_address);
+  room_size.room_size = 0;
+
+  auto room_size_address = 0xF8000 + (room_id * 3);
+  util::logf("Room #%#03X Addresss: %#06X", room_id, room_size_address);
+
+  // Reading bytes for long address construction
+  uint8_t low = rom->data()[room_size_address];
+  uint8_t high = rom->data()[room_size_address + 1];
+  uint8_t bank = rom->data()[room_size_address + 2];
+
+  // Constructing the long address
+  int long_address = (bank << 16) | (high << 8) | low;
+  util::logf("%#06X", long_address);
+  room_size.room_size_pointer = long_address;
+
+  if (long_address == 0x0A8000) {
+    // Blank room disregard in size calculation
+    util::logf("Size of Room #%#03X: 0 bytes", room_id);
+    room_size.room_size = 0;
+  } else {
+    // use the long address to calculate the size of the room
+    // we will use the room_id_ to calculate the next room's address
+    // and subtract the two to get the size of the room
+
+    int next_room_address = 0xF8000 + ((room_id + 1) * 3);
+    util::logf("Next Room Address: %#06X", next_room_address);
 
     // Reading bytes for long address construction
-    uint8_t low = rom()->data()[room_size_address];
+    uint8_t next_low = rom->data()[next_room_address];
-    uint8_t high = rom()->data()[room_size_address + 1];
+    uint8_t next_high = rom->data()[next_room_address + 1];
-    uint8_t bank = rom()->data()[room_size_address + 2];
+    uint8_t next_bank = rom->data()[next_room_address + 2];
 
     // Constructing the long address
-    int long_address = (bank << 16) | (high << 8) | low;
+    int next_long_address = (next_bank << 16) | (next_high << 8) | next_low;
-    util::logf("%#06X", long_address);
+    util::logf("%#06X", next_long_address);
-    room_size_pointer_ = long_address;
 
-    if (long_address == 0x0A8000) {
+    // Calculate the size of the room
-      // Blank room disregard in size calculation
+    int actual_room_size = next_long_address - long_address;
-      util::logf("Size of Room #%#03X: 0 bytes", room_id_);
+    room_size.room_size = actual_room_size;
-      room_size_ = 0;
+    util::logf("Size of Room #%#03X: %d bytes", room_id, actual_room_size);
-    } else {
-      // use the long address to calculate the size of the room
-      // we will use the room_id_ to calculate the next room's address
-      // and subtract the two to get the size of the room
-
-      int next_room_address = 0xF8000 + ((room_id_ + 1) * 3);
-      util::logf("Next Room Address: %#06X", next_room_address);
-
-      // Reading bytes for long address construction
-      uint8_t next_low = rom()->data()[next_room_address];
-      uint8_t next_high = rom()->data()[next_room_address + 1];
-      uint8_t next_bank = rom()->data()[next_room_address + 2];
-
-      // Constructing the long address
-      int next_long_address = (next_bank << 16) | (next_high << 8) | next_low;
-      util::logf("%#06X", next_long_address);
-
-      // Calculate the size of the room
-      int room_size = next_long_address - long_address;
-      room_size_ = room_size;
-      util::logf("Size of Room #%#03X: %d bytes", room_id_, room_size_);
-    }
-  } catch (const std::exception &e) {
-    std::cout << "Error: " << e.what() << std::endl;
   }
+
+  return room_size;
 }
 
-void Room::LoadRoomFromROM() {
+Room LoadRoomFromRom(Rom *rom, int room_id) {
-  auto rom_data = rom()->vector();
+  Room room(room_id);
-  int header_pointer = SnesToPc(kRoomHeaderPointer);
 
-  message_id_ = messages_id_dungeon + (room_id_ * 2);
+  int header_pointer = (rom->data()[kRoomHeaderPointer + 2] << 16) +
+                       (rom->data()[kRoomHeaderPointer + 1] << 8) +
+                       (rom->data()[kRoomHeaderPointer]);
+  header_pointer = SnesToPc(header_pointer);
 
-  int address = (rom()->data()[kRoomHeaderPointerBank] << 16) +
+  int address = (rom->data()[kRoomHeaderPointerBank] << 16) +
-                (rom()->data()[(header_pointer + 1) + (room_id_ * 2)] << 8) +
+                (rom->data()[(header_pointer + 1) + (room_id * 2)] << 8) +
-                rom()->data()[(header_pointer) + (room_id_ * 2)];
+                rom->data()[(header_pointer) + (room_id * 2)];
 
-  int hpos = SnesToPc(address);
+  auto header_location = SnesToPc(address);
+
+  room.bg2_ = (background2)((rom->data()[header_location] >> 5) & 0x07);
+  room.collision_ = (CollisionKey)((rom->data()[header_location] >> 2) & 0x07);
+  room.is_light_ = ((rom->data()[header_location]) & 0x01) == 1;
+
+  if (room.is_light_) {
+    room.bg2_ = background2::DarkRoom;
+  }
+
+  room.palette = ((rom->data()[header_location + 1] & 0x3F));
+  room.blockset = (rom->data()[header_location + 2]);
+  room.spriteset = (rom->data()[header_location + 3]);
+  room.effect_ = (EffectKey)((rom->data()[header_location + 4]));
+  room.tag1_ = (TagKey)((rom->data()[header_location + 5]));
+  room.tag2_ = (TagKey)((rom->data()[header_location + 6]));
+
+  room.staircase_plane_[0] = ((rom->data()[header_location + 7] >> 2) & 0x03);
+  room.staircase_plane_[1] = ((rom->data()[header_location + 7] >> 4) & 0x03);
+  room.staircase_plane_[2] = ((rom->data()[header_location + 7] >> 6) & 0x03);
+  room.staircase_plane_[3] = ((rom->data()[header_location + 8]) & 0x03);
+
+  room.holewarp = (rom->data()[header_location + 9]);
+  room.staircase_rooms_[0] = (rom->data()[header_location + 10]);
+  room.staircase_rooms_[1] = (rom->data()[header_location + 11]);
+  room.staircase_rooms_[2] = (rom->data()[header_location + 12]);
+  room.staircase_rooms_[3] = (rom->data()[header_location + 13]);
+
+  // =====
+
+  int header_pointer_2 = (rom->data()[kRoomHeaderPointer + 2] << 16) +
+                         (rom->data()[kRoomHeaderPointer + 1] << 8) +
+                         (rom->data()[kRoomHeaderPointer]);
+  header_pointer_2 = SnesToPc(header_pointer_2);
+
+  int address_2 = (rom->data()[kRoomHeaderPointerBank] << 16) +
+                  (rom->data()[(header_pointer_2 + 1) + (room_id * 2)] << 8) +
+                  rom->data()[(header_pointer_2) + (room_id * 2)];
+
+  room.message_id_ = messages_id_dungeon + (room_id * 2);
+
+  auto hpos = SnesToPc(address_2);
   hpos++;
-  uint8_t b = rom_data[hpos];
+  uint8_t b = rom->data()[hpos];
 
-  layer2_mode_ = (b >> 5);
+  room.layer2_mode_ = (b >> 5);
-  layer_merging_ = kLayerMergeTypeList[(b & 0x0C) >> 2];
+  room.layer_merging_ = kLayerMergeTypeList[(b & 0x0C) >> 2];
 
-  is_dark_ = (b & 0x01) == 0x01;
+  room.is_dark_ = (b & 0x01) == 0x01;
+  hpos++;
+  room.palette_ = rom->data()[hpos];
   hpos++;
 
-  palette_ = rom_data[hpos];
+  room.background_tileset_ = rom->data()[hpos];
   hpos++;
 
-  background_tileset_ = rom_data[hpos];
+  room.sprite_tileset_ = rom->data()[hpos];
   hpos++;
 
-  sprite_tileset_ = rom_data[hpos];
+  room.layer2_behavior_ = rom->data()[hpos];
   hpos++;
 
-  layer2_behavior_ = rom_data[hpos];
+  room.tag1_ = (TagKey)rom->data()[hpos];
   hpos++;
 
-  tag1_ = (TagKey)rom_data[hpos];
+  room.tag2_ = (TagKey)rom->data()[hpos];
   hpos++;
 
-  tag2_ = (TagKey)rom_data[hpos];
+  b = rom->data()[hpos];
+
+  room.pits_.target_layer = (uint8_t)(b & 0x03);
+  room.stair1_.target_layer = (uint8_t)((b >> 2) & 0x03);
+  room.stair2_.target_layer = (uint8_t)((b >> 4) & 0x03);
+  room.stair3_.target_layer = (uint8_t)((b >> 6) & 0x03);
+  hpos++;
+  room.stair4_.target_layer = (uint8_t)(rom->data()[hpos] & 0x03);
   hpos++;
 
-  b = rom_data[hpos];
+  room.pits_.target = rom->data()[hpos];
-
-  pits_.target_layer = (uint8_t)(b & 0x03);
-  stair1_.target_layer = (uint8_t)((b >> 2) & 0x03);
-  stair2_.target_layer = (uint8_t)((b >> 4) & 0x03);
-  stair3_.target_layer = (uint8_t)((b >> 6) & 0x03);
   hpos++;
-  stair4_.target_layer = (uint8_t)(rom_data[hpos] & 0x03);
+  room.stair1_.target = rom->data()[hpos];
   hpos++;
-
+  room.stair2_.target = rom->data()[hpos];
-  pits_.target = rom_data[hpos];
   hpos++;
-  stair1_.target = rom_data[hpos];
+  room.stair3_.target = rom->data()[hpos];
   hpos++;
-  stair2_.target = rom_data[hpos];
+  room.stair4_.target = rom->data()[hpos];
-  hpos++;
-  stair3_.target = rom_data[hpos];
-  hpos++;
-  stair4_.target = rom_data[hpos];
   hpos++;
 
   // Load room objects
   int object_pointer = SnesToPc(room_object_pointer);
-  int room_address = object_pointer + (room_id_ * 3);
+  int room_address = object_pointer + (room_id * 3);
   int objects_location = SnesToPc(room_address);
 
   // Load sprites
-  // int spr_ptr = 0x040000 | rooms_sprite_pointer;
+  int spr_ptr = 0x040000 | rooms_sprite_pointer;
-  // int sprite_address =
+  int sprite_address = SnesToPc(dungeon_spr_ptrs | spr_ptr + (room_id * 2));
-  //     SnesToPc(dungeon_spr_ptrs | spr_ptr + (room_id_ * 2));
+
+  return room;
 }
 
 void Room::LoadRoomGraphics(uint8_t entrance_blockset) {
@@ -194,10 +197,9 @@ void Room::LoadRoomGraphics(uint8_t entrance_blockset) {
     blocks_[i] = main_gfx[blockset][i];
     if (i >= 6 && i <= 6) {
       // 3-6
-      if (entrance_blockset != 0xFF) {
+      if (entrance_blockset != 0xFF &&
-        if (room_gfx[entrance_blockset][i - 3] != 0) {
+          room_gfx[entrance_blockset][i - 3] != 0) {
-          blocks_[i] = room_gfx[entrance_blockset][i - 3];
+        blocks_[i] = room_gfx[entrance_blockset][i - 3];
-        }
       }
     }
   }

src/app/zelda3/dungeon/room.h

@@ -11,7 +11,6 @@
 #include "app/rom.h"
 #include "app/zelda3/dungeon/room_object.h"
 #include "app/zelda3/sprite/sprite.h"
-#include "util/macro.h"
 
 namespace yaze {
 namespace zelda3 {
@@ -196,24 +195,9 @@ enum TagKey {
   Kill_boss_Again
 };
 
-static const std::string RoomEffect[] = {"Nothing",
-                                         "Nothing",
-                                         "Moving Floor",
-                                         "Moving Water",
-                                         "Trinexx Shell",
-                                         "Red Flashes",
-                                         "Light Torch to See Floor",
-                                         "Ganon's Darkness"};
-
 class Room : public SharedRom {
  public:
-  Room() = default;
   Room(int room_id) : room_id_(room_id) {}
-  ~Room() = default;
-
-  void LoadHeader();
-  void CalculateRoomSize();
-  void LoadRoomFromROM();
 
   void LoadRoomGraphics(uint8_t entrance_blockset = 0xFF);
   void CopyRoomGraphicsToBuffer();
@@ -228,9 +212,6 @@ class Room : public SharedRom {
   auto &layer1() { return background_bmps_[0]; }
   auto &layer2() { return background_bmps_[1]; }
   auto &layer3() { return background_bmps_[2]; }
-  auto room_size() const { return room_size_; }
-  auto room_size_ptr() const { return room_size_pointer_; }
-  auto set_room_size(uint64_t size) { room_size_ = size; }
 
   uint8_t blockset = 0;
   uint8_t spriteset = 0;
@@ -247,7 +228,6 @@ class Room : public SharedRom {
   std::vector<uint8_t> bg2_buffer_;
   std::vector<uint8_t> current_gfx16_;
 
- private:
   bool is_light_;
   bool is_loaded_;
   bool is_dark_;
@@ -267,9 +247,6 @@ class Room : public SharedRom {
   uint8_t floor2_graphics_;
   uint8_t layer2_mode_;
 
-  uint64_t room_size_;
-  int64_t room_size_pointer_;
-
   std::array<uint8_t, 16> blocks_;
   std::array<chest, 16> chest_list_;
 
@@ -293,6 +270,26 @@ class Room : public SharedRom {
   destination stair4_;
 };
 
+// Loads a room from the ROM.
+Room LoadRoomFromRom(Rom *rom, int room_id);
+
+struct RoomSize {
+  int64_t room_size_pointer;
+  int64_t room_size;
+};
+
+// Calculates the size of a room in the ROM.
+RoomSize CalculateRoomSize(Rom *rom, int room_id);
+
+static const std::string RoomEffect[] = {"Nothing",
+                                         "Nothing",
+                                         "Moving Floor",
+                                         "Moving Water",
+                                         "Trinexx Shell",
+                                         "Red Flashes",
+                                         "Light Torch to See Floor",
+                                         "Ganon's Darkness"};
+
 constexpr std::string_view kRoomNames[] = {
     "Ganon",
     "Hyrule Castle (North Corridor)",