Multiplayer savegame #7047

phosit · 2024-09-15T09:40:37+02:00

phosit commented

2024-09-15 09:40:37 +02:00

This PR allows to save and later load multiplayer games.
This PR is also usefull rebalance after a client is kicked.

We might unfreze more settings (victory condition, player color...) in future PRs.

Continuation of https://code.wildfiregames.com/D4770
Compared to the phab version it uses the filetransferer, download is done when the loading screan is shown and an issue with player names is fixed.

This PR allows to save and later load multiplayer games. This PR is also usefull rebalance after a client is kicked. We might unfreze more settings (victory condition, player color...) in future PRs. Continuation of https://code.wildfiregames.com/D4770 Compared to the phab version it uses the filetransferer, download is done when the loading screan is shown and an issue with player names is fixed.

❤️ 5

phosit added 2 commits 2024-09-15 09:40:41 +02:00

Enable multiple RequestTypes 88b7a6b4d7

Multiplayer saved games

0ad-freebsd/pipeline/pr-main There was a failure building this commit

Details

0ad-linux/pipeline/pr-main There was a failure building this commit

Details

checkrefs / checkrefs (pull_request) Successful in 1m1s

Details

pre-commit / build (pull_request) Successful in 1m17s

Details

0ad-windows/pipeline/pr-main There was a failure building this commit

Details

0ad-macos/pipeline/pr-main There was a failure building this commit

Details

05c0bb07d1

Enables to save multiplayer games.
When the savegame is loaded, the settings are frozen (except the non-AI-player assignment settings).

phosit requested review from Imarok 2024-09-15 09:40:41 +02:00

phosit requested review from s0600204 2024-09-15 09:40:41 +02:00

phosit requested review from wraitii 2024-09-15 09:40:41 +02:00

phosit requested review from marder 2024-09-15 09:40:41 +02:00

phosit changed title from ~~mp-savegame~~ to Multiplayer savegame

2024-09-15 09:41:56 +02:00

abian commented

2024-09-15 15:27:45 +02:00

This is a great one, thank you!

Some suggestions come to mind:

Clear m_SavedState (and possibly other data) as soon as it's no longer in use.

For future patches:

Introduce checksums in the saved game data file and use them for data integrity.
Introduce comprehensive checks on clients to ensure that they can trust the data they receive and handle security risks.
Ensure that all participants have successfully completed loading the game before anyone can start the simulation and, after that, ensure that everyone starts playing at the same time.

This is a great one, thank you! Some suggestions come to mind: - [x] Clear m_SavedState (and possibly other data) as soon as it's no longer in use. For future patches: - Introduce checksums in the saved game data file and use them for data integrity. - Introduce comprehensive checks on clients to ensure that they can trust the data they receive and handle security risks. - Ensure that all participants have successfully completed loading the game before anyone can start the simulation and, after that, ensure that everyone starts playing at the same time.

real_tabasco_sauce commented

2024-09-15 21:42:08 +02:00

@phosit let me know if you want to test this sometime.
I'd suggest that a super awesome addition could be a periodical autosave but that should be a separate PR.

@phosit let me know if you want to test this sometime. I'd suggest that a super awesome addition could be a periodical autosave but that should be a separate PR.

phosit commented

2024-09-16 18:22:59 +02:00

Clear m_SavedState (and possibly other data) as soon as it's no longer in use.

That should be easy.

Introduce multiplayer checksums in the saved game data and use them for data integrity.

Do you mean adding a checksum to the saved game (on disk) or when transfereding it to the clients? In both cases it should be done in it's own commit.

Introduce comprehensive checks to ensure that the saved game data is compatible with all the participants' game versions and mods.

There is already a warning when one loads an incompatible game. Same for joining games via Lobby.

Introduce comprehensive checks on clients to ensure that they can trust the data they receive and handle security risks.

Do you want to protect against a host loading a game where they have an advantage?
A protection would be to send the id (as chat message) when the host saves a game. And then show the id in the gamesetup. The clients could check that it's the same.
Don't know about security risks, rejoin-gamestate already would have the same risks.

Ensure that all participants have successfully completed loading the game before anyone can start the simulation and, after that, ensure that everyone starts playing at the same time.

The game starts when all clients have loaded the game. One would have to edit JS to bypass that. That's hard to prevent.

> - [ ] Clear m_SavedState (and possibly other data) as soon as it's no longer in use. That should be easy. > - [ ] Introduce multiplayer checksums in the saved game data and use them for data integrity. Do you mean adding a checksum to the saved game (on disk) or when transfereding it to the clients? In both cases it should be done in it's own commit. > - [ ] Introduce comprehensive checks to ensure that the saved game data is compatible with all the participants' game versions and mods. There is already a warning when one loads an incompatible game. Same for joining games via Lobby. > - [ ] Introduce comprehensive checks on clients to ensure that they can trust the data they receive and handle security risks. Do you want to protect against a host loading a game where they have an advantage? A protection would be to send the id (as chat message) when the host saves a game. And then show the id in the gamesetup. The clients could check that it's the same. Don't know about security risks, rejoin-gamestate already would have the same risks. > - [ ] Ensure that all participants have successfully completed loading the game before anyone can start the simulation and, after that, ensure that everyone starts playing at the same time. The game starts when all clients have loaded the game. One would have to edit JS to bypass that. That's hard to prevent.

👍 1

Norse_Harold commented

2024-09-17 17:47:06 +02:00

First-time contributor

Introduce multiplayer checksums in the saved game data and use them for data integrity.

The game is already sending hashes of the game state on a regular basis to check for OOS. Assuming that that has complete coverage then a corrupted save game would be detected by the OOS system.

Introduce comprehensive checks on clients to ensure that they can trust the data they receive and handle security risks.

I consider security important. I think that this PR does not add any new potential security vulnerabilities, because it basically uses the existing join-in-progress feature to resume a saved state.

I think that more checks should be done, but in a separate PR. This PR has taken a long time to get to this point and we don't want to have mission creep.

If you are able to help with writing code that does security checks then it would be greatly appreciated.

> Introduce multiplayer checksums in the saved game data and use them for data integrity. The game is already sending hashes of the game state on a regular basis to check for OOS. Assuming that that has complete coverage then a corrupted save game would be detected by the OOS system. > Introduce comprehensive checks on clients to ensure that they can trust the data they receive and handle security risks. I consider security important. I think that this PR does not add any new potential security vulnerabilities, because it basically uses the existing join-in-progress feature to resume a saved state. I think that more checks should be done, but in a separate PR. This PR has taken a long time to get to this point and we don't want to have mission creep. If you are able to help with writing code that does security checks then it would be greatly appreciated.

👍 1

phosit force-pushed mp-savegame from 05c0bb07d1 to ed8ddc8398

2024-09-17 20:06:50 +02:00

Compare

phosit commented

2024-09-17 20:08:47 +02:00

m_SavedState is now cleared when it's no longer in use.
I hope it builds now with the rebase

m_SavedState is now cleared when it's no longer in use. I hope it builds now with the rebase

🎉 1

phosit force-pushed mp-savegame from ed8ddc8398 to a101db61e8

2024-09-18 21:44:17 +02:00

Compare