Merge pull request #2630 from ivan-mogilko/362--systemimporttests

SystemImports tests

Engine/CMakeLists.txt

@@ -648,6 +648,7 @@ if(AGS_TESTS)
     add_executable(
         engine_test
         test/scsprintf_test.cpp
+        test/systemimports_test.cpp
     )
     set_target_properties(engine_test PROPERTIES
         CXX_STANDARD 11
@@ -659,6 +660,7 @@ if(AGS_TESTS)
     target_link_libraries(
         engine_test
         engine
+        common
         gtest_main
     )
 

Engine/script/script_runtime.cpp

@@ -20,6 +20,10 @@
 #include "script/systemimports.h"
 
 
+SystemImports simp;
+SystemImports simp_for_plugin;
+
+
 bool ccAddExternalStaticFunction(const String &name, ScriptAPIFunction *scfn, void *dirfn)
 {
     return simp.Add(name, RuntimeScriptValue().SetStaticFunction(scfn), nullptr) != UINT32_MAX &&

Engine/script/script_runtime.h

@@ -97,4 +97,10 @@ void ccSetScriptAliveTimer(unsigned sys_poll_timeout, unsigned abort_timeout, un
 // reset the current while loop counter
 void ccNotifyScriptStillAlive();
 
+// Symbols registered for scripts
+extern SystemImports simp;
+// This is to register symbols exclusively for plugins, to allow them
+// perform old style unsafe function calls
+extern SystemImports simp_for_plugin;
+
 #endif // __AGS_EE_CC__SCRIPTRUNTIME_H

Engine/script/systemimports.cpp

@@ -14,13 +14,9 @@
 #include "script/systemimports.h"
 #include <stdlib.h>
 #include <string.h>
-#include "script/cc_instance.h"
 
 using namespace AGS::Common;
 
-SystemImports simp;
-SystemImports simp_for_plugin;
-
 
 void ScriptSymbolsMap::Add(const String &name, uint32_t index)
 {
@@ -56,10 +52,11 @@ uint32_t ScriptSymbolsMap::GetIndexOfAny(const String &name) const
     // where "type" is the name of a type, "name" is the name of a function,
     // "argnum" is the number of arguments.
 
-    const size_t argnum_at = name.FindChar(_appendageSeparator);
+    const size_t argnum_at = std::min(name.GetLength(), name.FindChar(_appendageSeparator));
+    const size_t append_end = name.GetLength();
     // TODO: optimize this by supporting string views! or compare methods which let compare with a substring of input
     const String name_only = name.Left(argnum_at);
-    const String argnum_only = (argnum_at != UINT32_MAX) ? name.Mid(argnum_at + 1) : String();
+    const String argnum_only = name.Mid(argnum_at + 1, append_end - argnum_at - 1);
 
     // Scan the range of possible matches, starting with pure name without appendages.
     // The match logic is this:
@@ -79,32 +76,41 @@ uint32_t ScriptSymbolsMap::GetIndexOfAny(const String &name) const
         // If base name is not matching, then there's no reason to continue the range
         if (try_sym.CompareLeft(name_only, argnum_at) != 0)
             break;
+
         // If the symbol is longer, but there's no separator after base name,
         // then the symbol has a different, longer base name (e.g. "FindChar" vs "FindCharacter")
         if ((try_sym.GetLength() > name_only.GetLength()) && try_sym[name_only.GetLength()] != _appendageSeparator)
             continue;
-        // If the request is without appendage, then choose the first found symbol
-        // which has at least base name matching (it will be exact match if one exists in symbol map)
-        if (argnum_at == String::NoIndex)
+
+        // Search for appendage separators in the matching symbol
+        const size_t sym_argnum_at = std::min(try_sym.GetLength(), try_sym.FindChar(_appendageSeparator, argnum_at));
+        const size_t sym_append_end = try_sym.GetLength();
+
+        //---------------------------------------------------------------------
+        // Check the argnum appendage
+
+        // If the symbol does not have any appendages, then:
+        // - if request does not have any either, that's the exact match;
+        // - otherwise save it as a best match and continue searching;
+        if (sym_argnum_at == try_sym.GetLength())
         {
-            if ((try_sym.GetLength() == name_only.GetLength()) ||
-                _allowMatchExpanded)
-            {
+            if (argnum_at == name.GetLength())
                 return it->second;
-            }
-            break; // exact base-name match would be first in order, so no reason to continue
-        }
-
-        // Second - compare argnum appendage
-        // If the request has appendage, but the symbol does not, then save it as a best match and continue
-        if (try_sym.GetLength() == name_only.GetLength())
-        {
             best_match = it->second;
             continue;
         }
 
+        // If the request is without argnum, then optionally choose the first found symbol
+        // which has at least base name matching
+        if (argnum_at == name.GetLength())
+        {
+            if (_allowMatchExpanded)
+                return it->second;
+            break; // exact base-name match would be first in order, so no reason to continue
+        }
+
         // Compare argnum appendage, and skip on failure
-        if (try_sym.CompareMid(argnum_only, argnum_at + 1) != 0)
+        if ((sym_append_end != append_end) || try_sym.CompareMid(argnum_only, argnum_at + 1) != 0)
             continue;
 
         // Matched whole appendage, found exact match

Engine/script/systemimports.h

@@ -109,9 +109,4 @@ class SystemImports
     ScriptSymbolsMap _lookup;
 };
 
-extern SystemImports simp;
-// This is to register symbols exclusively for plugins, to allow them
-// perform old style unsafe function calls
-extern SystemImports simp_for_plugin;
-
 #endif  // __CC_SYSTEMIMPORTS_H

Engine/test/systemimports_test.cpp

@@ -0,0 +1,71 @@
+//=============================================================================
+//
+// Adventure Game Studio (AGS)
+//
+// Copyright (C) 1999-2011 Chris Jones and 2011-2024 various contributors
+// The full list of copyright holders can be found in the Copyright.txt
+// file, which is part of this source code distribution.
+//
+// The AGS source code is provided under the Artistic License 2.0.
+// A copy of this license can be found in the file License.txt and at
+// https://opensource.org/license/artistic-2-0/
+//
+//=============================================================================
+#include "gtest/gtest.h"
+#include "script/systemimports.h"
+
+using namespace AGS::Common;
+
+ScriptSymbolsMap sym('^', true);
+
+TEST(SystemImports, ScriptSymbolsMap_GetIndexOf) {
+    sym.Add("Function", 0);
+    sym.Add("Function^1", 1);
+    sym.Add("FunctionLong^5", 2);
+
+    ASSERT_EQ(sym.GetIndexOf("Unknown"), UINT32_MAX);
+    ASSERT_EQ(sym.GetIndexOf("Function"), 0);
+    ASSERT_EQ(sym.GetIndexOf("Function^1"), 1);
+    ASSERT_EQ(sym.GetIndexOf("Function^2"), UINT32_MAX);
+    ASSERT_EQ(sym.GetIndexOf("FunctionLong"), UINT32_MAX);
+    ASSERT_EQ(sym.GetIndexOf("FunctionLong^5"), 2);
+}
+
+TEST(SystemImports, ScriptSymbolsMap_GetIndexOfAny) {
+    sym.Add("Func", 0);
+    sym.Add("Function", 1);
+    sym.Add("Function^1", 2);
+    sym.Add("FunctionLong^1", 3);
+    sym.Add("FunctionLong^3", 4);
+    sym.Add("FunctionLong^5", 5);
+    sym.Add("FunctionNoAppendage", 6);
+    sym.Add("FunctionWithLongAppendage^123", 7);
+    sym.Add("FunctionWithLongAppendage^12345", 8);
+
+    ASSERT_EQ(sym.GetIndexOfAny("Unknown"), UINT32_MAX);
+    // Exact matches always have priority
+    ASSERT_EQ(sym.GetIndexOfAny("Function"), 1);
+    ASSERT_EQ(sym.GetIndexOfAny("Function^1"), 2);
+    ASSERT_EQ(sym.GetIndexOfAny("FunctionLong^5"), 5);
+    ASSERT_EQ(sym.GetIndexOfAny("FunctionNoAppendage"), 6);
+    ASSERT_EQ(sym.GetIndexOfAny("FunctionWithLongAppendage^123"), 7);
+    // Request without appendage
+    // - exact match is the one without appendage,
+    // - otherwise, first found match of the base name
+    ASSERT_EQ(sym.GetIndexOfAny("Function"), 1); // "Function" - exact match
+    ASSERT_EQ(sym.GetIndexOfAny("FunctionLong"), 3); // "FunctionLong^1" - first match of the base name
+    ASSERT_EQ(sym.GetIndexOfAny("FunctionNoAppendage"), 6); // "FunctionNoAppendage" - exact match
+    ASSERT_EQ(sym.GetIndexOfAny("FunctionWithLongAppendage"), 7); // "FunctionWithLongAppendage^123" - first match of the base name
+    // Request with appendage
+    // - exact match is the one that matches appendage,
+    // - otherwise, match the one without appendage
+    ASSERT_EQ(sym.GetIndexOfAny("Function^1"), 2); // "Function^1" - exact match
+    ASSERT_EQ(sym.GetIndexOfAny("Function^2"), 1); // "Function" - no appendage match found, best match of the base name
+    ASSERT_EQ(sym.GetIndexOfAny("FunctionLong^0"), UINT32_MAX); // not matching any variant
+    ASSERT_EQ(sym.GetIndexOfAny("FunctionLong^10"), UINT32_MAX); // not matching any variant
+    ASSERT_EQ(sym.GetIndexOfAny("FunctionLong^3"), 4); // "FunctionLong^3" - exact match
+    ASSERT_EQ(sym.GetIndexOfAny("FunctionNoAppendage^5"), 6); // "FunctionNoAppendage" - no appendage match found, best match of the base name
+    ASSERT_EQ(sym.GetIndexOfAny("FunctionWithLongAppendage^1"), UINT32_MAX); // not matching any variant
+    ASSERT_EQ(sym.GetIndexOfAny("FunctionWithLongAppendage^123"), 7); // "FunctionWithLongAppendage^123" - exact match
+    ASSERT_EQ(sym.GetIndexOfAny("FunctionWithLongAppendage^123456"), UINT32_MAX); // not matching any variant
+}

Solutions/Tests.App/Engine.App.Test.vcxproj

@@ -21,8 +21,13 @@
   <ItemGroup>
     <ClCompile Include="..\..\Common\libsrc\googletest\src\gtest-all.cc" />
     <ClCompile Include="..\..\Common\libsrc\googletest\src\gtest_main.cc" />
+    <ClCompile Include="..\..\Common\util\stream.cpp" />
+    <ClCompile Include="..\..\Common\util\string.cpp" />
+    <ClCompile Include="..\..\Common\util\string_compat.c" />
     <ClCompile Include="..\..\Engine\script\script_api.cpp" />
+    <ClCompile Include="..\..\Engine\script\systemimports.cpp" />
     <ClCompile Include="..\..\Engine\test\scsprintf_test.cpp" />
+    <ClCompile Include="..\..\Engine\test\systemimports_test.cpp" />
     <ClCompile Include="..\..\libsrc\allegro\src\allegro.c" />
     <ClCompile Include="..\..\libsrc\allegro\src\unicode.c" />
   </ItemGroup>

Solutions/Tests.App/Engine.App.Test.vcxproj.filters

@@ -19,6 +19,21 @@
     <ClCompile Include="..\..\libsrc\allegro\src\allegro.c">
       <Filter>libsrc\allegro</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\Engine\script\systemimports.cpp">
+      <Filter>Engine</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\Common\util\string.cpp">
+      <Filter>Common</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\Common\util\stream.cpp">
+      <Filter>Common</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\Common\util\string_compat.c">
+      <Filter>Common</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\Engine\test\systemimports_test.cpp">
+      <Filter>Test</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <Filter Include="Common">