refactor overworld items to flat functions

src/zelda3/common.h

@@ -44,6 +44,12 @@ class GameEntity {
   virtual void UpdateMapProperties(uint16_t map_id) = 0;
 };
 
+constexpr int kNumOverworldMaps = 160;
+
+// 1 byte, not 0 if enabled
+// vanilla, v2, v3
+constexpr int OverworldCustomASMHasBeenApplied = 0x140145;
+
 constexpr const char* kEntranceNames[] = {
     "Link's House Intro",
     "Link's House Post-intro",

src/zelda3/overworld/overworld.cc

@@ -16,6 +16,7 @@
 #include "app/gfx/types/snes_tile.h"
 #include "app/rom.h"
 #include "app/snes.h"
+#include "zelda3/common.h"
 #include "zelda3/overworld/overworld_entrance.h"
 #include "zelda3/overworld/overworld_exit.h"
 #include "util/hex.h"
@@ -94,7 +95,7 @@ absl::Status Overworld::Load(Rom* rom) {
     
     {
       gfx::ScopedTimer items_timer("LoadItems");
-      RETURN_IF_ERROR(LoadItems());
+      ASSIGN_OR_RETURN(all_items_, LoadItems(rom_, overworld_maps_));
     }
     
     {
@@ -687,73 +688,6 @@ void Overworld::LoadTileTypes() {
   }
 }
 
-absl::Status Overworld::LoadItems() {
-  // Version 0x03 of the OW ASM added item support for the SW.
-  uint8_t asm_version = (*rom_)[zelda3::OverworldCustomASMHasBeenApplied];
-
-  // Determine max number of overworld maps based on actual ASM version
-  // Only use expanded maps (0xA0) if v3+ ASM is actually applied
-  int max_ow =
-      (asm_version >= 0x03 && asm_version != 0xFF) ? kNumOverworldMaps : 0x80;
-
-  ASSIGN_OR_RETURN(uint32_t pointer_snes,
-                   rom()->ReadLong(zelda3::overworldItemsAddress));
-  uint32_t item_pointer_address =
-      SnesToPc(pointer_snes);  // 0x1BC2F9 -> 0x0DC2F9
-
-  for (int i = 0; i < max_ow; i++) {
-    ASSIGN_OR_RETURN(uint8_t bank_byte,
-                     rom()->ReadByte(zelda3::overworldItemsAddressBank));
-    int bank = bank_byte & 0x7F;
-
-    ASSIGN_OR_RETURN(uint8_t addr_low,
-                     rom()->ReadByte(item_pointer_address + (i * 2)));
-    ASSIGN_OR_RETURN(uint8_t addr_high,
-                     rom()->ReadByte(item_pointer_address + (i * 2) + 1));
-
-    uint32_t addr = (bank << 16) +      // 1B
-                    (addr_high << 8) +  // F9
-                    addr_low;           // 3C
-    addr = SnesToPc(addr);
-
-    // Check if this is a large map and skip if not the parent
-    if (overworld_maps_[i].area_size() != zelda3::AreaSizeEnum::SmallArea) {
-      if (overworld_maps_[i].parent() != (uint8_t)i) {
-        continue;
-      }
-    }
-
-    while (true) {
-      ASSIGN_OR_RETURN(uint8_t b1, rom()->ReadByte(addr));
-      ASSIGN_OR_RETURN(uint8_t b2, rom()->ReadByte(addr + 1));
-      ASSIGN_OR_RETURN(uint8_t b3, rom()->ReadByte(addr + 2));
-
-      if (b1 == 0xFF && b2 == 0xFF) {
-        break;
-      }
-
-      int p = (((b2 & 0x1F) << 8) + b1) >> 1;
-
-      int x = p % 0x40;  // Use 0x40 instead of 64 to match ZS
-      int y = p >> 6;
-
-      int fakeID = i % 0x40;  // Use modulo 0x40 to match ZS
-
-      int sy = fakeID / 8;
-      int sx = fakeID - (sy * 8);
-
-      all_items_.emplace_back(b3, (uint16_t)i, (x * 16) + (sx * 512),
-                              (y * 16) + (sy * 512), false);
-      auto size = all_items_.size();
-
-      all_items_[size - 1].game_x_ = (uint8_t)x;
-      all_items_[size - 1].game_y_ = (uint8_t)y;
-      addr += 3;
-    }
-  }
-  return absl::OkStatus();
-}
-
 absl::Status Overworld::LoadSprites() {
   std::vector<std::future<absl::Status>> futures;
 
@@ -2479,149 +2413,8 @@ absl::Status Overworld::SaveExits() {
   return absl::OkStatus();
 }
 
-namespace {
-bool CompareItemsArrays(std::vector<OverworldItem> item_array1,
-                        std::vector<OverworldItem> item_array2) {
-  if (item_array1.size() != item_array2.size()) {
-    return false;
-  }
-
-  bool match;
-  for (size_t i = 0; i < item_array1.size(); i++) {
-    match = false;
-    for (size_t j = 0; j < item_array2.size(); j++) {
-      // Check all sprite in 2nd array if one match
-      if (item_array1[i].x_ == item_array2[j].x_ &&
-          item_array1[i].y_ == item_array2[j].y_ &&
-          item_array1[i].id_ == item_array2[j].id_) {
-        match = true;
-        break;
-      }
-    }
-
-    if (!match) {
-      return false;
-    }
-  }
-
-  return true;
-}
-}  // namespace
-
 absl::Status Overworld::SaveItems() {
-  const int pointer_count = zelda3::kNumOverworldMaps;
+  RETURN_IF_ERROR(::yaze::zelda3::SaveItems(rom_, all_items_));
-
-  std::vector<std::vector<OverworldItem>> room_items(pointer_count);
-
-  // Reset bomb door lookup table used by special item (0x86)
-  for (int i = 0; i < zelda3::kNumOverworldMaps; ++i) {
-    RETURN_IF_ERROR(rom()->WriteShort(zelda3::kOverworldBombDoorItemLocationsNew +
-                                         (i * 2),
-                                     0x0000));
-  }
-
-  for (const OverworldItem& item : all_items_) {
-    if (item.deleted) continue;
-
-    const int map_index = static_cast<int>(item.room_map_id_);
-    if (map_index < 0 || map_index >= pointer_count) {
-      LOG_WARN("Overworld::SaveItems",
-               "Skipping item with map index %d outside pointer table (size=%d)",
-               map_index, pointer_count);
-      continue;
-    }
-
-    room_items[map_index].push_back(item);
-
-    if (item.id_ == 0x86) {
-      const int lookup_index = std::min(map_index, zelda3::kNumOverworldMaps - 1);
-      RETURN_IF_ERROR(rom()->WriteShort(
-          zelda3::kOverworldBombDoorItemLocationsNew + (lookup_index * 2),
-          static_cast<uint16_t>((item.game_x_ + (item.game_y_ * 64)) * 2)));
-    }
-  }
-
-  // Prepare pointer reuse cache
-  std::vector<int> item_pointers(pointer_count, -1);
-  std::vector<int> item_pointers_reuse(pointer_count, -1);
-
-  for (int i = 0; i < pointer_count; ++i) {
-    item_pointers_reuse[i] = -1;
-    for (int ci = 0; ci < i; ++ci) {
-      if (room_items[i].empty()) {
-        item_pointers_reuse[i] = -2;  // reuse empty terminator
-        break;
-      }
-
-      if (CompareItemsArrays(room_items[i], room_items[ci])) {
-        item_pointers_reuse[i] = ci;
-        break;
-      }
-    }
-  }
-
-  // Item data always lives in the vanilla data block
-  int data_pos = zelda3::kOverworldItemsStartDataNew;
-  int empty_pointer = -1;
-
-  for (int i = 0; i < pointer_count; ++i) {
-    if (item_pointers_reuse[i] == -1) {
-      item_pointers[i] = data_pos;
-      for (const OverworldItem& item : room_items[i]) {
-        const uint16_t map_pos =
-            static_cast<uint16_t>(((item.game_y_ << 6) + item.game_x_) << 1);
-        const uint32_t data =
-            static_cast<uint32_t>(map_pos & 0xFF) |
-            (static_cast<uint32_t>((map_pos >> 8) & 0xFF) << 8) |
-            (static_cast<uint32_t>(item.id_) << 16);
-
-        RETURN_IF_ERROR(rom()->WriteLong(data_pos, data));
-        data_pos += 3;
-      }
-
-      empty_pointer = data_pos;
-      RETURN_IF_ERROR(rom()->WriteShort(data_pos, 0xFFFF));
-      data_pos += 2;
-    } else if (item_pointers_reuse[i] == -2) {
-      if (empty_pointer < 0) {
-        item_pointers[i] = data_pos;
-        empty_pointer = data_pos;
-        RETURN_IF_ERROR(rom()->WriteShort(data_pos, 0xFFFF));
-        data_pos += 2;
-      } else {
-        item_pointers[i] = empty_pointer;
-      }
-    } else {
-      item_pointers[i] = item_pointers[item_pointers_reuse[i]];
-    }
-  }
-
-  if (data_pos > kOverworldItemsEndData) {
-    return absl::AbortedError("Too many items");
-  }
-
-  // Update pointer table metadata to the expanded location used by ZScream
-  RETURN_IF_ERROR(rom()->WriteLong(
-      zelda3::overworldItemsAddress, PcToSnes(zelda3::kOverworldItemsPointersNew)));
-  RETURN_IF_ERROR(rom()->WriteByte(
-      zelda3::overworldItemsAddressBank,
-      static_cast<uint8_t>((PcToSnes(zelda3::kOverworldItemsStartDataNew) >> 16) &
-                           0xFF)));
-
-  // Clear pointer table (write zero) to avoid stale values when pointer count shrinks
-  for (int i = 0; i < zelda3::kNumOverworldMaps; ++i) {
-    RETURN_IF_ERROR(rom()->WriteShort(zelda3::kOverworldItemsPointersNew + (i * 2),
-                                     0x0000));
-  }
-
-  for (int i = 0; i < pointer_count; ++i) {
-    const uint32_t snes_addr = PcToSnes(item_pointers[i]);
-    RETURN_IF_ERROR(rom()->WriteShort(zelda3::kOverworldItemsPointersNew + (i * 2),
-                                     static_cast<uint16_t>(snes_addr & 0xFFFF)));
-  }
-
-  util::logf("End of Items : %d", data_pos);
-
   return absl::OkStatus();
 }
 

src/zelda3/overworld/overworld.h

@@ -101,21 +101,12 @@ constexpr int overworldTilesType = 0x071459;
 constexpr int overworldMessages = 0x03F51D;
 constexpr int overworldMessagesExpanded = 0x1417F8;
 
-constexpr int overworldItemsPointers = 0x0DC2F9;
-constexpr int overworldItemsAddress = 0x0DC8B9; // 1BC2F9
-constexpr int overworldItemsAddressBank = 0x0DC8BF;
-constexpr int overworldItemsEndData = 0x0DC89C; // 0DC89E
-
-constexpr int overworldBombDoorItemLocationsNew = 0x012644;
-constexpr int overworldItemsPointersNew = 0x012784;
-constexpr int overworldItemsStartDataNew = 0x0DC2F9;
-
 constexpr int kOverworldCompressedMapPos = 0x058000;
 constexpr int kOverworldCompressedOverflowPos = 0x137FFF;
 
 constexpr int kNumTileTypes = 0x200;
 constexpr int kMap16Tiles = 0x78000;
-constexpr int kNumOverworldMaps = 160;
+
 constexpr int kNumTile16Individual = 4096;
 constexpr int Map32PerScreen = 256;
 constexpr int NumberOfMap16 = 3752;    // 4096
@@ -139,7 +130,7 @@ class Overworld {
   absl::Status LoadOverworldMaps();
   void LoadTileTypes();
 
-  absl::Status LoadItems();
+  // absl::Status LoadItems();
   absl::Status LoadSprites();
   absl::Status LoadSpritesFromMap(int sprite_start, int sprite_count,
                                   int sprite_index);

src/zelda3/overworld/overworld_item.cc

@@ -0,0 +1,208 @@
+#include "zelda3/overworld/overworld_item.h"
+
+#include <algorithm>
+#include <cstdint>
+#include <vector>
+
+#include "absl/status/status.h"
+#include "absl/status/statusor.h"
+#include "app/rom.h"
+#include "app/snes.h"
+#include "util/log.h"
+#include "util/macro.h"
+#include "zelda3/common.h"
+#include "zelda3/overworld/overworld_map.h"
+
+namespace yaze::zelda3 {
+
+absl::StatusOr<std::vector<OverworldItem>> LoadItems(
+    Rom* rom, std::vector<OverworldMap>& overworld_maps) {
+  std::vector<OverworldItem> items;
+
+  // Version 0x03 of the OW ASM added item support for the SW.
+  uint8_t asm_version = (*rom)[OverworldCustomASMHasBeenApplied];
+
+  // Determine max number of overworld maps based on actual ASM version
+  // Only use expanded maps (0xA0) if v3+ ASM is actually applied
+  int max_ow =
+      (asm_version >= 0x03 && asm_version != 0xFF) ? kNumOverworldMaps : 0x80;
+
+  ASSIGN_OR_RETURN(uint32_t pointer_snes,
+                   rom->ReadLong(zelda3::overworldItemsAddress));
+  uint32_t item_pointer_address =
+      SnesToPc(pointer_snes);  // 0x1BC2F9 -> 0x0DC2F9
+
+  for (int i = 0; i < max_ow; i++) {
+    ASSIGN_OR_RETURN(uint8_t bank_byte,
+                     rom->ReadByte(zelda3::overworldItemsAddressBank));
+    int bank = bank_byte & 0x7F;
+
+    ASSIGN_OR_RETURN(uint8_t addr_low,
+                     rom->ReadByte(item_pointer_address + (i * 2)));
+    ASSIGN_OR_RETURN(uint8_t addr_high,
+                     rom->ReadByte(item_pointer_address + (i * 2) + 1));
+
+    uint32_t addr = (bank << 16) +      // 1B
+                    (addr_high << 8) +  // F9
+                    addr_low;           // 3C
+    addr = SnesToPc(addr);
+
+    // Check if this is a large map and skip if not the parent
+    if (overworld_maps[i].area_size() != zelda3::AreaSizeEnum::SmallArea) {
+      if (overworld_maps[i].parent() != (uint8_t)i) {
+        continue;
+      }
+    }
+
+    while (true) {
+      ASSIGN_OR_RETURN(uint8_t b1, rom->ReadByte(addr));
+      ASSIGN_OR_RETURN(uint8_t b2, rom->ReadByte(addr + 1));
+      ASSIGN_OR_RETURN(uint8_t b3, rom->ReadByte(addr + 2));
+
+      if (b1 == 0xFF && b2 == 0xFF) {
+        break;
+      }
+
+      int p = (((b2 & 0x1F) << 8) + b1) >> 1;
+
+      int x = p % 0x40;  // Use 0x40 instead of 64 to match ZS
+      int y = p >> 6;
+
+      int fakeID = i % 0x40;  // Use modulo 0x40 to match ZS
+
+      int sy = fakeID / 8;
+      int sx = fakeID - (sy * 8);
+
+      items.emplace_back(b3, (uint16_t)i, (x * 16) + (sx * 512),
+                         (y * 16) + (sy * 512), false);
+      auto size = items.size();
+
+      items[size - 1].game_x_ = (uint8_t)x;
+      items[size - 1].game_y_ = (uint8_t)y;
+      addr += 3;
+    }
+  }
+  return items;
+}
+
+absl::Status SaveItems(Rom* rom, const std::vector<OverworldItem>& items) {
+  const int pointer_count = zelda3::kNumOverworldMaps;
+
+  std::vector<std::vector<OverworldItem>> room_items(pointer_count);
+
+  // Reset bomb door lookup table used by special item (0x86)
+  for (int i = 0; i < zelda3::kNumOverworldMaps; ++i) {
+    RETURN_IF_ERROR(rom->WriteShort(
+        zelda3::kOverworldBombDoorItemLocationsNew + (i * 2), 0x0000));
+  }
+
+  for (const OverworldItem& item : items) {
+    if (item.deleted)
+      continue;
+
+    const int map_index = static_cast<int>(item.room_map_id_);
+    if (map_index < 0 || map_index >= pointer_count) {
+      LOG_WARN(
+          "Overworld::SaveItems",
+          "Skipping item with map index %d outside pointer table (size=%d)",
+          map_index, pointer_count);
+      continue;
+    }
+
+    room_items[map_index].push_back(item);
+
+    if (item.id_ == 0x86) {
+      const int lookup_index =
+          std::min(map_index, zelda3::kNumOverworldMaps - 1);
+      RETURN_IF_ERROR(rom->WriteShort(
+          zelda3::kOverworldBombDoorItemLocationsNew + (lookup_index * 2),
+          static_cast<uint16_t>((item.game_x_ + (item.game_y_ * 64)) * 2)));
+    }
+  }
+
+  // Prepare pointer reuse cache
+  std::vector<int> item_pointers(pointer_count, -1);
+  std::vector<int> item_pointers_reuse(pointer_count, -1);
+
+  for (int i = 0; i < pointer_count; ++i) {
+    item_pointers_reuse[i] = -1;
+    for (int ci = 0; ci < i; ++ci) {
+      if (room_items[i].empty()) {
+        item_pointers_reuse[i] = -2;  // reuse empty terminator
+        break;
+      }
+
+      if (CompareItemsArrays(room_items[i], room_items[ci])) {
+        item_pointers_reuse[i] = ci;
+        break;
+      }
+    }
+  }
+
+  // Item data always lives in the vanilla data block
+  int data_pos = zelda3::kOverworldItemsStartDataNew;
+  int empty_pointer = -1;
+
+  for (int i = 0; i < pointer_count; ++i) {
+    if (item_pointers_reuse[i] == -1) {
+      item_pointers[i] = data_pos;
+      for (const OverworldItem& item : room_items[i]) {
+        const uint16_t map_pos =
+            static_cast<uint16_t>(((item.game_y_ << 6) + item.game_x_) << 1);
+        const uint32_t data =
+            static_cast<uint32_t>(map_pos & 0xFF) |
+            (static_cast<uint32_t>((map_pos >> 8) & 0xFF) << 8) |
+            (static_cast<uint32_t>(item.id_) << 16);
+
+        RETURN_IF_ERROR(rom->WriteLong(data_pos, data));
+        data_pos += 3;
+      }
+
+      empty_pointer = data_pos;
+      RETURN_IF_ERROR(rom->WriteShort(data_pos, 0xFFFF));
+      data_pos += 2;
+    } else if (item_pointers_reuse[i] == -2) {
+      if (empty_pointer < 0) {
+        item_pointers[i] = data_pos;
+        empty_pointer = data_pos;
+        RETURN_IF_ERROR(rom->WriteShort(data_pos, 0xFFFF));
+        data_pos += 2;
+      } else {
+        item_pointers[i] = empty_pointer;
+      }
+    } else {
+      item_pointers[i] = item_pointers[item_pointers_reuse[i]];
+    }
+  }
+
+  if (data_pos > kOverworldItemsEndData) {
+    return absl::AbortedError("Too many items");
+  }
+
+  // Update pointer table metadata to the expanded location used by ZScream
+  RETURN_IF_ERROR(rom->WriteLong(zelda3::overworldItemsAddress,
+                                 PcToSnes(zelda3::kOverworldItemsPointersNew)));
+  RETURN_IF_ERROR(rom->WriteByte(
+      zelda3::overworldItemsAddressBank,
+      static_cast<uint8_t>(
+          (PcToSnes(zelda3::kOverworldItemsStartDataNew) >> 16) & 0xFF)));
+
+  // Clear pointer table (write zero) to avoid stale values when pointer count shrinks
+  for (int i = 0; i < zelda3::kNumOverworldMaps; ++i) {
+    RETURN_IF_ERROR(
+        rom->WriteShort(zelda3::kOverworldItemsPointersNew + (i * 2), 0x0000));
+  }
+
+  for (int i = 0; i < pointer_count; ++i) {
+    const uint32_t snes_addr = PcToSnes(item_pointers[i]);
+    RETURN_IF_ERROR(
+        rom->WriteShort(zelda3::kOverworldItemsPointersNew + (i * 2),
+                        static_cast<uint16_t>(snes_addr & 0xFFFF)));
+  }
+
+  util::logf("End of Items : %d", data_pos);
+
+  return absl::OkStatus();
+}
+
+}  // namespace yaze::zelda3

src/zelda3/overworld/overworld_item.h

@@ -8,11 +8,17 @@
 #include <string>
 #include <vector>
 
+#include "absl/status/status.h"
+#include "absl/status/statusor.h"
+#include "app/rom.h"
 #include "zelda3/common.h"
 
 namespace yaze {
 namespace zelda3 {
 
+// Forward declaration of OverworldMap class
+class OverworldMap;
+
 constexpr int kNumOverworldMapItemPointers = 0x80;
 constexpr int kOverworldItemsPointers = 0xDC2F9;
 constexpr int kOverworldItemsAddress = 0xDC8B9;  // 1BC2F9
@@ -23,6 +29,15 @@ constexpr int kOverworldBombDoorItemLocationsNew = 0x012644;
 constexpr int kOverworldItemsPointersNew = 0x012784;
 constexpr int kOverworldItemsStartDataNew = 0x0DC2F9;
 
+constexpr int overworldItemsPointers = 0x0DC2F9;
+constexpr int overworldItemsAddress = 0x0DC8B9; // 1BC2F9
+constexpr int overworldItemsAddressBank = 0x0DC8BF;
+constexpr int overworldItemsEndData = 0x0DC89C; // 0DC89E
+
+constexpr int overworldBombDoorItemLocationsNew = 0x012644;
+constexpr int overworldItemsPointersNew = 0x012784;
+constexpr int overworldItemsStartDataNew = 0x0DC2F9;
+
 class OverworldItem : public GameEntity {
  public:
   OverworldItem() = default;
@@ -83,6 +98,36 @@ inline bool CompareOverworldItems(const std::vector<OverworldItem>& items1,
                      });
 }
 
+inline bool CompareItemsArrays(std::vector<OverworldItem> item_array1,
+                        std::vector<OverworldItem> item_array2) {
+  if (item_array1.size() != item_array2.size()) {
+    return false;
+  }
+
+  bool match;
+  for (size_t i = 0; i < item_array1.size(); i++) {
+    match = false;
+    for (size_t j = 0; j < item_array2.size(); j++) {
+      // Check all sprite in 2nd array if one match
+      if (item_array1[i].x_ == item_array2[j].x_ &&
+          item_array1[i].y_ == item_array2[j].y_ &&
+          item_array1[i].id_ == item_array2[j].id_) {
+        match = true;
+        break;
+      }
+    }
+
+    if (!match) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
+absl::StatusOr<std::vector<OverworldItem>> LoadItems(Rom* rom, std::vector<OverworldMap>& overworld_maps);
+absl::Status SaveItems(Rom* rom, const std::vector<OverworldItem>& items);
+
 const std::vector<std::string> kSecretItemNames = {
     "Nothing",        // 0
     "Green Rupee",    // 1

src/zelda3/overworld/overworld_map.h

@@ -10,17 +10,12 @@
 #include "app/gfx/types/snes_palette.h"
 #include "app/gfx/types/snes_tile.h"
 #include "app/rom.h"
-#include "zelda3/overworld/overworld_item.h"
 
 namespace yaze {
 namespace zelda3 {
 
 static constexpr int kTileOffsets[] = {0, 8, 4096, 4104};
 
-// 1 byte, not 0 if enabled
-// vanilla, v2, v3
-constexpr int OverworldCustomASMHasBeenApplied = 0x140145;
-
 // 2 bytes for each overworld area (0x140)
 constexpr int OverworldCustomAreaSpecificBGPalette = 0x140000;
 

src/zelda3/zelda3_library.cmake

@@ -12,6 +12,7 @@ set(
   zelda3/overworld/overworld_map.cc
   zelda3/overworld/overworld_entrance.cc
   zelda3/overworld/overworld_exit.cc
+  zelda3/overworld/overworld_item.cc
   zelda3/palette_constants.cc
   zelda3/screen/dungeon_map.cc
   zelda3/screen/inventory.cc