Opened 7 years ago

Closed 4 years ago

#4587 closed defect (fixed)

Match setup option 'Revealed Map' does not persist

Reported by: Phormio Owned by: elexis
Priority: Should Have Milestone: Alpha 24
Component: UI – Game setup Keywords:
Cc: Patch:

Description

When enabled, the option 'Revealed Map' changes to 'Explored Map' for the next game.

Change History (4)

comment:1 by Imarok, 5 years ago

Component: UI & SimulationGame setup

Move tickets to Game Setup as UI & Simulation got some sub components.

comment:2 by elexis, 4 years ago

One way to reproduce:

  1. Enable persist-match-settings option
  2. Select skirmish Acropolis Bay (2)
  3. Enable revealed map (this also ticks the explored map setting as implemented follwoing the review recommendation in Phab:D322)
  4. Start Game
  5. End Game
  6. Open gamesetup
  7. Notice that Explored Map is true but Revealed map is false

For this specific way to reproduce, it's because the persist-match-settings feature persists all settings except the ones overwritten by the map. The Acropolis Bay (2) map specifies RevealedMap=false, and no value for ExploredMap so it works as designed.

So it's again the problem of two contradicting use cases: (1) persisting everything, so users like you expect all settings to persist and (2) the map overwriting user settings, so users are surprised if a setting was changed behind their back.

The "persist user settings" use case could be fully implemented by deleting and disregarding map default values at the cost of not being able to allow maps to predetermine fixed or default values.

If one makes it so that a map default value means that the according control is disabled, then it will still be unexpected that some user chosen setting values are overwritten when changing the map (or as here when restarting the game).

So it will be conceptually flawed as long as we want to follow both use cases, but one can mitigate the problem by displaying in chat which settings changed, so setting changes don't occur anymore behind the users back, see Phab:D1195 for example. Then the user is surprise-notified with a setting change before starting the game at least.

There are multiple trac gamesetup tickets with the same symptom and a missing concept that addresses both use cases without logical contradiction.

comment:3 by elexis, 4 years ago

Milestone: BacklogAlpha 24
Owner: set to elexis
Patch: Phab:D2483

comment:4 by elexis, 4 years ago

Patch: Phab:D2483
Resolution: fixed
Status: newclosed

In r23279: Clean Skirmish maps of default and invalid gamesetting values.

The gamesetup class rewrite in D2483 will enable Skirmish and Random maps to fix AIs and Civs by specifying them. Hence remove them from all maps that don't intend to fix them (i.e. all except Egypt 3v3).
Remove default values (and the few irrelevant non-default values) so that the player-chosen settings may be persisted between Skirmish map selection changes, refs #3120, D2483.
Remove invalid settings: mapType, AISeed and random map settings Script, Size, Seed, Nomad, BaseHeight, BaseTerrain, refs rP20726, rP21274.
Remove StartingCamera values since they are useless, refs #4839, D1098.
Add the "new" keyword to Atlas Valleys and Vesuvius from rP23278, so they appear in the "new" filter.
Unhide Barcania (3)

This fixes the ticket in so far as that no Skirmish map has the Revealed map flag anymore. New maps may very well come with that again.

Note: See TracTickets for help on using tickets.