Disable QuickLoad during multiplayer
ClosedPublic
Actions

Authored by elexis on Jan 20 2020, 1:44 PM.

Details

Reviewers: None
Commits: rP23429: Disable Quicksave during multiplayer, since the feature is not implemented and…

Summary

In current svn, attempting to quickload a quicksave will yield a disconnect from the server and the game possibly ending with that erorr.

NotifyFinishedClientCommands: Client 1 (elexis2233) is ready for turn 2, but expected 256

This patch disables the hotkey and prints a LOGERROR to avoid that.

Test Plan

Consider whether the better approach would be implementing it as a feature.
This might be useful for some cooperative scenario multiplayer campaign.
Determine the likelihood of that implementation and expected implementation date to see if this patch would just add noise until then.
(Introducing a JS class is not the objective of this diff)

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Jan 20 2020, 1:44 PM

I think it would be nicer to have the MP savegame features, but if it isn't gonna happen soon, I believe this is the next best thing.

source/ps/scripting/JSInterface_SavedGame.cpp
58 ↗	(On Diff #11118)	Can't quicksave during multiplayer! ?
68 ↗	(On Diff #11118)	Can't quickload during multiplayer! ?

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/ps/scripting/JSInterface_SavedGame.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1616/display/redirect

Harbormaster completed remote builds in B10891: Diff 11118.Jan 20 2020, 2:01 PM

elexis edited the summary of this revision. (Show Details)Jan 21 2020, 1:49 PM

In D2584#108308, @Stan wrote:

I think it would be nicer to have the MP savegame features

I don't know what you refer to when you speak about MP savegame features, because so far there is only one MP savegame feature on the table, which is picking a savegame, starting a host, and then running that match.
Loading a new savegame during the match has not been on the menu so far, but as mentioned in the test plan, it could be conceivable in a cooperative environment (essentially humans vs bots or scripted enemies).
In a scenario (and Im not refering to scenario maps) where humans play against each other, loading a quicksave would be essentially equal to cheating, because someone would load a quicksave state before he loses and the game would never end.

but if it isn't gonna happen soon, I believe this is the next best thing.

Currently its a big defect and if implementing quickload during the match is implemented at all, it wouldn't be implemented first, and since implementing an mpsavegame feature reuiring fixing defects first, this should be considered a precondition for implementing mp savegames (unless someone comes along and says that quicksave loading during mp is a must have for the first version of mpsavegames and that fixing this defect is only deflecting from that path)

In D2584#108479, @elexis wrote:

I don't know what you refer to when you speak about MP savegame features, because so far there is only one MP savegame feature on the table, which is picking a savegame, starting a host, and then running that match.

I mostly refer to this: #1088.

In D2584#108479, @elexis wrote:

Currently its a big defect and if implementing quickload during the match is implemented at all, it wouldn't be implemented first, and since implementing an mpsavegame feature reuiring fixing defects first, this should be considered a precondition for implementing mp savegames (unless someone comes along and says that quicksave loading during mp is a must have for the first version of mpsavegames and that fixing this defect is only deflecting from that path)

Agreed.

elexis added inline comments.Jan 22 2020, 2:59 PM

source/ps/scripting/JSInterface_SavedGame.cpp
57 ↗	(On Diff #11118)	g_NetServer is redundant with g_NetClient for the current implementation, and in an implementation where g_NetServer could run without a g_NetClient, the GUI would probably not be initialized (#5643), unless for that an UI is implemented too. Adding the NetServer check adds additional certainty for that case and is consistent with the ENSUREs below.
58 ↗	(On Diff #11118)	I chose these strings since they're not wrong and since they're consistent with the ones below.