From 0113b634a128ddc179d931f411adf812e7b318fb Mon Sep 17 00:00:00 2001 From: Skyaero <21192585+Skyaero42@users.noreply.github.com> Date: Tue, 4 Nov 2025 21:54:38 +0100 Subject: [PATCH 1/2] refactor: Replace manual string prefix logic with safer offset-based handling --- .../Source/GameLogic/ScriptEngine/Scripts.cpp | 8 ++++---- .../WorldBuilder/src/WorldBuilderDoc.cpp | 12 ++++++------ .../Source/GameLogic/ScriptEngine/Scripts.cpp | 8 ++++---- .../WorldBuilder/src/WorldBuilderDoc.cpp | 19 ++++++++++--------- 4 files changed, 24 insertions(+), 23 deletions(-) diff --git a/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp b/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp index e95d33714bc..2bcf5811da9 100644 --- a/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp +++ b/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp @@ -2059,13 +2059,13 @@ Parameter *Parameter::ReadParameter(DataChunkInput &file) // quick hack to make loading models with "Fundamentalist" switch to "GLA" if (pParm->m_string.startsWith("Fundamentalist")) { - char oldName[256]; + const char* replacePrefix = "Fundamentalist"; + const size_t offset = pParm->m_string.startsWith(replacePrefix) ? strlen(replacePrefix) : 0u; char newName[256]; - strcpy(oldName, pParm->m_string.str()); strcpy(newName, "GLA"); - strlcat(newName, oldName+strlen("Fundamentalist"), ARRAY_SIZE(newName)); + strlcat(newName, pParm->m_string.str() + offset, ARRAY_SIZE(newName)); pParm->m_string.set(newName); - DEBUG_LOG(("Changing Script Ref from %s to %s", oldName, newName)); + DEBUG_LOG(("Changing Script Ref from %s to %s", pParm->m_string, newName)); } } diff --git a/Generals/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp b/Generals/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp index b47f959dee9..d2f89e964f7 100644 --- a/Generals/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp +++ b/Generals/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp @@ -422,11 +422,11 @@ void CWorldBuilderDoc::Serialize(CArchive& ar) AsciiString ConvertName(AsciiString name) { - char oldName[256]; + const char* replacePrefix = "Fundamentalist"; + const size_t offset = name.startsWith(replacePrefix) ? strlen(replacePrefix) : 0u; char newName[256]; - strcpy(oldName, name.str()); strcpy(newName, "GLA"); - strlcat(newName, oldName+strlen("Fundamentalist"), ARRAY_SIZE(newName)); + strlcat(newName, name.str() + offset, ARRAY_SIZE(newName)); AsciiString swapName; swapName.set(newName); const ThingTemplate *tt = TheThingFactory->findTemplate(swapName); @@ -438,11 +438,11 @@ AsciiString ConvertName(AsciiString name) AsciiString ConvertFaction(AsciiString name) { - char oldName[256]; + const char* replacePrefix = "FactionFundamentalist"; + const size_t offset = name.startsWith(replacePrefix) ? strlen(replacePrefix) : 0u; char newName[256]; - strcpy(oldName, name.str()); strcpy(newName, "FactionGLA"); - strlcat(newName, oldName+strlen("FactionFundamentalist"), ARRAY_SIZE(newName)); + strlcat(newName, name.str() + offset, ARRAY_SIZE(newName)); AsciiString swapName; swapName.set(newName); const PlayerTemplate* pt = ThePlayerTemplateStore->findPlayerTemplate(NAMEKEY(swapName)); diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp index 2c7057c02e7..cb5501714f1 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp @@ -2134,13 +2134,13 @@ Parameter *Parameter::ReadParameter(DataChunkInput &file) // quick hack to make loading models with "Fundamentalist" switch to "GLA" if (pParm->m_string.startsWith("Fundamentalist")) { - char oldName[256]; + const char* replacePrefix = "Fundamentalist"; + const size_t offset = pParm->m_string.startsWith(replacePrefix) ? strlen(replacePrefix) : 0u; char newName[256]; - strcpy(oldName, pParm->m_string.str()); strcpy(newName, "GLA"); - strlcat(newName, oldName+strlen("Fundamentalist"), ARRAY_SIZE(newName)); + strlcat(newName, pParm->m_string.str() + offset, ARRAY_SIZE(newName)); pParm->m_string.set(newName); - DEBUG_LOG(("Changing Script Ref from %s to %s", oldName, newName)); + DEBUG_LOG(("Changing Script Ref from %s to %s", pParm->m_string, newName)); } } diff --git a/GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp b/GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp index 0dc41750737..dd65b980167 100644 --- a/GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp +++ b/GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp @@ -434,10 +434,10 @@ void CWorldBuilderDoc::Serialize(CArchive& ar) AsciiString ConvertToNonGCName(AsciiString name, Bool checkTemplate=true) { - char oldName[256]; + const char* replacePrefix = "GC_"; + const size_t offset = name.startsWith(replacePrefix) ? strlen(replacePrefix) : 0u; char newName[256]; - strcpy(oldName, name.str()); - strcpy(newName, oldName+strlen("GC_")); + strlcpy(newName, name.str() + offset, ARRAY_SIZE(newName)); AsciiString swapName; swapName.set(newName); if (checkTemplate) @@ -453,11 +453,12 @@ AsciiString ConvertToNonGCName(AsciiString name, Bool checkTemplate=true) AsciiString ConvertName(AsciiString name) { - char oldName[256]; + const char* replacePrefix = "Fundamentalist"; + const size_t offset = name.startsWith(replacePrefix) ? strlen(replacePrefix) : 0u; char newName[256]; - strcpy(oldName, name.str()); strcpy(newName, "GLA"); - strlcat(newName, oldName+strlen("Fundamentalist"), ARRAY_SIZE(newName)); + strlcat(newName, name.str() + offset, ARRAY_SIZE(newName)); + AsciiString swapName; AsciiString swapName; swapName.set(newName); const ThingTemplate *tt = TheThingFactory->findTemplate(swapName); @@ -469,11 +470,11 @@ AsciiString ConvertName(AsciiString name) AsciiString ConvertFaction(AsciiString name) { - char oldName[256]; + const char* replacePrefix = "FactionFundamentalist"; + const size_t offset = name.startsWith(replacePrefix) ? strlen(replacePrefix) : 0u; char newName[256]; - strcpy(oldName, name.str()); strcpy(newName, "FactionGLA"); - strlcat(newName, oldName+strlen("FactionFundamentalist"), ARRAY_SIZE(newName)); + strlcat(newName, name.str() + offset, ARRAY_SIZE(newName)); AsciiString swapName; swapName.set(newName); const PlayerTemplate* pt = ThePlayerTemplateStore->findPlayerTemplate(NAMEKEY(swapName)); From ae7941fc0eca9d40ca7a1b5ca1b2edd3a6f34c15 Mon Sep 17 00:00:00 2001 From: Skyaero <21192585+Skyaero42@users.noreply.github.com> Date: Wed, 5 Nov 2025 09:32:02 +0100 Subject: [PATCH 2/2] Addressing review comments xezon 2025-11-04 --- .../Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp | 2 +- .../Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp | 2 +- GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp b/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp index 2bcf5811da9..fee39f690db 100644 --- a/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp +++ b/Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp @@ -2064,8 +2064,8 @@ Parameter *Parameter::ReadParameter(DataChunkInput &file) char newName[256]; strcpy(newName, "GLA"); strlcat(newName, pParm->m_string.str() + offset, ARRAY_SIZE(newName)); + DEBUG_LOG(("Changing Script Ref from %s to %s", pParm->m_string.str(), newName)); pParm->m_string.set(newName); - DEBUG_LOG(("Changing Script Ref from %s to %s", pParm->m_string, newName)); } } diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp index cb5501714f1..b4a63c4ef9b 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/Scripts.cpp @@ -2139,8 +2139,8 @@ Parameter *Parameter::ReadParameter(DataChunkInput &file) char newName[256]; strcpy(newName, "GLA"); strlcat(newName, pParm->m_string.str() + offset, ARRAY_SIZE(newName)); + DEBUG_LOG(("Changing Script Ref from %s to %s", pParm->m_string.str(), newName)); pParm->m_string.set(newName); - DEBUG_LOG(("Changing Script Ref from %s to %s", pParm->m_string, newName)); } } diff --git a/GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp b/GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp index dd65b980167..24442484d8b 100644 --- a/GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp +++ b/GeneralsMD/Code/Tools/WorldBuilder/src/WorldBuilderDoc.cpp @@ -459,7 +459,6 @@ AsciiString ConvertName(AsciiString name) strcpy(newName, "GLA"); strlcat(newName, name.str() + offset, ARRAY_SIZE(newName)); AsciiString swapName; - AsciiString swapName; swapName.set(newName); const ThingTemplate *tt = TheThingFactory->findTemplate(swapName); if (tt) {