From 9bfc95de37151a9dc4ef753397b5cfb4fbe4640a Mon Sep 17 00:00:00 2001 From: scawful Date: Sat, 4 Oct 2025 00:15:21 -0400 Subject: [PATCH] feat: Refactor RoomObject decoding logic to prioritize Type3 detection and update related unit tests --- CMakePresets.json | 5 +- src/app/zelda3/dungeon/room_object.cc | 82 ++++++++----------- .../dungeon/room_object_encoding_test.cc | 33 ++++---- 3 files changed, 57 insertions(+), 63 deletions(-) diff --git a/CMakePresets.json b/CMakePresets.json index 34fcf094..edcffac5 100644 --- a/CMakePresets.json +++ b/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", diff --git a/src/app/zelda3/dungeon/room_object.cc b/src/app/zelda3/dungeon/room_object.cc index 5c48bc69..0adfe97a 100644 --- a/src/app/zelda3/dungeon/room_object.cc +++ b/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) { - case 1: { - // 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; - break; - } + // ZScream's approach: Check Type3 first, then decode as Type1, + // then override with Type2 if b1 >= 0xFC + // This is critical because Type1 objects can have b1 >= 0xFC when X is at max position + + if (b3 >= 0xF8) { + // Type3: xxxxxxii yyyyyyii 11111iii + // X position: bits 2-7 of byte 1 + x = (b1 & 0xFC) >> 2; - 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(id), x, y, size, layer); diff --git a/test/zelda3/dungeon/room_object_encoding_test.cc b/test/zelda3/dungeon/room_object_encoding_test.cc index 23c2a90b..5fa58a5f 100644 --- a/test/zelda3/dungeon/room_object_encoding_test.cc +++ b/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 - RoomObject obj(0xFF, 63, 63, 15, 2); + // Constraints: + // - 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 - // Expected: X=10, Y=20, Size=3, ID=0x00 + // Correct bytes for X=10, Y=20, Size=3, ID=0x00: 0x28 0x53 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 - // Max X = 63, Max Y = 63 - RoomObject obj(0x13F, 63, 63, 0, 2); + // Type2 allows larger position range, but has constraints: + // When Y=63 and ID=0x13F, b3 becomes 0xFF >= 0xF8, triggering Type3 detection + // 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 - RoomObject type1(0xFF, 10, 20, 3, 0); + // Last safe Type1 object + 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);