Store received savegames in save folder

[DevOpsOfChaos]

Summary

• store received savegame transfers in the savegame folder instead of the played-maps folder
• keep regular map and Lua transfers in the played-maps folder
• add focused regression coverage for received savegame path selection

Motivation

Received map data is currently written to the played-maps folder unconditionally. This also affects MapType::Savegame, leaving .sav files in MAPS.

Savegame transfers should not be stored as played maps.

Fixes #1846

Validation

• Built Test_network locally with VS2022 Debug
• Ran GameClientTests/ClientStoresReceivedSavegamesInSaveFolder
• Ran GameClientTests

[Flow86]

I like the solution. simple and effective

[DevOpsOfChaos]

Tightened the implementation and regression test.

The target folder is now created before assigning the received map/savegame path, so savegame transfers do not depend on the SAVE directory already existing.

The regression test now sends actual compressed transfer data through GameMessage_Map_Data and verifies that the received savegame payload is written to the save folder and not to the played-maps folder. The transferred payload intentionally uses existing map test data, so the client reports an error after the storage path has been verified.

Validation:

• Built Test_network locally with VS2022 Debug
• Ran GameClientTests/ClientStoresReceivedSavegamesInSaveFolder
• Ran GameClientTests/ClientFollowsConnectProtocol
• Ran Test_network

[Flamefire]

Should be ok, not sure if we should overwrite the users savegames with that received in multiplayer, but I guess that's rare enough

[DevOpsOfChaos]

Thanks for the review.

Good point. I’ll tighten the test so it focuses on the savegame path instead of duplicating the full connect protocol already covered by ClientFollowsConnectProtocol.

I’ll also make the path isolation explicit with rttr::test::ConfigOverride / a test-owned temp folder, so the test cannot write to the user’s real save/map folders.

The production change should stay unchanged; this should only need a test cleanup.

[DevOpsOfChaos]

Addressed both review points.

The savegame-path test now uses an explicit TmpFolder plus rttr::test::ConfigOverride("USERDATA", ...), and it asserts that both the expected SAVE path and the unexpected MAPS path are under that test-owned folder.

I also moved the minimal connection setup into AdvanceClientToMapInfoRequest() so the test no longer reasserts the full connect protocol already covered by ClientFollowsConnectProtocol. The production change is unchanged.

Validation:

• Built Test_network locally with VS2022 Debug
• Ran GameClientTests/ClientStoresReceivedSavegamesInSaveFolder
• Ran GameClientTests

[Flamefire]

I also moved the minimal connection setup into AdvanceClientToMapInfoRequest() so the test no longer reasserts the full connect protocol already covered by ClientFollowsConnectProtocol. The production change is unchanged.

This is still done, isn't it? MockClientInterface doesn't seem necessary and the AdvanceClientToMapInfoRequest overkill

[DevOpsOfChaos]

Updated again.

I removed the helper and the MockClientInterface setup. The test now only brings the client to QueryMapInfo, because GameMessage_Map_Info is ignored before that state.

After that it only checks the savegame path handling:

• savegames go to SAVE
• the played-maps path is not used
• both paths are under the test USERDATA override

Validation:

• Built Test_network
• Ran GameClientTests/ClientStoresReceivedSavegamesInSaveFolder
• Ran GameClientTests

[Flow86]

now basically only a proper git squash/rebase is necessary to clean the git history

[Flamefire]

now basically only a proper git squash/rebase is necessary to clean the git history

@Flow86 Can you enable the squash-merge feature for this repo? IMO it is better to use that, than requiring force-pushes for (very) small changes where a single commit makes sense.

For others an update is not (always) necessary and could be done by a merge too. As mentioned elsewhere this is quite common and doesn't break reviews, so I'd prefer that if updating at all.