Merge pull request #311 from zrax/fix_location_encoding

zrax · web-flow · commit 083742b619b4 · 2025-11-01T10:32:45.000-07:00
Fix plLocation encoding/decoding to match Plasma, and add tests.
diff --git a/.github/workflows/linux-ci.yml b/.github/workflows/linux-ci.yml
@@ -24,3 +24,4 @@ jobs:
           cmake -DCMAKE_BUILD_TYPE=Debug -DCMAKE_PREFIX_PATH="${GITHUB_WORKSPACE}/build_deps/prefix" \
             -DENABLE_PYTHON=ON -DENABLE_TOOLS=ON -DENABLE_NET=ON -DENABLE_PHYSX=OFF ..
           make -j2
+          make test
diff --git a/.github/workflows/macos-ci.yml b/.github/workflows/macos-ci.yml
@@ -38,3 +38,4 @@ jobs:
             -DCMAKE_PREFIX_PATH="${GITHUB_WORKSPACE}/build_deps/prefix;/usr/local/opt/jpeg-turbo;/usr/local/opt/openssl" \
             -DENABLE_PYTHON=ON -DENABLE_TOOLS=ON -DENABLE_NET=ON -DENABLE_PHYSX=OFF ..
           make -j2
+          make test
diff --git a/.github/workflows/windows-ci.yml b/.github/workflows/windows-ci.yml
@@ -78,6 +78,7 @@ jobs:
       - name: Build
         run: |
           cmake --build build --config Release -j 2
+          cmake --build build --target RUN_TESTS --config Release -j 2
 
       - name: Install
         run: |
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -66,6 +66,7 @@ option(ENABLE_PYTHON "Build HSPlasma Python integration" ON)
 option(ENABLE_TOOLS "Build the HSPlasma tools" ON)
 option(ENABLE_NET "Build HSPlasmaNet" ON)
 option(ENABLE_PHYSX "Build with PhysX Support" ON)
+option(ENABLE_TESTS "Build test suite" ON)
 option(BUILD_SHARED_LIBS "Build shared libraries" ON)
 
 find_package(string_theory 2.0 REQUIRED)
@@ -84,5 +85,10 @@ if(ENABLE_NET)
     add_subdirectory(net)
 endif()
 
+if(ENABLE_TESTS)
+    enable_testing()
+    add_subdirectory(Tests)
+endif()
+
 include(FeatureSummary)
 feature_summary(WHAT ALL FATAL_ON_MISSING_REQUIRED_PACKAGES)
diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt
@@ -0,0 +1,19 @@
+include(FetchContent)
+FetchContent_Declare(Catch2
+    GIT_REPOSITORY  https://github.com/catchorg/Catch2.git
+    GIT_TAG         v2.13.10
+)
+FetchContent_MakeAvailable(Catch2)
+
+set(test_SOURCES
+    catch2_main.cpp
+    Test_Location.cpp
+)
+add_executable(test_HSPlasma ${test_SOURCES})
+target_link_libraries(test_HSPlasma PRIVATE Catch2::Catch2 HSPlasma)
+
+# Integrate with CTest so we can use the built-in test targets
+list(APPEND CMAKE_MODULE_PATH "${catch2_SOURCE_DIR}/contrib")
+include(CTest)
+include(Catch)
+catch_discover_tests(test_HSPlasma)
diff --git a/Tests/Test_Location.cpp b/Tests/Test_Location.cpp
@@ -0,0 +1,124 @@
+/* This file is part of HSPlasma.
+ *
+ * HSPlasma is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * HSPlasma is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with HSPlasma.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <catch2/catch.hpp>
+
+#include "PRP/KeyedObject/plLocation.h"
+
+#define CHECK_LOCATION(version, seqNum, valid, virt, seqPrefix, pageNum)    \
+    do {                                                                    \
+        plLocation loc(version);                                            \
+        loc.parse(seqNum);                                                  \
+        CHECK(loc.isValid() == valid);                                      \
+        CHECK(loc.isVirtual() == virt);                                     \
+        CHECK(loc.getSeqPrefix() == seqPrefix);                             \
+        CHECK(loc.getPageNum() == pageNum);                                 \
+        CHECK(loc.unparse() == seqNum);                                     \
+    } while (false)
+
+#define CHECK_LOCATION_MOUL(seqNum, seqPrefix, pageNum)                     \
+    CHECK_LOCATION(PlasmaVer::pvMoul, seqNum, true, false, seqPrefix, pageNum)
+
+#define CHECK_LOCATION_CLASSIC(seqNum, seqPrefix, pageNum)                  \
+    CHECK_LOCATION(PlasmaVer::pvPrime, seqNum, true, false, seqPrefix, pageNum)
+
+TEST_CASE("plLocation packing and unpacking", "[location]")
+{
+    SECTION("Uru Live format plLocations") {
+        CHECK_LOCATION(PlasmaVer::pvMoul, 0, true, true, 0, 0);
+        CHECK_LOCATION(PlasmaVer::pvMoul, 0xFFFF'FFFF, false, false, 0, 0);
+
+        CHECK_LOCATION_MOUL(0x0000'0021, 0, 0);
+        CHECK_LOCATION_MOUL(0x0000'0022, 0, 1);
+        CHECK_LOCATION_MOUL(0x0000'7FFF, 0, 0x7FDE);
+        CHECK_LOCATION_MOUL(0x0000'8020, 0, 0x7FFF);
+        CHECK_LOCATION_MOUL(0x0001'0000, 0, 0xFFDF);
+        CHECK_LOCATION_MOUL(0x0001'0001, 0, -32);   // 0, 0xFFE0
+        CHECK_LOCATION_MOUL(0x0001'0020, 0, -1);    // 0, 0xFFFF
+
+        CHECK_LOCATION_MOUL(0x0001'0021, 1, 0);
+        CHECK_LOCATION_MOUL(0x00FF'0021, 255, 0);
+        CHECK_LOCATION_MOUL(0x0100'0020, 255, -1);
+        CHECK_LOCATION_MOUL(0x7FFF'0021, 0x7FFF, 0);
+        CHECK_LOCATION_MOUL(0xFEFF'0021, 0xFEFF, 0);
+        CHECK_LOCATION_MOUL(0xFF00'0020, 0xFEFF, -1);
+
+        CHECK_LOCATION_MOUL(0xFF01'0001, -1, 0);
+        CHECK_LOCATION_MOUL(0xFF01'0002, -1, 1);
+        CHECK_LOCATION_MOUL(0xFF01'8000, -1, 0x7FFF);
+        CHECK_LOCATION_MOUL(0xFF02'0000, -1, -1);
+        CHECK_LOCATION_MOUL(0xFFFE'0001, -254, 0);
+        CHECK_LOCATION_MOUL(0xFFFF'0000, -254, -1);
+
+        // Examples from real PRPs
+        CHECK_LOCATION_MOUL(0x0006'0022, 6, 1);         // city:canyon
+        CHECK_LOCATION_MOUL(0x0006'0086, 6, 101);       // city:museumDoor
+        CHECK_LOCATION_MOUL(0x0007'001F, 6, -2);        // city:BuiltIn
+        CHECK_LOCATION_MOUL(0x0007'0020, 6, -1);        // city:Textures
+        CHECK_LOCATION_MOUL(0x04D2'0026, 1234, 5);      // GoMePubNew:Entry
+        CHECK_LOCATION_MOUL(0x9C44'0022, 40004, 1);     // VeeTsah:Temple
+        CHECK_LOCATION_MOUL(0x9C45'0020, 40004, -1);    // VeeTsah:Textures
+        CHECK_LOCATION_MOUL(0xFF01'0009, -1, 8);        // GlobalAnimations:MaleIdle
+        CHECK_LOCATION_MOUL(0xFF01'01B4, -1, 435);      // GlobalAnimations:FemaleSwimDockExit
+        CHECK_LOCATION_MOUL(0xFF06'0004, -6, 3);        // GlobalAvatars:Audio
+    }
+
+    SECTION("Classic format plLocations") {
+        CHECK_LOCATION(PlasmaVer::pvPrime, 0, true, true, 0, 0);
+        CHECK_LOCATION(PlasmaVer::pvPrime, 0xFFFF'FFFF, false, false, 0, 0);
+
+        CHECK_LOCATION_CLASSIC(0x0000'0021, 0, 0);
+        CHECK_LOCATION_CLASSIC(0x0000'0022, 0, 1);
+        CHECK_LOCATION_CLASSIC(0x0000'007F, 0, 0x5E);
+        CHECK_LOCATION_CLASSIC(0x0000'00A0, 0, 0x7F);
+        CHECK_LOCATION_CLASSIC(0x0000'0100, 0, 0xDF);
+        CHECK_LOCATION_CLASSIC(0x0000'0101, 0, -32);   // 0, 0xE0
+        CHECK_LOCATION_CLASSIC(0x0000'0120, 0, -1);    // 0, 0xFF
+
+        CHECK_LOCATION_CLASSIC(0x0000'0121, 1, 0);
+        CHECK_LOCATION_CLASSIC(0x0000'FF21, 255, 0);
+        CHECK_LOCATION_CLASSIC(0x0001'0020, 255, -1);
+        CHECK_LOCATION_CLASSIC(0x007F'FF21, 0x7FFF, 0);
+        CHECK_LOCATION_CLASSIC(0x00FE'FF21, 0xFEFF, 0);
+        CHECK_LOCATION_CLASSIC(0x00FF'0020, 0xFEFF, -1);
+        CHECK_LOCATION_CLASSIC(0xFFFE'FF21, 0xFF'FEFF, 0);
+        CHECK_LOCATION_CLASSIC(0xFFFF'0020, 0xFF'FEFF, -1);
+
+        CHECK_LOCATION_CLASSIC(0xFFFF'0101, -1, 0);
+        CHECK_LOCATION_CLASSIC(0xFFFF'0102, -1, 1);
+        CHECK_LOCATION_CLASSIC(0xFFFF'0180, -1, 0x7F);
+        CHECK_LOCATION_CLASSIC(0xFFFF'0200, -1, -1);
+        CHECK_LOCATION_CLASSIC(0xFFFF'FE01, -254, 0);
+        CHECK_LOCATION_CLASSIC(0xFFFF'FF00, -254, -1);
+
+        // Examples from real PRPs
+        CHECK_LOCATION_CLASSIC(0x0000'0622, 6, 1);      // city:canyon
+        CHECK_LOCATION_CLASSIC(0x0000'065E, 6, 61);     // city:islmLibBanners03Vis
+        CHECK_LOCATION_CLASSIC(0x0000'071F, 6, -2);     // city:BuiltIn
+        CHECK_LOCATION_CLASSIC(0x0000'0720, 6, -1);     // city:Textures
+        CHECK_LOCATION_CLASSIC(0x0000'2727, 39, 6);     // GreatZero:CalibrationMarkerGameGUI
+        CHECK_LOCATION_CLASSIC(0x0000'2820, 39, -1);    // GreatZero:Textures
+        CHECK_LOCATION_CLASSIC(0xFFFF'0109, -1, 8);     // GlobalAnimations:MaleIdle
+        CHECK_LOCATION_CLASSIC(0xFFFF'0604, -6, 3);     // GlobalAvatars:Audio
+
+        // The below test case reveals a fault in the Plasma data files -- the
+        // suffix (page) number overflows the maximum range of the encoding
+        // and ends up overlapping with <-2|11>, and therefore can't actually
+        // be decoded correctly.
+        //CHECK_LOCATION_CLASSIC(0xFFFF'020C, -1, 267);   // GlobalAnimations:FemaleBallPushWalk
+        CHECK_LOCATION_CLASSIC(0xFFFF'020C, -2, 11);   // GlobalAnimations:FemaleBallPushWalk
+    }
+}
diff --git a/Tests/catch2_main.cpp b/Tests/catch2_main.cpp
@@ -0,0 +1,19 @@
+/* This file is part of HSPlasma.
+ *
+ * HSPlasma is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * HSPlasma is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with HSPlasma.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+// Stub source file that just provides a main() for catch2
+#define CATCH_CONFIG_MAIN
+#include <catch2/catch.hpp>
diff --git a/core/PRP/KeyedObject/plLocation.cpp b/core/PRP/KeyedObject/plLocation.cpp
@@ -15,6 +15,7 @@
  */
 
 #include "plLocation.h"
+#include "Debug/plDebug.h"
 #include <string_theory/format>
 
 bool plLocation::operator==(const plLocation& other) const
@@ -53,20 +54,37 @@ void plLocation::parse(unsigned int id)
         throw hsBadParamException(__FILE__, __LINE__, "Universal PRPs don't use encoded locations");
 
     fState = kStateNormal;
-    if ((id & 0x80000000) != 0) {
-        id -= (fVer.isLive() ? 0xFF000001 : 0xFFFF0001);
-        fSeqPrefix = id >> (fVer.isLive() ? 16 : 8);
-        fPageNum = id - (fSeqPrefix << (fVer.isLive() ? 16 : 8));
-        fSeqPrefix = -fSeqPrefix;
+    if (fVer.isLive()) {
+        if (id >= 0xFF01'0000) {
+            id -= 0xFF00'0001;
+            fSeqPrefix = -(int)(id >> 16);
+        } else {
+            id -= 33;
+            fSeqPrefix = id >> 16;
+        }
+        fPageNum = (int16_t)(id & 0xFFFF);
     } else {
-        id -= 33;
-        fSeqPrefix = id >> (fVer.isLive() ? 16 : 8);
-        fPageNum = id - (fSeqPrefix << (fVer.isLive() ? 16 : 8));
+        if (id >= 0xFFFF'0100) {
+            id -= 0xFFFF'0001;
+            fSeqPrefix = -(int)(id >> 8);
+        } else {
+            id -= 33;
+            fSeqPrefix = id >> 8;
+        }
+        fPageNum = (int8_t)(id & 0xFF);
+    }
+
+    // Plasma does not actually use negative page numbers.  However, for
+    // correctly mapping and converting the "reserved" pages at the end of the
+    // max-uint8/max-uint16 range, we pretend they are signed and negative.
+    // In order to leave more room for non-reserved pages, we only apply this
+    // special logic up to -32
+    if (fPageNum < -32) {
+        if (fVer.isLive())
+            fPageNum += 0x10000;
+        else
+            fPageNum += 0x100;
     }
-    if (fVer.isLive())
-        fPageNum = (signed short)(unsigned short)fPageNum;
-    else
-        fPageNum = (signed char)(unsigned char)fPageNum;
 }
 
 unsigned int plLocation::unparse() const
@@ -78,16 +96,26 @@ unsigned int plLocation::unparse() const
     if (fVer.isUniversal())
         throw hsBadParamException(__FILE__, __LINE__, "Universal PRPs don't use encoded locations");
 
-    int pgNum;
-    if (fVer.isLive())
-        pgNum = (unsigned short)(signed short)fPageNum;
-    else
-        pgNum = (unsigned char)(signed char)fPageNum;
-    if (fSeqPrefix < 0) {
-        return pgNum - (fSeqPrefix << (fVer.isLive() ? 16 : 8))
-                     + (fVer.isLive() ? 0xFF000001 : 0xFFFF0001);
+    if (fVer.isLive()) {
+        if (fSeqPrefix < -254 || fSeqPrefix > 0xFEFF)
+            plDebug::Warning("Sequence prefix {} cannot be uniquely encoded.", fSeqPrefix);
+        if (fPageNum < -32 || fPageNum > 0xFFDF)
+            plDebug::Warning("Page number {} cannot be uniquely encoded.", fPageNum);
+
+        if (fSeqPrefix < 0)
+            return (-fSeqPrefix << 16) + (fPageNum & 0xFFFF) + 0xFF00'0001;
+        else
+            return (fSeqPrefix << 16) + (fPageNum & 0xFFFF) + 33;
     } else {
-        return pgNum + (fSeqPrefix << (fVer.isLive() ? 16 : 8)) + 33;
+        if (fSeqPrefix < -254 || fSeqPrefix > 0xFF'FEFF)
+            plDebug::Warning("Sequence prefix {} cannot be uniquely encoded.", fSeqPrefix);
+        if (fPageNum < -32 || fPageNum > 0xDF)
+            plDebug::Warning("Page number {} cannot be uniquely encoded.", fPageNum);
+
+        if (fSeqPrefix < 0)
+            return (-fSeqPrefix << 8) + (fPageNum & 0xFF) + 0xFFFF'0001;
+        else
+            return (fSeqPrefix << 8) + (fPageNum & 0xFF) + 33;
     }
 }
 
