Skip to content

bugfix(ThingFactory): Return the final override of the ThingTemplate. #3445

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DrGoldFish1 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

bugfix(ThingFactory): Return the final override of the ThingTemplate. #3445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions Generals/Code/GameEngine/Source/Common/Thing/ThingFactory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,14 @@ const ThingTemplate *ThingFactory::findByTemplateID( UnsignedShort id )
for (ThingTemplate *tmpl = m_firstTemplate; tmpl; tmpl = tmpl->friend_getNextTemplate())
{
if (tmpl->getTemplateID() == id)
{
#if RETAIL_COMPATIBLE_SCRIPTING
return tmpl;
#else
// TheSuperHackers @bugfix DrGoldFish 08/10/2026 Return the final override of the ThingTemplate.
return (ThingTemplate*)tmpl->getFinalOverride();
#endif
}
}
DEBUG_CRASH(("template %d not found",(Int)id));
return nullptr;
Expand All @@ -276,7 +283,12 @@ ThingTemplate *ThingFactory::findTemplateInternal( const AsciiString& name, Bool
ThingTemplateHashMapIt tIt = m_templateHashMap.find(name);

if (tIt != m_templateHashMap.end()) {
#if RETAIL_COMPATIBLE_SCRIPTING
return tIt->second;
#else
// TheSuperHackers @bugfix DrGoldFish 08/10/2026 Return the final override of the ThingTemplate.
return (ThingTemplate*)tIt->second->getFinalOverride();
Comment on lines +289 to +290

@coderabbitai coderabbitai Bot Oct 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

set -o pipefail
printf '%s\n' '--- changed files and hunks ---'
git diff --stat 0e6a06c7c94f73c56de1f2c4d8e619deb12b8402 9600fab50dcd84bdad5cc28462471f0a354a561a
git diff -- Generals/Code/GameEngine/Source/Common/Thing/ThingFactory.cpp GeneralsMD/Code/GameEngine/Source/Common/Thing/ThingFactory.cpp 0e6a06c7c94f73c56de1f2c4d8e619deb12b8402 9600fab50dcd84bdad5cc28462471f0a354a561a
printf '%s\n' '--- cost definitions and references ---'
rg -n -F --glob '*.{cpp,h}' -- 'calcCostToBuild' Generals/Code/GameEngine/Source GeneralsMD/Code/GameEngine/Source || test "$?" -eq 1
printf '%s\n' '--- template lookup references ---'
rg -n -F --glob '*.{cpp,h}' -- 'findTemplate' Generals/Code/GameEngine/Source GeneralsMD/Code/GameEngine/Source || test "$?" -eq 1
printf '%s\n' '--- production queue restore terms ---'
rg -n -i --glob '*.{cpp,h}' -- 'production.*(restore|load|save)|restore.*production|production.*queue|queue.*production' Generals/Code/GameEngine/Source GeneralsMD/Code/GameEngine/Source || test "$?" -eq 1

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 41019

🏁 Script executed:

set -o pipefail
printf '%s\n' '--- all cost-method definitions and declarations ---'
rg -n -F -- 'calcCostToBuild' Generals GeneralsMD || test "$?" -eq 1
printf '%s\n' '--- ThingTemplate declarations and override APIs ---'
rg -n -F -- 'class ThingTemplate' Generals/Code/GameEngine/Source GeneralsMD/Code/GameEngine/Source || test "$?" -eq 1
rg -n -i --glob '*.{cpp,h}' -- 'finaloverride|getFinalOverride|override' Generals/Code/GameEngine/Source/Common/Thing GeneralsMD/Code/GameEngine/Source/Common/Thing || test "$?" -eq 1
printf '%s\n' '--- production queue serialization and load functions ---'
for f in Generals/Code/GameEngine/Source/GameLogic/Object/Update/ProductionUpdate.cpp GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/ProductionUpdate.cpp; do
  echo "### $f"
  nl -ba "$f" | sed -n '250,450p;1030,1120p;1240,1460p'
done
printf '%s\n' '--- representative AI and Player cost callers ---'
for f in Generals/Code/GameEngine/Source/GameLogic/AI/AIPlayer.cpp GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPlayer.cpp Generals/Code/GameEngine/Source/Common/RTS/Player.cpp GeneralsMD/Code/GameEngine/Source/Common/RTS/Player.cpp; do
  echo "### $f"
  nl -ba "$f" | sed -n '130,170p;500,535p;700,730p;810,835p;1260,1305p;1360,1390p;1465,1510p;1580,1660p;2345,2380p;2445,2480p;2785,2830p;2975,3010p'
done

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 42895

🏁 Script executed:

set -o pipefail
printf '%s\n' '--- changed diff ---'
git diff 0e6a06c7c94f73c56de1f2c4d8e619deb12b8402 9600fab50dcd84bdad5cc28462471f0a354a561a -- Generals/Code/GameEngine/Source/Common/Thing/ThingFactory.cpp GeneralsMD/Code/GameEngine/Source/Common/Thing/ThingFactory.cpp
printf '%s\n' '--- template files and cost-related identifiers ---'
rg --files Generals/Code/GameEngine GeneralsMD/Code/GameEngine | rg '/(Thing|Object|Build|Template|Overrid).*\.(h|cpp)$' | head -200
rg -n -i --glob '*.{h,cpp}' -- 'calcCost|costToBuild|buildCost|m_cost|m_build' Generals/Code/GameEngine/Include/Common Generals/Code/GameEngine/Source/Common/Thing GeneralsMD/Code/GameEngine/Include/Common GeneralsMD/Code/GameEngine/Source/Common/Thing || test "$?" -eq 1
printf '%s\n' '--- findTemplate declarations and implementation context ---'
nl -ba Generals/Code/GameEngine/Include/Common/ThingFactory.h | sed -n '1,180p'
nl -ba Generals/Code/GameEngine/Source/Common/Thing/ThingFactory.cpp | sed -n '250,315p'
printf '%s\n' '--- exact queue cancel/complete/load blocks ---'
for f in Generals/Code/GameEngine/Source/GameLogic/Object/Update/ProductionUpdate.cpp GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/ProductionUpdate.cpp; do
  echo "### $f"
  nl -ba "$f" | sed -n '430,680p;800,875p;1040,1110p;1245,1455p'
done

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 42461

🏁 Script executed:

set -o pipefail
printf '%s\n' '--- ThingTemplate symbols and cost ownership ---'
rg -n -F -- 'ThingTemplate' Generals/Code/GameEngine/Include Generals/Code/GameEngine/Source GeneralsMD/Code/GameEngine/Include GeneralsMD/Code/GameEngine/Source | rg 'class |struct |typedef |calcCostToBuild|calcTimeToBuild|getFinalOverride|friend_getFinalOverride' || test "$?" -eq 1
rg -n -F -- 'calcCostToBuild' . --glob '!*.log' || test "$?" -eq 1
printf '%s\n' '--- queueCreateUnit callers ---'
rg -n -F -- 'queueCreateUnit(' Generals/Code/GameEngine/Source GeneralsMD/Code/GameEngine/Source || test "$?" -eq 1
printf '%s\n' '--- AI plan acquisition and queue calls ---'
for f in Generals/Code/GameEngine/Source/GameLogic/AI/AIPlayer.cpp GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPlayer.cpp Generals/Code/GameEngine/Source/GameLogic/AI/AISkirmishPlayer.cpp GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AISkirmishPlayer.cpp; do
  echo "### $f"
  rg -n -C 8 -F -- 'queueCreateUnit' "$f" || true
  rg -n -C 5 -F -- 'bldgPlan = TheThingFactory->findTemplate' "$f" || true
  rg -n -C 5 -F -- 'thing = TheThingFactory->findTemplate' "$f" || true
done
printf '%s\n' '--- production message handlers and template resolution ---'
rg -n -C 8 -i --glob '*.{cpp,h}' -- 'MSG_QUEUE_(UNIT|UPGRADE)|findByTemplateID|findTemplateInternal' Generals/Code/GameEngine/Source/GameLogic GeneralsMD/Code/GameEngine/Source/GameLogic || test "$?" -eq 1

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 34331

🏁 Script executed:

set -o pipefail
printf '%s\n' '--- ThingTemplate cost and override implementation ---'
nl -ba Core/GameEngine/Include/Common/ThingTemplate.h | sed -n '535,650p'
nl -ba Core/GameEngine/Source/Common/Thing/ThingTemplate.cpp | sed -n '1540,1595p'
rg -n -F -- 'getFinalOverride' Core/GameEngine/Include Core/GameEngine/Source | head -50
printf '%s\n' '--- all WorkOrder thing assignments and startTraining declarations/callers ---'
rg -n -C 4 -F -- 'm_thing' Generals/Code/GameEngine/Source/GameLogic/AI GeneralsMD/Code/GameEngine/Source/GameLogic/AI | rg -E 'm_thing|startTraining|WorkOrder' || true
rg -n -C 8 -F -- 'startTraining(' Generals/Code/GameEngine/Source/GameLogic/AI GeneralsMD/Code/GameEngine/Source/GameLogic/AI | head -240
printf '%s\n' '--- direct queue callers and template member initialization ---'
for f in Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/FlightDeckBehavior.cpp; do
  echo "### $f"
  rg -n -C 12 -F -- 'queueCreateUnit' "$f" || true
  rg -n -C 8 -F -- 'm_thingTemplate' "$f" || true
done

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 39078

🏁 Script executed:

set -o pipefail
printf '%s\n' '--- WorkOrder construction and assignments ---'
rg -n -C 5 --glob '*.{cpp,h}' 'WorkOrder|m_thing[[:space:]]*=' Generals/Code/GameEngine/Source/GameLogic/AI GeneralsMD/Code/GameEngine/Source/GameLogic/AI Core/GameEngine/Source/GameLogic/AI || test "$?" -eq 1
printf '%s\n' '--- AI template pointer handoff around all order creation ---'
for f in Generals/Code/GameEngine/Source/GameLogic/AI/AIPlayer.cpp GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPlayer.cpp Generals/Code/GameEngine/Source/GameLogic/AI/AISkirmishPlayer.cpp GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AISkirmishPlayer.cpp; do
  echo "### $f"
  rg -n -C 10 -- 'newInstance\(WorkOrder\)|m_thing[[:space:]]*=' "$f" || true
done

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 41569

🏁 Script executed:

set -o pipefail
printf '%s\n' '--- firstTemplate binding ---'
rg -n -C 5 -F -- 'firstTemplate()' Generals/Code/GameEngine/Include/Common/ThingFactory.h Generals/Code/GameEngine/Source/Common/Thing/ThingFactory.cpp GeneralsMD/Code/GameEngine/Include/Common/ThingFactory.h GeneralsMD/Code/GameEngine/Source/Common/Thing/ThingFactory.cpp

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 2271

Resolve final overrides in ThingTemplate::calcCostToBuild().

firstTemplate() returns m_firstTemplate without resolving overrides. Both AI variants assign this pointer to WorkOrder::m_thing for supply trucks and dozers, then pass it to queueCreateUnit(). That function calls unitType->calcCostToBuild(player), which reads the base template’s m_buildCost. AI purchases and their refunds can therefore use the base cost instead of the final map.ini or solo.ini cost.

Make calcCostToBuild() use the final override before reading the cost and applying player modifiers. The restored-queue path already resolves the name through findTemplate() during load.

📍 Affects 2 files
  • Generals/Code/GameEngine/Source/Common/Thing/ThingFactory.cpp#L289-L290 (this comment)
  • GeneralsMD/Code/GameEngine/Source/Common/Thing/ThingFactory.cpp#L289-L290

#endif
}

#ifdef LOAD_TEST_ASSETS
Expand Down
12 changes: 12 additions & 0 deletions GeneralsMD/Code/GameEngine/Source/Common/Thing/ThingFactory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,14 @@ const ThingTemplate *ThingFactory::findByTemplateID( UnsignedShort id )
for (ThingTemplate *tmpl = m_firstTemplate; tmpl; tmpl = tmpl->friend_getNextTemplate())
{
if (tmpl->getTemplateID() == id)
{
#if RETAIL_COMPATIBLE_SCRIPTING
return tmpl;
#else
// TheSuperHackers @bugfix DrGoldFish 08/10/2026 Return the final override of the ThingTemplate.
return (ThingTemplate*)tmpl->getFinalOverride();
#endif
}
}
DEBUG_CRASH(("template %d not found",(Int)id));
return nullptr;
Expand All @@ -276,7 +283,12 @@ ThingTemplate *ThingFactory::findTemplateInternal( const AsciiString& name, Bool
ThingTemplateHashMapIt tIt = m_templateHashMap.find(name);

if (tIt != m_templateHashMap.end()) {
#if RETAIL_COMPATIBLE_SCRIPTING
return tIt->second;
#else
// TheSuperHackers @bugfix DrGoldFish 08/10/2026 Return the final override of the ThingTemplate.
return (ThingTemplate*)tIt->second->getFinalOverride();

@greptile-apps greptile-apps Bot Oct 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Bridge cache keeps deleted templates

findTemplateInternal now returns an override that reset can delete. Bridge::Bridge stores its first findTemplate("GenericBridge") result in a static pointer. If the first bridge map overrides GenericBridge, ThingFactory::reset deletes that cached template. The next bridge map then passes the freed pointer to newObject, which can crash or read invalid data.

Keep name lookups returning the base template, or update persistent callers before changing this behavior. Both game versions have this change.

#endif
}

#ifdef LOAD_TEST_ASSETS
Expand Down
Loading