bugfix(savegame): Fix object list order after load

[bobtista]

Summary

• Objects were saved oldest-first but loaded via prepending, which reversed their order
• New saves now write objects in reverse order (newest first) so they load correctly
• Old saves have their object list reversed after loading to fix the order
• Bumped xfer version to 11 (Zero Hour) and 10 (Generals) for backwards compatibility

Test plan

• Load an existing save from before this change, verify objects behave correctly
• Create a new save, reload it, verify object order matches the original session
• Verify game runs normally

Todo:

• Replicate to Generals

[greptile-apps]

Greptile Overview

Greptile Summary

Fixed object list order after save/load by saving objects in reverse order (newest first) so they load correctly when prepended. Old saves are automatically reversed after loading to maintain correct order. Version bumped to 10 (Generals) and 11 (Zero Hour) for backwards compatibility.

Key changes:

• Save loop now traverses backwards through object list using new getPrevObject() accessor
• Load operation reverses object list for old saves (version <= 9 for Generals, <= 10 for Zero Hour)
• Added getPrevObject(), friend_setNextObject(), and friend_setPrevObject() accessors to Object class
• Comprehensive comments explain the fix and version-specific behavior

Confidence Score: 5/5

• Safe to merge with high confidence - elegant solution with proper backwards compatibility
• The implementation correctly addresses the root cause by reversing save order to compensate for prepend behavior. Backwards compatibility is properly handled through version checking and list reversal for old saves. The logic is sound and well-commented.
• No files require special attention

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Include/GameLogic/Object.h	Added accessor methods for previous object pointer and friend setters for list manipulation during save/load reversal
GeneralsMD/Code/GameEngine/Include/GameLogic/Object.h	Added accessor methods for previous object pointer and friend setters for list manipulation during save/load reversal
Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp	Saves objects in reverse order (newest first) and reverses object list after loading old saves to maintain correct order
GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp	Saves objects in reverse order (newest first) and reverses object list after loading old saves to maintain correct order

Sequence Diagram

sequenceDiagram
    participant Game
    participant GameLogic
    participant Xfer
    participant ObjectList
    
    alt XFER_SAVE (New Version 10/11)
        Game->>GameLogic: xfer(XFER_SAVE)
        GameLogic->>ObjectList: Find last object via getNextObject()
        loop For each object (newest to oldest)
            GameLogic->>ObjectList: Traverse backwards via getPrevObject()
            GameLogic->>Xfer: Write object data (newest first)
        end
    else XFER_LOAD (Old Save version <= 9/10)
        Game->>GameLogic: xfer(XFER_LOAD)
        loop For each saved object (oldest to newest)
            Xfer->>GameLogic: Read object data
            GameLogic->>ObjectList: Create object via prependToList()
            Note over ObjectList: Prepending reverses order
        end
        GameLogic->>ObjectList: Reverse entire list
        loop For each object
            ObjectList->>ObjectList: Swap next/prev pointers
        end
        Note over ObjectList: Correct order restored
    else XFER_LOAD (New Save version >= 10/11)
        Game->>GameLogic: xfer(XFER_LOAD)
        loop For each saved object (newest to oldest)
            Xfer->>GameLogic: Read object data
            GameLogic->>ObjectList: Create object via prependToList()
            Note over ObjectList: Order already correct
        end
    end

Loading

[Stubbjax]

Nice. You'd be also able to test this by saving a game with two Command Centers, then pressing H and expecting to navigate to the same Command Center before and after loading a save.

[Caball009]

Nice. You'd be also able to test this by saving a game with two Command Centers, then pressing H and expecting to navigate to the same Command Center before and after loading a save.

That's not related to this change, though the underlying issue is the same.

You can uncomment this code to get the same behavior for H before and after loading a save:

GeneralsGameCode/GeneralsMD/Code/GameEngine/Source/Common/RTS/Team.cpp

Lines 2742 to 2743 in a4482e0

	// since we prepended the object member pointers, reverse that list so it's just like before
	// reverse_TeamMemberList();

[Mauller]

Could reverse iterating during saving also fix this?

[Caball009]

Could reverse iterating during saving also fix this?

I think so. That wouldn't work with already existing save games, though. Also, I think it requires changes to multiple functions.

[xezon]

This is a good find.

Could reverse iterating during saving also fix this?

I support this. Fixing the order of objects after creation is alright, but creating the objects in the correct order to begin with will be superior, because that will respect the right ordering even during creation, which hopefully is logically irrelevant, but technically more correct.

[Caball009]

The code actually get simpler if you do the work when saving. This will only work properly for new save games.

GeneralsGameCode/GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp

Line 4844 in 2c26a90

for( obj = getFirstObject(); obj; obj = obj->getNextObject() )

->

Object *lastObj = nullptr;
for (obj = getFirstObject(); obj; obj = obj->getNextObject())
{
		lastObj = obj;
}

for( obj = lastObj; obj; obj = obj->getPrevObject() )

New Object member functions:

Object* getPrevObject() { return m_prev; }
const Object* getPrevObject() const { return m_prev; }

[xezon]

Good damn single linked list 😄

[Caball009]

Does it make sense to increase the xfer version for this? It would give us a way to distinguish new save games with inverse object list if we ever needed to.

[xezon]

Yes can do. Is not strictly necessary but fair to do as you have described.

[OmniBlade]

I'd be inclined to do both, reverse the list on load if the xfer version is retail, save with correct order with new version and load as is if version is same or greater than new version.

[xezon]

I assume RETAIL_COMPATIBLE_XFER_SAVE is not strictly necessary? Retail game can still load these save games correctly?

[bobtista]

I think if an old game tries to load a new save it will just fail gracefully. I don't think this will be an issue - if someone asks a friend with TSH to create a .sav and share it with them maybe? But why would that ever happen? If we need to be able to load new saves on old game, we could use RETAIL_COMPATIBLE_XFER_SAVE

[OmniBlade]

So I think the way it would work is that version 10 isn't known by retail and isn't checked so it will load the list the same way it always did which will be the new correct order for saves saved after this version, the old incorrect order for save from before this. This new version checks on load so the order loaded will be correct for builds after this PR is merged no matter what version wrote the save.

[greptile-apps]

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[xezon]

Looks correct. See suggestion from Caball regarding comment tag.

There are a bunch of TODO's in the opening description.