Opened 8 years ago

Closed 8 years ago

Last modified 3 years ago

#2963 closed enhancement (fixed)

[PATCH] Preserve game settings after launching/canceling a game

Reported by: Alex Owned by: leper
Priority: Should Have Milestone: Alpha 18
Component: Multiplayer lobby Keywords: gamesettings
Cc: Patch:

Description

Just had this annoying experience where I wanted to play the same map again and therefore had to set up all the game settings a second time.

Time to get this patched away.

Note: In the patch I've appended, player settings are preserved as well, so I don't exactly know how this will interfere with multiplayer games. Gonna test it out soon.

I hope it's acceptable to store all settings in a JSONized string in the user config - if any part of the settings naming 'API' should be changed in the future, it's rather no problem then.

Attachments (3)

preserveGameAttributes.patch (3.6 KB ) - added by Alex 8 years ago.
Map reselection works now. It's still needed to test multiplayer behaviour.
preserveGameAttributes.2.patch (6.4 KB ) - added by Alex 8 years ago.
Implemented a WriteJSONFile method to let it dump the settings directly into .json files
saveStuff.patch (743 bytes ) - added by Alex 8 years ago.
Fixed not having random civs anymore after launching a game with previously saved settings

Download all attachments as: .zip

Change History (28)

comment:1 by Alex, 8 years ago

1) Also, things like the AISeed shouldn't be persisted. 2) Perhaps it would be the best to manually remove these fields from the object. Then, the object had to be cloned though.

3) I don't know whether cancelSetup() in gamesetup.js gets called for multiplayer lobbies, too.

4) Would it make even more sense to split up into single/multiplayer-settings?

5) Just checked the behaviour of scenarios where civs are fixed, then after reloading, this setting confuses the UI logic etc. - perhaps omitting the entire player settings part from the settings might help there a lot - or just persist AI difficulties.

Last edited 8 years ago by Alex (previous) (diff)

comment:2 by Lionkanzen, 8 years ago

I have a questions This one need custom build to work? I wanna test as user .

Pd give the tag "review" for Dev can give a review for it. Thank you

comment:3 by Alex, 8 years ago

No, you don't have to build the entire game on your own. Instead:

  • Just search for a 'public.zip' in your game folders,
  • open it using any zip tool,
  • head to binaries/data/mods/public/gui/gamesetup/gamesetup.js
  • extract it to have a backup of it
  • duplicate the gamesetup.js backup, this is the one that gets modified now
  • replace the lines with those given by the patch
  • put the modified gamesetup.js back into the zip
  • Start 0ad

Please do keep in mind that this is a first version that should cause misbehaviour when switching between single/multiplayer lobbies. Still, just try it out, you still have the backed-up gamesetup.js :)

Last edited 8 years ago by Alex (previous) (diff)

comment:4 by Alex, 8 years ago

Keywords: review added

comment:5 by Alex, 8 years ago

So, I pretty much got it working now.

1) Also, things like the AISeed shouldn't be persisted. Done. Also, the MatchId is regenerated as well.

2) Perhaps it would be the best to manually remove these fields from the object. Then, the object had to be cloned though. Not necessary because they will become overwritten anyway.

3) I don't know whether cancelSetup() in gamesetup.js gets called for multiplayer lobbies, too. Works properly. gamesetup_mp.js is an entirely different thing (where I originally assumed it somehow inherited code from gamesetup.js).

4) Would it make even more sense to split up into single/multiplayer-settings? Yes, thinking of campaign or multiplayer/singleplayer-only maps.

5) Just checked the behaviour of scenarios where civs are fixed, then after reloading, this setting confuses the UI logic etc. - perhaps omitting the entire player settings part from the settings might help there a lot - or just persist AI difficulties. This part has been solved as well now, at least entirely for singleplayer. It's still required to check the behaviour when there are other real players logged in.

Furthermore, how to protect the settings against corruption? Try to hide this JSONized part from the config? Enforce proper semantics checks? Build up a custom settings data object that stores only 'visible' information, so e.g. no map meta info or AI-internal things?

comment:6 by Alex, 8 years ago

Okay, just tested said multiplayer case. All slots that are filled with real players will be 'unassigned' after hosting an other game. I think that this is somewhat acceptable - I probably could fill these slots with AIs again though.

comment:7 by Alex, 8 years ago

Component: UI & SimulationMultiplayer lobby

by Alex, 8 years ago

Map reselection works now. It's still needed to test multiplayer behaviour.

comment:8 by Stan`, 8 years ago

I'm not sure saving the *.json file in the public.zip is a good idea. Maybe save it in the user.cfg instead ? Or create a preference file in that folder ?

comment:9 by Alex, 8 years ago

Because WriteJSONFile uses pyrogenesis' vfs-infrastructure to write files, the relative path 'config/matchsettings.json' means it gets written into the user.cfg-directory, not somewhere in the 0ad binaries :)

comment:10 by Alex, 8 years ago

There was discussion in the irc whether the settings shall be saved only during runtime, so not after the game was shut down and restarted.

Why I think that this is a bad idea:

  • When you play in multiplayer (mp), you won't always host, but when you host, all your settings you entered sometime earlier will likely be gone.
  • From my experience, I only do one, two games per 0ad runtime, so it's then as awful as before to have to set up everything again.
  • All Age of Empires episodes save settings, too.
  • In the rare case that a default map (or other values) changes, gamers who've been playing 0ad will change the map to their needs anyway.
  • Invalid values will be ignored because they won't get found by indexOf().
  • I'd put in my custom mod that persists these settings inter-runtime anyway, so screw others' decisions :P
  • It's way faster for map devs to check and test their maps as they don't have to reselect it each game start.
Last edited 8 years ago by Alex (previous) (diff)

comment:11 by Stan`, 8 years ago

Dev's usually use quickstart for that purpose. (CommandLine Option) that's why they probably don't see the utility. I don't really think it matters to save the multiplayer stuff though. Because most of the guys don't play everyday with each other. (However saving the current data might be a good idea, ie : player joining 3 player game making the host change map and therefore lose all the settings.

BTW, since you are at saving stuff could you have a look at #1580 ? This is a feature I'd really like to have, and that will need to be saved if it gets in the game.

Stan.

comment:12 by Alex, 8 years ago

In my relative long RTS experience, saving those details (even if it's only about fixed teams and revealed map by default) is essential to me. Still, you'd definitely want to preserve all settings between matches, otherwise it's bothering me enormously.

Having colors added later on will unlikely interfere with this feature, as the entire gameattribute object is getting dumped into the json.

Right now I'm adding a reset button so you can reset all the settings to the given default - although I don't know why some people are frightened of not having defaults in their settings.

comment:13 by fabio, 8 years ago

What about adding a gameplay tab to options menu with these settings, rather than automatically save the last used as a default?

in reply to:  13 comment:14 by Itms, 8 years ago

Replying to fabio:

What about adding a gameplay tab to options menu with these settings, rather than automatically save the last used as a default?

I like that! Dropping a link to #1810 and #1811 :)

comment:15 by Alex, 8 years ago

No, not gonna do this. I'm happy with just let it silently work for me.

I'm furthermore not sure whether this reset button is really needed. I mean, what's so bad about having settings from the lastly hosted game in multiplayer?

Last edited 8 years ago by Alex (previous) (diff)

comment:16 by leper, 8 years ago

(Following from some discussion/opinion gathering on irc yesterday)

Persisiting it across restarts is fine, but I'd like to see a way to clear that (somewhere in the settings dialog probably).

comment:17 by Alex, 8 years ago

Alright, added an extra option where you can disable saving those settings. Disabling the option also deletes the matchsettings.json file (despite the actual removal happens when leaving a gamelobby)

by Alex, 8 years ago

Implemented a WriteJSONFile method to let it dump the settings directly into .json files

comment:18 by leper, 8 years ago

Owner: set to leper
Resolution: fixed
Status: newclosed

In 16089:

Save map settings. Patch by @aBothe. Fixes #2963.

comment:19 by leper, 8 years ago

Keywords: review removed

Thanks for the patch.

by Alex, 8 years ago

Attachment: saveStuff.patch added

Fixed not having random civs anymore after launching a game with previously saved settings

comment:20 by Alex, 8 years ago

Keywords: review added
Resolution: fixed
Status: closedreopened

comment:21 by mimo, 8 years ago

In 16095:

preserve random civ in saved game settings, refs #2963

comment:22 by mimo, 8 years ago

Thanks @aBothe for the patch.

comment:23 by mimo, 8 years ago

Resolution: fixed
Status: reopenedclosed

comment:24 by sanderd17, 7 years ago

Keywords: review removed

comment:25 by elexis, 3 years ago

In 23374:

Gamesetup class rewrite, fixes #5322, refs #5387.

  • Decouples settings logically which in turn allows fixing many problems arising from previous coupling.
  • Fixes the persist-match-settings feature, refs #2963, refs #3049.
  • Improves performance of the matchsetup by rebuilding GUI objects only when necessary.

Provides groundwork for:

  • UI to control per-player handicap, such as StartingResources, PopulationCap, StartingTechnologies, DisabledTechnologies, DisabledTemplates, ..., refs #812.
  • Map specific settings (onMapChange event), refs #4838.
  • Chat notifications announcing which settings changed, refs D1195,
  • Multiple controllers setting up the game (since setting types can check for permissions in onUpdateGameAttributes without the need for a new data model or a second gamesetup data network message type), refs #3806, subsequently dedicated server, refs #3556.
  • MapBrowser (MapCache, MapTypes, onUpdateGameAttributes interface), refs D1703 and D1777,
  • Multiplayer saved games (decoupling and setting dependent unique logic), refs #1088.

Refs https://wildfiregames.com/forum/index.php?/topic/20787-paid-development-2016/ https://wildfiregames.com/forum/index.php?/topic/20789-paid-development-2016/

Enable maps to restrict setting values:

  • If a map specifies an AI or Civs for a playerslot, the controller can't assign a player/other AI or Civ to that slot, refs #3049, #3013.

Fix per player StartingResources, PopulationCap, StartingTechnologies, DisabledTechnologies, DisabledTemplates following rP12756, refs #812, fixes #4504. Use this for DisabledTechnologies on Polar Sea.

Persist user settings for Skirmish maps:

  • All user chosen settings are persisted when changing the selected map or maptype, except where the selected map overwrites the setting value and except for Scenario maps which still use the default value where the map doesn't specify the setting value.
  • Tickets relating to that Skirmish mapchange user setting persistance:
    • Selecting a map doesn't change the selected civilizations, fixes #3120 (together with r23279 removing map specified Civs).
    • Selecting a map type doesn't reset the selected settings, fixes #5372.
    • Selecting a map doesn't change the selected victory conditions, unless the map specifies those, refs #4661, #3209. (Atlas still writes VictoryConditions to every map.)
    • Consume the player color palette from Skirmish maps, refs rP17040 / #1580. Preserve the selected playercolors when switching the Skirmish/Random map by chosing the most similar colors if the map comes with a different palette.

Rated games:

  • Hide and disable Rated game setting unless there are exactly two players, fixes #3950, supersedes D2117.
  • Display conspicuous warning if the game is rated, so players are perfectly aware.

Autostarted games:

  • Allow using the gamesetup page to autostart matches with arbitrary maps, not only this one tutorial, as reported in D194 and rP19599, refs D11.

Networking:

  • Keep gamesetup page open after disconnect, allowing players to read chat messages indicating why the host stopped the server, fixes #4114.
  • The message subscription system allows new and mod settings to run custom logic on arbitrary setting changes (most importantly on map change). This removes hardcoded logic restrictions from the gamesetup option unification rewrite in rP19504/D322, refs #3994, such as the hardcoding of setting references in selectMap to biomes from rP20115/D852 and the difficulty from rP20760/D1189, RelicDuration, WonderDuration, LastManStanding, RegicideGarrison, TriggerScripts, CircularMap, Garrison, DisabledTemplates.

Checkboxes:

  • Display values of disabled checkboxes with Yes/No labels, fixes D2349, reviewed by nani.

Clean g_GameAttributes of invalid values and gamesetup GUI temporaries, refs #3049, #3883:

MapCache:

  • Refactor to MapCache class, store maps of all types and use it in the replaymenu, lobby and session as well.

SettingTabsPanel:

  • Remove hardcodings and coupling of the SettingTabsPanel with biomes/difficulties/chat UI from D1027/rP20945.

GamesetupPage.xml:

  • Restructure the page to use hierarchical object organization (topPanel, centerPanel, centerLeftPanel, bottomPanel, centerCenterPanel, centerRightPanel, bottomLeftPanel, bottomRightPanel), allowing to deduplicate object position margins and size math and ease navigation.

New defaults:

  • Check LockedTeams default in multiplayer (not only rated games).
  • Persist the rated game setting instead of defaulting to true when restarting a match, which often lead to unintentional rated games when rehosting.
  • 60 FPS in menus since they are animated

Autocomplete sorting fixed (playernames should be completed first).
Refactoring encompasses the one proposed in Polakrity and bb D1651.

Differential Revision: https://code.wildfiregames.com/D2483
Tested by: nani
Discussed with:

Emojis by: asterix, Imarok, fpre, nani, Krinkle, Stan, Angen, Freagarach

Note: See TracTickets for help on using tickets.