Skip to content

Conversation

@bobtista
Copy link

@bobtista bobtista commented Jan 21, 2026

Summary

Fixes weapon timing state corruption after loading a saved game by ensuring WeaponSet uses the same template override as Object.

Notes

  • Object constructor calls getFinalOverride() on the template (Object.cpp:230)
  • WeaponSet::xfer(LOAD) was using TheThingFactory->findTemplate() which returns the base template, not the final override
  • This caused m_curWeaponTemplateSet to point to a different ThingTemplate's weapon sets than obj->getTemplate()
  • The pointer mismatch in updateWeaponSet() triggered unnecessary weapon reallocation, resetting timing state

Fix

Add getFinalOverride() call in WeaponSet::xfer(LOAD) to match what Object does, ensuring consistent template pointers.

Testing

  • Save game with units that have weapons mid-reload
  • Load the save and verify weapons maintain their reload state
  • Verify normal weapon switching still works correctly

Todo

  • Replicate to Generals

…g WeaponSet flags instead of pointer

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@greptile-apps
Copy link

greptile-apps bot commented Jan 21, 2026

Greptile Overview

Greptile Summary

Fixes weapon timing state corruption after loading saved games by ensuring WeaponSet::xfer(LOAD) uses getFinalOverride() to match the template resolution behavior in Object constructor.

Key Changes:

  • Added getFinalOverride() call after findTemplate() in WeaponSet::xfer(LOAD) (both Generals and GeneralsMD)
  • Uses static_cast<const ThingTemplate*> to convert the result back to the correct type
  • Ensures m_curWeaponTemplateSet points to the same ThingTemplate as obj->getTemplate()

Root Cause:

  • Object constructor calls getFinalOverride() on the template (Object.cpp:211) to get the most overridden version
  • WeaponSet::xfer(LOAD) was using findTemplate() which returns the base template
  • Pointer mismatch triggered unnecessary weapon reallocation in updateWeaponSet() (WeaponSet.cpp:296)
  • Reallocation reset weapon timing state (reload progress, etc.)

Impact:
This fix prevents weapons from losing their mid-reload state when loading a saved game, maintaining gameplay consistency.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The fix directly addresses a pointer consistency issue by mirroring the exact template resolution logic used in Object constructor (verified at Object.cpp:211). The change is minimal (4 lines), uses safe casting, is identical across both codebase variants (Generals/GeneralsMD), and has comprehensive inline documentation explaining the rationale
  • No files require special attention

Important Files Changed

Filename Overview
Generals/Code/GameEngine/Source/GameLogic/Object/WeaponSet.cpp Adds getFinalOverride() call in xfer(LOAD) to match Object constructor behavior, ensuring consistent template pointers and preventing weapon timing corruption
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/WeaponSet.cpp Adds getFinalOverride() call in xfer(LOAD) to match Object constructor behavior, ensuring consistent template pointers and preventing weapon timing corruption

Sequence Diagram

sequenceDiagram
    participant SG as SaveGame Load
    participant WS as WeaponSet::xfer(LOAD)
    participant TF as ThingFactory
    participant TT as ThingTemplate
    participant Obj as Object Constructor
    participant UWS as updateWeaponSet

    Note over SG,UWS: Game Load Sequence

    SG->>Obj: Create Object(tt)
    Obj->>TT: getFinalOverride()
    TT-->>Obj: Final overridden template
    Note over Obj: Stores final template

    SG->>WS: Load weapon state
    WS->>TF: findTemplate(ttName)
    TF-->>WS: Base template
    WS->>TT: getFinalOverride()
    TT-->>WS: Final overridden template
    Note over WS: Now matches Object's template
    WS->>WS: Set m_curWeaponTemplateSet

    Note over Obj,UWS: Later During Gameplay

    Obj->>UWS: Update weapons
    UWS->>Obj: getTemplate()
    Obj-->>UWS: Final template pointer
    UWS->>UWS: Compare pointers
    alt Pointers Match
        UWS-->>Obj: No reallocation needed
        Note over UWS: Weapon timing preserved
    else Pointers Differ (Bug)
        UWS->>UWS: Reallocate all weapons
        Note over UWS: Timing state lost!
    end
Loading

const WeaponTemplateSet* set = obj->getTemplate()->findWeaponTemplateSet(obj->getWeaponSetFlags());
DEBUG_ASSERTCRASH(set, ("findWeaponSet should never return null"));
if (set && set != m_curWeaponTemplateSet)
// TheSuperHackers @bugfix bobtista 20/01/2026 After checkpoint load, the m_curWeaponTemplateSet pointer
Copy link

Choose a reason for hiding this comment

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

My first hunch here is that this change is a hack. Why is m_curWeaponTemplateSet set to something that satisfies the weapon flags but is not actually the real deal? It indicates that the issue is higher up.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, updated with a better approach.
When an Object is created (Object.cpp:230):
tt = (const ThingTemplate*)tt->getFinalOverride();
The Object uses the final override of the template.

But when WeaponSet::xfer(LOAD) restored weapon data (WeaponSet.cpp:231):
const ThingTemplate* tt = TheThingFactory->findTemplate(ttName);
m_curWeaponTemplateSet = tt->findWeaponTemplateSet(wsFlags);
It used findTemplate() which returns the base template, not the final override.

The fix is to just add t = (const ThingTemplate*)tt->getFinalOverride(); to WeaponSet::xfer(LOAD):

@xezon xezon added Bug Something is not working right, typically is user facing Minor Severity: Minor < Major < Critical < Blocker Investigate Gen Relates to Generals ZH Relates to Zero Hour labels Jan 22, 2026
throw INI_INVALID_DATA;

// TheSuperHackers @bugfix bobtista 23/01/2026 Use final override to match what Object uses.
tt = (const ThingTemplate*)tt->getFinalOverride();

Choose a reason for hiding this comment

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

Is a static_cast better here?

if (tt == nullptr)
throw INI_INVALID_DATA;

// TheSuperHackers @bugfix bobtista 23/01/2026 Use final override to match what Object uses.

Choose a reason for hiding this comment

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

I'm not sure if this comment explains why we need this fix.

Should probably be a fix as there's no user facing issue here.

@Caball009
Copy link

Looks like a good change, but the title and issue description need to be updated (the summary is no longer in line with the current code change).

@bobtista bobtista changed the title bugfix(savegame): Fix weapon timing corruption after load by comparing WeaponSet flags instead of pointer fix(savegame): Use getFinalOverride in WeaponSet xfer load to match Object constructor Jan 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something is not working right, typically is user facing Gen Relates to Generals Investigate Minor Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants