From 2d0167f2ba89d801fdb495d9322151913c332b0a Mon Sep 17 00:00:00 2001 From: Ivan Mogilko Date: Tue, 24 Dec 2024 21:43:43 +0300 Subject: [PATCH 1/2] Tests: add SystemImports test --- Engine/CMakeLists.txt | 2 + Engine/script/script_runtime.cpp | 4 ++ Engine/script/script_runtime.h | 6 ++ Engine/script/systemimports.cpp | 4 -- Engine/script/systemimports.h | 5 -- Engine/test/systemimports_test.cpp | 71 +++++++++++++++++++ Solutions/Tests.App/Engine.App.Test.vcxproj | 5 ++ .../Tests.App/Engine.App.Test.vcxproj.filters | 15 ++++ 8 files changed, 103 insertions(+), 9 deletions(-) create mode 100644 Engine/test/systemimports_test.cpp diff --git a/Engine/CMakeLists.txt b/Engine/CMakeLists.txt index 03c9880aa33..85013c611a1 100644 --- a/Engine/CMakeLists.txt +++ b/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 ) diff --git a/Engine/script/script_runtime.cpp b/Engine/script/script_runtime.cpp index da26d189924..9dc79aceee4 100644 --- a/Engine/script/script_runtime.cpp +++ b/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 && diff --git a/Engine/script/script_runtime.h b/Engine/script/script_runtime.h index 0dac5af418f..06073c97115 100644 --- a/Engine/script/script_runtime.h +++ b/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 diff --git a/Engine/script/systemimports.cpp b/Engine/script/systemimports.cpp index 5bfeeaadd9b..fc75a18eaea 100644 --- a/Engine/script/systemimports.cpp +++ b/Engine/script/systemimports.cpp @@ -14,13 +14,9 @@ #include "script/systemimports.h" #include #include -#include "script/cc_instance.h" using namespace AGS::Common; -SystemImports simp; -SystemImports simp_for_plugin; - void ScriptSymbolsMap::Add(const String &name, uint32_t index) { diff --git a/Engine/script/systemimports.h b/Engine/script/systemimports.h index 40dfb492e5c..151d82ba1f5 100644 --- a/Engine/script/systemimports.h +++ b/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 diff --git a/Engine/test/systemimports_test.cpp b/Engine/test/systemimports_test.cpp new file mode 100644 index 00000000000..10465b4eaa7 --- /dev/null +++ b/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 +} diff --git a/Solutions/Tests.App/Engine.App.Test.vcxproj b/Solutions/Tests.App/Engine.App.Test.vcxproj index e761cf08cb6..1ff848fd323 100644 --- a/Solutions/Tests.App/Engine.App.Test.vcxproj +++ b/Solutions/Tests.App/Engine.App.Test.vcxproj @@ -21,8 +21,13 @@ + + + + + diff --git a/Solutions/Tests.App/Engine.App.Test.vcxproj.filters b/Solutions/Tests.App/Engine.App.Test.vcxproj.filters index 6768f0ec521..a5c4427b4be 100644 --- a/Solutions/Tests.App/Engine.App.Test.vcxproj.filters +++ b/Solutions/Tests.App/Engine.App.Test.vcxproj.filters @@ -19,6 +19,21 @@ libsrc\allegro + + Engine + + + Common + + + Common + + + Common + + + Test + From 3c72bc1bf80bd336c4ca0c11d2bdca7c85772214 Mon Sep 17 00:00:00 2001 From: Ivan Mogilko Date: Tue, 24 Dec 2024 21:58:02 +0300 Subject: [PATCH 2/2] Engine: updated ScriptSymbolsMap::GetIndexOfAny() ...this fixes couple of edge cases. --- Engine/script/systemimports.cpp | 44 ++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/Engine/script/systemimports.cpp b/Engine/script/systemimports.cpp index fc75a18eaea..db702ce96ff 100644 --- a/Engine/script/systemimports.cpp +++ b/Engine/script/systemimports.cpp @@ -52,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: @@ -75,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