Merge pull request #176 from zrax/location_dup_xtor

Remove duplicate Location constructor, and add unit tests for it
H-uru · Jul 12, 2023 · ace0af7 · ace0af7
2 parents a307705 + c7c2519
commit ace0af7
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 10 deletions.
diff --git a/GameServ/GameHost.cpp b/GameServ/GameHost.cpp
@@ -332,7 +332,7 @@ void dm_send_state(GameHost_Private* host, GameClient_Private* client)
     DS::Blob ageSdlBlob = host->m_ageSdlHook.toBlob();
     if (ageSdlBlob.size()) {
         Game_AgeInfo info = s_ages[host->m_ageFilename];
-        state->m_object.m_location = MOUL::Location::Make(info.m_seqPrefix, -2, MOUL::Location::e_BuiltIn);
+        state->m_object.m_location = MOUL::Location(info.m_seqPrefix, -2, MOUL::Location::e_BuiltIn);
         state->m_object.m_name = "AgeSDLHook";
         state->m_object.m_type = 1;  // SceneObject
         state->m_object.m_id = 1;
@@ -734,7 +734,7 @@ void dm_bcast_agesdl_hook(GameHost_Private* host)
     bcast->m_isInitial = true;
     bcast->m_persistOnServer = true;
     bcast->m_isAvatar = false;
-    bcast->m_object.m_location = MOUL::Location::Make(info.m_seqPrefix, -2, MOUL::Location::e_BuiltIn);
+    bcast->m_object.m_location = MOUL::Location(info.m_seqPrefix, -2, MOUL::Location::e_BuiltIn);
     bcast->m_object.m_name = "AgeSDLHook";
     bcast->m_object.m_type = 1;  // SceneObject
     bcast->m_object.m_id = 1;

diff --git a/PlasMOUL/Key.h b/PlasMOUL/Key.h
@@ -63,14 +63,6 @@ namespace MOUL
         bool operator>=(const Location& other) const
         { return m_sequence >= other.m_sequence; }
 
-        static Location Make(int32_t prefix, uint16_t page, uint16_t flags)
-        {
-            if (prefix < 0)
-                return Location(page - (prefix << 16) + 0xFF000001, flags);
-            else
-                return Location(page + (prefix << 16) + 0x00000021, flags);
-        }
-
     public:
         uint32_t m_sequence;
         uint16_t m_flags;

diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt
@@ -8,6 +8,7 @@ FetchContent_MakeAvailable(Catch2)
 
 set(test_SOURCES
     main.cpp
+    Test_Location.cpp
     Test_SDL.cpp
     Test_ShaHash.cpp
 )

diff --git a/Tests/Test_Location.cpp b/Tests/Test_Location.cpp
@@ -0,0 +1,57 @@
+/******************************************************************************
+ * This file is part of dirtsand.                                             *
+ *                                                                            *
+ * dirtsand is free software: you can redistribute it and/or modify           *
+ * it under the terms of the GNU Affero General Public License as             *
+ * published by the Free Software Foundation, either version 3 of the         *
+ * License, or (at your option) any later version.                            *
+ *                                                                            *
+ * dirtsand 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 Affero General Public License for more details.                        *
+ *                                                                            *
+ * You should have received a copy of the GNU Affero General Public License   *
+ * along with dirtsand.  If not, see <http://www.gnu.org/licenses/>.          *
+ ******************************************************************************/
+
+#include <catch2/catch.hpp>
+
+#include "PlasMOUL/Key.h"
+
+#define CHECK_SEQUENCE(loc, sequence) \
+    CHECK((loc).m_sequence == sequence)
+
+TEST_CASE("Test MOUL::Location", "[location]")
+{
+    SECTION("From prefix and page number") {
+        // Yes, there are multiple ways of encoding the same sequence...
+        CHECK_SEQUENCE(MOUL::Location(0, 0, 0), 0x00000021);
+        CHECK_SEQUENCE(MOUL::Location(1, 0, 0), 0x00010021);
+        CHECK_SEQUENCE(MOUL::Location(100, 1, 0), 0x00640022);
+        CHECK_SEQUENCE(MOUL::Location(65535, 65502, 0), 0xFFFFFFFF);
+
+        CHECK_SEQUENCE(MOUL::Location(1, -1, 0), 0x00020020);
+        CHECK_SEQUENCE(MOUL::Location(100, -33, 0), 0x00650000);
+        CHECK_SEQUENCE(MOUL::Location(65534, -33, 0), 0xFFFF0000);
+
+        CHECK_SEQUENCE(MOUL::Location(-1, 0, 0), 0xFF010001);
+        CHECK_SEQUENCE(MOUL::Location(-100, 1, 0), 0xFF640002);
+        CHECK_SEQUENCE(MOUL::Location(-255, 65534, 0), 0xFFFFFFFF);
+
+        CHECK_SEQUENCE(MOUL::Location(-1, -1, 0), 0xFF020000);
+        CHECK_SEQUENCE(MOUL::Location(-254, -1, 0), 0xFFFF0000);
+
+        // Wrap around -- not actually valid...
+        CHECK_SEQUENCE(MOUL::Location(65537, 0, 0), 0x00010021);
+        CHECK_SEQUENCE(MOUL::Location(65536, -1, 0), 0x00010020);
+        CHECK_SEQUENCE(MOUL::Location(65536, -33, 0), 0x00010000);
+        CHECK_SEQUENCE(MOUL::Location(-256, 0, 0), 0x00000001);
+        CHECK_SEQUENCE(MOUL::Location(-255, -1, 0), 0x00000000);
+
+        CHECK_SEQUENCE(MOUL::Location(1, 65503, 0), 0x00020000);
+        CHECK_SEQUENCE(MOUL::Location(1, -34, 0), 0x0001FFFF);
+        CHECK_SEQUENCE(MOUL::Location(-255, 65535, 0), 0x00000000);
+        CHECK_SEQUENCE(MOUL::Location(-255, -2, 0), 0xFFFFFFFF);
+    }
+}