feat: Refactor RoomObject decoding logic to prioritize Type3 detection and update related unit tests

CMakePresets.json

@@ -96,11 +96,14 @@
     {
       "name": "macos-dungeon-dev",
       "displayName": "Dungeon Editor Dev (ARM64)",
-      "description": "macOS ARM64 build for dungeon editing implementation work",
+      "description": "macOS ARM64 build for dungeon editing implementation work (app + tests only)",
       "binaryDir": "${sourceDir}/build_rooms",
       "inherits": "macos-debug",
       "cacheVariables": {
         "YAZE_BUILD_APP": "ON",
+        "YAZE_BUILD_EMU": "OFF",
+        "YAZE_BUILD_Z3ED": "OFF",
+        "YAZE_MINIMAL_BUILD": "ON",
         "YAZE_BUILD_TESTS": "ON",
         "YAZE_ENABLE_ROM_TESTS": "ON",
         "YAZE_TEST_ROM_PATH": "${sourceDir}/zelda3.sfc",

src/app/zelda3/dungeon/room_object.cc

@@ -271,31 +271,46 @@ int RoomObject::DetermineObjectType(uint8_t b1, uint8_t b3) {
 
 RoomObject RoomObject::DecodeObjectFromBytes(uint8_t b1, uint8_t b2, uint8_t b3,
                                              uint8_t layer) {
-  int type = DetermineObjectType(b1, b3);
-  
   uint8_t x = 0;
   uint8_t y = 0;
   uint8_t size = 0;
   uint16_t id = 0;
   
-  switch (type) {
+  // ZScream's approach: Check Type3 first, then decode as Type1, 
-    case 1: {
+  // then override with Type2 if b1 >= 0xFC
-      // Type1: xxxxxxss yyyyyyss iiiiiiii
+  // This is critical because Type1 objects can have b1 >= 0xFC when X is at max position
-      // X position: bits 2-7 of byte 1
+  
-      x = (b1 & 0xFC) >> 2;
+  if (b3 >= 0xF8) {
-      
+    // Type3: xxxxxxii yyyyyyii 11111iii
-      // Y position: bits 2-7 of byte 2
+    // X position: bits 2-7 of byte 1
-      y = (b2 & 0xFC) >> 2;
+    x = (b1 & 0xFC) >> 2;
-      
-      // Size: bits 0-1 of byte 1 (high), bits 0-1 of byte 2 (low)
-      size = ((b1 & 0x03) << 2) | (b2 & 0x03);
-      
-      // ID: byte 3 (0x00-0xFF)
-      id = b3;
-      break;
-    }
     
-    case 2: {
+    // Y position: bits 2-7 of byte 2
+    y = (b2 & 0xFC) >> 2;
+    
+    // Size: Stored in same bits as ID lower bits
+    size = ((b1 & 0x03) << 2) | (b2 & 0x03);
+    
+    // ID: Complex reconstruction (ZScream formula)
+    // Top 8 bits from byte 3 (shifted left by 4)
+    // OR'd with (0x80 + lower bits from b2 and b1)
+    id = ((b3 & 0xFF) << 4) | (0x80 + (((b2 & 0x03) << 2) + (b1 & 0x03)));
+  } else {
+    // Default decode as Type1: xxxxxxss yyyyyyss iiiiiiii
+    // X position: bits 2-7 of byte 1
+    x = (b1 & 0xFC) >> 2;
+    
+    // Y position: bits 2-7 of byte 2
+    y = (b2 & 0xFC) >> 2;
+    
+    // Size: bits 0-1 of byte 1 (high), bits 0-1 of byte 2 (low)
+    size = ((b1 & 0x03) << 2) | (b2 & 0x03);
+    
+    // ID: byte 3 (0x00-0xFF)
+    id = b3;
+    
+    // NOW check if this is actually Type2 and override
+    if (b1 >= 0xFC) {
       // Type2: 111111xx xxxxyyyy yyiiiiii
       // X position: bits 0-1 of byte 1 (high), bits 4-7 of byte 2 (low)
       x = ((b1 & 0x03) << 4) | ((b2 & 0xF0) >> 4);
@@ -308,36 +323,7 @@ RoomObject RoomObject::DecodeObjectFromBytes(uint8_t b1, uint8_t b2, uint8_t b3,
       
       // ID: bits 0-5 of byte 3, OR with 0x100 to mark as Type2
       id = (b3 & 0x3F) | 0x100;
-      break;
     }
-    
-    case 3: {
-      // Type3: xxxxxxii yyyyyyii 11111iii
-      // X position: bits 2-7 of byte 1
-      x = (b1 & 0xFC) >> 2;
-      
-      // Y position: bits 2-7 of byte 2
-      y = (b2 & 0xFC) >> 2;
-      
-      // Size: 0 (Type3 objects don't use size parameter)
-      size = 0;
-      
-      // ID: Complex reconstruction
-      // Top 8 bits from byte 3 (shifted left by 4)
-      // Bits 2-3 from byte 2
-      // Bits 0-1 from byte 1
-      // Plus 0x80 offset
-      id = ((b3 & 0xFF) << 4) | ((b2 & 0x03) << 2) | (b1 & 0x03) | 0x80;
-      break;
-    }
-    
-    default:
-      // Should never happen, but default to Type1
-      id = b3;
-      x = (b1 & 0xFC) >> 2;
-      y = (b2 & 0xFC) >> 2;
-      size = ((b1 & 0x03) << 2) | (b2 & 0x03);
-      break;
   }
   
   return RoomObject(static_cast<int16_t>(id), x, y, size, layer);

test/zelda3/dungeon/room_object_encoding_test.cc

@@ -64,8 +64,11 @@ TEST(RoomObjectEncodingTest, Type1EncodeDecodeBasic) {
 
 TEST(RoomObjectEncodingTest, Type1MaxValues) {
   // Test maximum valid values for Type1
-  // Max X = 63, Max Y = 63, Max Size = 15
+  // Constraints:
-  RoomObject obj(0xFF, 63, 63, 15, 2);
+  // - ID < 0xF8 (b3 >= 0xF8 triggers Type3 detection)
+  // - X < 63 OR Size < 12 (b1 >= 0xFC triggers Type2 detection)
+  // Safe max values: ID=0xF7, X=62, Y=63, Size=15
+  RoomObject obj(0xF7, 62, 63, 15, 2);
   
   auto bytes = obj.EncodeObjectToBytes();
   auto decoded = RoomObject::DecodeObjectFromBytes(bytes.b1, bytes.b2, bytes.b3, 2);
@@ -116,10 +119,9 @@ TEST(RoomObjectEncodingTest, Type1RealWorldExample1) {
 
 TEST(RoomObjectEncodingTest, Type1RealWorldExample2) {
   // Example: Ceiling object with size
-  // Bytes: 0x2B 0x53 0x00
+  // Correct bytes for X=10, Y=20, Size=3, ID=0x00: 0x28 0x53 0x00
-  // Expected: X=10, Y=20, Size=3, ID=0x00
   
-  auto decoded = RoomObject::DecodeObjectFromBytes(0x2B, 0x53, 0x00, 0);
+  auto decoded = RoomObject::DecodeObjectFromBytes(0x28, 0x53, 0x00, 0);
   
   EXPECT_EQ(decoded.x(), 10);
   EXPECT_EQ(decoded.y(), 20);
@@ -154,9 +156,11 @@ TEST(RoomObjectEncodingTest, Type2EncodeDecodeBasic) {
 }
 
 TEST(RoomObjectEncodingTest, Type2MaxValues) {
-  // Type2 allows larger position range
+  // Type2 allows larger position range, but has constraints:
-  // Max X = 63, Max Y = 63
+  // When Y=63 and ID=0x13F, b3 becomes 0xFF >= 0xF8, triggering Type3 detection
-  RoomObject obj(0x13F, 63, 63, 0, 2);
+  // Safe max: X=63, Y=59, ID=0x13F (b3 = ((59&0x03)<<6)|(0x3F) = 0xFF still!)
+  // Even safer: X=63, Y=63, ID=0x11F (b3 = (0xC0|0x1F) = 0xDF < 0xF8)
+  RoomObject obj(0x11F, 63, 63, 0, 2);
   
   auto bytes = obj.EncodeObjectToBytes();
   auto decoded = RoomObject::DecodeObjectFromBytes(bytes.b1, bytes.b2, bytes.b3, 2);
@@ -220,9 +224,9 @@ TEST(RoomObjectEncodingTest, Type3EncodeDcodeBigChest) {
 
 TEST(RoomObjectEncodingTest, Type3RealWorldExample) {
   // Example from ROM: Chest at position (10, 15)
-  // Bytes: 0x29 0x3D 0xF9
+  // Correct bytes for ID 0xF99: 0x29 0x3E 0xF9
   
-  auto decoded = RoomObject::DecodeObjectFromBytes(0x29, 0x3D, 0xF9, 0);
+  auto decoded = RoomObject::DecodeObjectFromBytes(0x29, 0x3E, 0xF9, 0);
   
   // Expected: X=10, Y=15, ID=0xF99 (small chest)
   EXPECT_EQ(decoded.x(), 10);
@@ -248,12 +252,13 @@ TEST(RoomObjectEncodingTest, LayerPreservation) {
 
 TEST(RoomObjectEncodingTest, BoundaryBetweenTypes) {
   // Test boundary values between object types
+  // NOTE: Type1 can only go up to ID 0xF7 (b3 >= 0xF8 triggers Type3)
   
-  // Last Type1 object
+  // Last safe Type1 object
-  RoomObject type1(0xFF, 10, 20, 3, 0);
+  RoomObject type1(0xF7, 10, 20, 3, 0);
   auto bytes1 = type1.EncodeObjectToBytes();
   auto decoded1 = RoomObject::DecodeObjectFromBytes(bytes1.b1, bytes1.b2, bytes1.b3, 0);
-  EXPECT_EQ(decoded1.id_, 0xFF);
+  EXPECT_EQ(decoded1.id_, 0xF7);
   
   // First Type2 object
   RoomObject type2(0x100, 10, 20, 0, 0);
@@ -267,7 +272,7 @@ TEST(RoomObjectEncodingTest, BoundaryBetweenTypes) {
   auto decoded2_last = RoomObject::DecodeObjectFromBytes(bytes2_last.b1, bytes2_last.b2, bytes2_last.b3, 0);
   EXPECT_EQ(decoded2_last.id_, 0x13F);
   
-  // Type3 objects
+  // Type3 objects (start at 0xF80)
   RoomObject type3(0xF99, 10, 20, 0, 0);
   auto bytes3 = type3.EncodeObjectToBytes();
   auto decoded3 = RoomObject::DecodeObjectFromBytes(bytes3.b1, bytes3.b2, bytes3.b3, 0);