From 4890e400d4107ff65531fdcd23842375d70817ec Mon Sep 17 00:00:00 2001 From: Pieter Dewachter Date: Wed, 17 Apr 2024 22:34:47 +0200 Subject: [PATCH 1/3] Fix Lua array detection and order preservation when using toJSON --- .../deathmatch/logic/lua/CLuaArguments.cpp | 51 ++++++++++--------- .../deathmatch/logic/lua/CLuaArguments.cpp | 51 ++++++++++--------- 2 files changed, 56 insertions(+), 46 deletions(-) diff --git a/Client/mods/deathmatch/logic/lua/CLuaArguments.cpp b/Client/mods/deathmatch/logic/lua/CLuaArguments.cpp index c93cc86bb4b..606f3f88437 100644 --- a/Client/mods/deathmatch/logic/lua/CLuaArguments.cpp +++ b/Client/mods/deathmatch/logic/lua/CLuaArguments.cpp @@ -535,9 +535,9 @@ json_object* CLuaArguments::WriteTableToJSONObject(bool bSerialize, CFastHashMap pKnownTables->insert(std::make_pair(this, pKnownTables->size())); - bool bIsArray = true; - unsigned int iArrayPos = 1; // lua arrays are 1 based - vector::const_iterator iter = m_Arguments.begin(); + bool bIsArray = true; + vector> vecSortedArguments; // lua arrays are not necessarily sorted + vector::const_iterator iter = m_Arguments.begin(); for (; iter != m_Arguments.end(); iter += 2) { CLuaArgument* pArgument = *iter; @@ -545,36 +545,41 @@ json_object* CLuaArguments::WriteTableToJSONObject(bool bSerialize, CFastHashMap { double num = pArgument->GetNumber(); unsigned int iNum = static_cast(num); - if (num == iNum) - { - if (iArrayPos != iNum) // check if the value matches its index in the table - { - bIsArray = false; - break; - } - } - else - { - bIsArray = false; - break; - } + + vecSortedArguments.push_back({iNum, *(iter + 1)}); } else { bIsArray = false; break; } - iArrayPos++; } - if (bIsArray) + if (bIsArray) // the table could possibly be an array { - json_object* my_array = json_object_new_array(); - vector::const_iterator iter = m_Arguments.begin(); - for (; iter != m_Arguments.end(); iter++) + // sort the table based on the keys (already handled correctly by std::pair) + std::sort(vecSortedArguments.begin(), vecSortedArguments.end()); + + // only the first and last element are checked, everything else is correct by default because the vector was sorted + unsigned int const iFirstKey = vecSortedArguments.front().first; + unsigned int const iLastKey = vecSortedArguments.back().first; + + unsigned int const iFirstArrayPos = 1; // lua arrays are 1 based + unsigned int const iLastArrayPos = static_cast(vecSortedArguments.size()); + + if (vecSortedArguments.empty() || iFirstKey != iFirstArrayPos || iLastKey != iLastArrayPos) { - iter++; // skip the key values - CLuaArgument* pArgument = *iter; + bIsArray = vecSortedArguments.empty(); // an empty table is also considered an array + } + } + + if (bIsArray) // the table is definitely an array + { + json_object* my_array = json_object_new_array(); + vector>::const_iterator iter = vecSortedArguments.begin(); + for (; iter != vecSortedArguments.end(); ++iter) + { + CLuaArgument* pArgument = iter->second; // first is key, second is value json_object* object = pArgument->WriteToJSONObject(bSerialize, pKnownTables); if (object) { diff --git a/Server/mods/deathmatch/logic/lua/CLuaArguments.cpp b/Server/mods/deathmatch/logic/lua/CLuaArguments.cpp index 3feadfeedb3..7aca41a1499 100644 --- a/Server/mods/deathmatch/logic/lua/CLuaArguments.cpp +++ b/Server/mods/deathmatch/logic/lua/CLuaArguments.cpp @@ -611,9 +611,9 @@ json_object* CLuaArguments::WriteTableToJSONObject(bool bSerialize, CFastHashMap pKnownTables->insert(std::make_pair(this, pKnownTables->size())); - bool bIsArray = true; - unsigned int iArrayPos = 1; // lua arrays are 1 based - vector::const_iterator iter = m_Arguments.begin(); + bool bIsArray = true; + vector> vecSortedArguments; // lua arrays are not necessarily sorted + vector::const_iterator iter = m_Arguments.begin(); for (; iter != m_Arguments.end(); iter += 2) { CLuaArgument* pArgument = *iter; @@ -621,36 +621,41 @@ json_object* CLuaArguments::WriteTableToJSONObject(bool bSerialize, CFastHashMap { double num = pArgument->GetNumber(); unsigned int iNum = static_cast(num); - if (num == iNum) - { - if (iArrayPos != iNum) // check if the value matches its index in the table - { - bIsArray = false; - break; - } - } - else - { - bIsArray = false; - break; - } + + vecSortedArguments.push_back({iNum, *(iter + 1)}); } else { bIsArray = false; break; } - iArrayPos++; } - if (bIsArray) + if (bIsArray) // the table could possibly be an array { - json_object* my_array = json_object_new_array(); - vector::const_iterator iter = m_Arguments.begin(); - for (; iter != m_Arguments.end(); ++iter) + // sort the table based on the keys (already handled correctly by std::pair) + std::sort(vecSortedArguments.begin(), vecSortedArguments.end()); + + // only the first and last element are checked, everything else is correct by default because the vector was sorted + unsigned int const iFirstKey = vecSortedArguments.front().first; + unsigned int const iLastKey = vecSortedArguments.back().first; + + unsigned int const iFirstArrayPos = 1; // lua arrays are 1 based + unsigned int const iLastArrayPos = static_cast(vecSortedArguments.size()); + + if (vecSortedArguments.empty() || iFirstKey != iFirstArrayPos || iLastKey != iLastArrayPos) { - iter++; // skip the key values - CLuaArgument* pArgument = *iter; + bIsArray = vecSortedArguments.empty(); // an empty table is also considered an array + } + } + + if (bIsArray) // the table is definitely an array + { + json_object* my_array = json_object_new_array(); + vector>::const_iterator iter = vecSortedArguments.begin(); + for (; iter != vecSortedArguments.end(); ++iter) + { + CLuaArgument* pArgument = iter->second; // first is key, second is value json_object* object = pArgument->WriteToJSONObject(bSerialize, pKnownTables); if (object) { From 9060f2f42ba64066885f4bc12111b05f0a58ea17 Mon Sep 17 00:00:00 2001 From: Pieter Dewachter Date: Thu, 18 Apr 2024 20:24:19 +0200 Subject: [PATCH 2/3] Improve code style and make sure an empty vector is correctly handled --- .../deathmatch/logic/lua/CLuaArguments.cpp | 34 +++++++++---------- .../deathmatch/logic/lua/CLuaArguments.cpp | 34 +++++++++---------- 2 files changed, 32 insertions(+), 36 deletions(-) diff --git a/Client/mods/deathmatch/logic/lua/CLuaArguments.cpp b/Client/mods/deathmatch/logic/lua/CLuaArguments.cpp index 606f3f88437..1aaffa1d930 100644 --- a/Client/mods/deathmatch/logic/lua/CLuaArguments.cpp +++ b/Client/mods/deathmatch/logic/lua/CLuaArguments.cpp @@ -533,18 +533,18 @@ json_object* CLuaArguments::WriteTableToJSONObject(bool bSerialize, CFastHashMap bKnownTablesCreated = true; } - pKnownTables->insert(std::make_pair(this, pKnownTables->size())); + pKnownTables->insert({this, pKnownTables->size()}); - bool bIsArray = true; - vector> vecSortedArguments; // lua arrays are not necessarily sorted - vector::const_iterator iter = m_Arguments.begin(); + bool bIsArray = true; + std::vector> vecSortedArguments; // lua arrays are not necessarily sorted + std::vector::const_iterator iter = m_Arguments.begin(); for (; iter != m_Arguments.end(); iter += 2) { CLuaArgument* pArgument = *iter; if (pArgument->GetType() == LUA_TNUMBER) { - double num = pArgument->GetNumber(); - unsigned int iNum = static_cast(num); + double const num = pArgument->GetNumber(); + auto const iNum = static_cast(num); vecSortedArguments.push_back({iNum, *(iter + 1)}); } @@ -555,32 +555,30 @@ json_object* CLuaArguments::WriteTableToJSONObject(bool bSerialize, CFastHashMap } } - if (bIsArray) // the table could possibly be an array + if (bIsArray && !vecSortedArguments.empty()) // the table could possibly be an array { // sort the table based on the keys (already handled correctly by std::pair) std::sort(vecSortedArguments.begin(), vecSortedArguments.end()); // only the first and last element are checked, everything else is correct by default because the vector was sorted - unsigned int const iFirstKey = vecSortedArguments.front().first; - unsigned int const iLastKey = vecSortedArguments.back().first; + auto const iFirstKey = vecSortedArguments.front().first; + auto const iLastKey = vecSortedArguments.back().first; - unsigned int const iFirstArrayPos = 1; // lua arrays are 1 based - unsigned int const iLastArrayPos = static_cast(vecSortedArguments.size()); + auto const iFirstArrayPos = 1U; // lua arrays are 1 based + auto const iLastArrayPos = static_cast(vecSortedArguments.size()); - if (vecSortedArguments.empty() || iFirstKey != iFirstArrayPos || iLastKey != iLastArrayPos) + if (iFirstKey != iFirstArrayPos || iLastKey != iLastArrayPos) { - bIsArray = vecSortedArguments.empty(); // an empty table is also considered an array + bIsArray = false; } } if (bIsArray) // the table is definitely an array { - json_object* my_array = json_object_new_array(); - vector>::const_iterator iter = vecSortedArguments.begin(); - for (; iter != vecSortedArguments.end(); ++iter) + json_object* my_array = json_object_new_array(); + for (auto const& [iKey, pArgument] : vecSortedArguments) { - CLuaArgument* pArgument = iter->second; // first is key, second is value - json_object* object = pArgument->WriteToJSONObject(bSerialize, pKnownTables); + json_object* object = pArgument->WriteToJSONObject(bSerialize, pKnownTables); if (object) { json_object_array_add(my_array, object); diff --git a/Server/mods/deathmatch/logic/lua/CLuaArguments.cpp b/Server/mods/deathmatch/logic/lua/CLuaArguments.cpp index 7aca41a1499..859bf3e0c27 100644 --- a/Server/mods/deathmatch/logic/lua/CLuaArguments.cpp +++ b/Server/mods/deathmatch/logic/lua/CLuaArguments.cpp @@ -609,18 +609,18 @@ json_object* CLuaArguments::WriteTableToJSONObject(bool bSerialize, CFastHashMap bKnownTablesCreated = true; } - pKnownTables->insert(std::make_pair(this, pKnownTables->size())); + pKnownTables->insert({this, pKnownTables->size()}); - bool bIsArray = true; - vector> vecSortedArguments; // lua arrays are not necessarily sorted - vector::const_iterator iter = m_Arguments.begin(); + bool bIsArray = true; + std::vector> vecSortedArguments; // lua arrays are not necessarily sorted + std::vector::const_iterator iter = m_Arguments.begin(); for (; iter != m_Arguments.end(); iter += 2) { CLuaArgument* pArgument = *iter; if (pArgument->GetType() == LUA_TNUMBER) { - double num = pArgument->GetNumber(); - unsigned int iNum = static_cast(num); + double const num = pArgument->GetNumber(); + auto const iNum = static_cast(num); vecSortedArguments.push_back({iNum, *(iter + 1)}); } @@ -631,32 +631,30 @@ json_object* CLuaArguments::WriteTableToJSONObject(bool bSerialize, CFastHashMap } } - if (bIsArray) // the table could possibly be an array + if (bIsArray && !vecSortedArguments.empty()) // the table could possibly be an array { // sort the table based on the keys (already handled correctly by std::pair) std::sort(vecSortedArguments.begin(), vecSortedArguments.end()); // only the first and last element are checked, everything else is correct by default because the vector was sorted - unsigned int const iFirstKey = vecSortedArguments.front().first; - unsigned int const iLastKey = vecSortedArguments.back().first; + auto const iFirstKey = vecSortedArguments.front().first; + auto const iLastKey = vecSortedArguments.back().first; - unsigned int const iFirstArrayPos = 1; // lua arrays are 1 based - unsigned int const iLastArrayPos = static_cast(vecSortedArguments.size()); + auto const iFirstArrayPos = 1U; // lua arrays are 1 based + auto const iLastArrayPos = static_cast(vecSortedArguments.size()); - if (vecSortedArguments.empty() || iFirstKey != iFirstArrayPos || iLastKey != iLastArrayPos) + if (iFirstKey != iFirstArrayPos || iLastKey != iLastArrayPos) { - bIsArray = vecSortedArguments.empty(); // an empty table is also considered an array + bIsArray = false; } } if (bIsArray) // the table is definitely an array { - json_object* my_array = json_object_new_array(); - vector>::const_iterator iter = vecSortedArguments.begin(); - for (; iter != vecSortedArguments.end(); ++iter) + json_object* my_array = json_object_new_array(); + for (auto const& [iKey, pArgument] : vecSortedArguments) { - CLuaArgument* pArgument = iter->second; // first is key, second is value - json_object* object = pArgument->WriteToJSONObject(bSerialize, pKnownTables); + json_object* object = pArgument->WriteToJSONObject(bSerialize, pKnownTables); if (object) { json_object_array_add(my_array, object); From 81e4133bca4caeb21fa4cd83594bb4cac9971cb5 Mon Sep 17 00:00:00 2001 From: Pieter Dewachter Date: Thu, 2 May 2024 21:10:05 +0200 Subject: [PATCH 3/3] Add an extra comment to clarify the detection method of an array --- Client/mods/deathmatch/logic/lua/CLuaArguments.cpp | 1 + Server/mods/deathmatch/logic/lua/CLuaArguments.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/Client/mods/deathmatch/logic/lua/CLuaArguments.cpp b/Client/mods/deathmatch/logic/lua/CLuaArguments.cpp index 1aaffa1d930..3da7cf697ec 100644 --- a/Client/mods/deathmatch/logic/lua/CLuaArguments.cpp +++ b/Client/mods/deathmatch/logic/lua/CLuaArguments.cpp @@ -561,6 +561,7 @@ json_object* CLuaArguments::WriteTableToJSONObject(bool bSerialize, CFastHashMap std::sort(vecSortedArguments.begin(), vecSortedArguments.end()); // only the first and last element are checked, everything else is correct by default because the vector was sorted + // the last key should match the size of vecSortedArguments to ensure there are no gaps in this array-like table auto const iFirstKey = vecSortedArguments.front().first; auto const iLastKey = vecSortedArguments.back().first; diff --git a/Server/mods/deathmatch/logic/lua/CLuaArguments.cpp b/Server/mods/deathmatch/logic/lua/CLuaArguments.cpp index 859bf3e0c27..200c9e0d7c5 100644 --- a/Server/mods/deathmatch/logic/lua/CLuaArguments.cpp +++ b/Server/mods/deathmatch/logic/lua/CLuaArguments.cpp @@ -637,6 +637,7 @@ json_object* CLuaArguments::WriteTableToJSONObject(bool bSerialize, CFastHashMap std::sort(vecSortedArguments.begin(), vecSortedArguments.end()); // only the first and last element are checked, everything else is correct by default because the vector was sorted + // the last key should match the size of vecSortedArguments to ensure there are no gaps in this array-like table auto const iFirstKey = vecSortedArguments.front().first; auto const iLastKey = vecSortedArguments.back().first;