Opened 7 years ago

Closed 7 years ago

Last modified 2 years ago

#3209 closed defect (fixed)

[PATCH] Changing the map shouldn't reset the victory condition without a reason

Reported by: elexis Owned by: Itms
Priority: Should Have Milestone: Alpha 19
Component: UI & Simulation Keywords:
Cc: Patch:

Description

If you anticipate playing a wonder game and change the map, the victory condition will be set to conquest back again. Often it won't be noticed until the wonder is built.

I see that not all maps might support alternative victory conditions. However if wonder mode is supported, the victory condition shouldn't be reset to conquest after changing a map. The other settings like population capacity are not reset either.

Attachments (2)

t3209_keep_victory_condition.patch (3.8 KB ) - added by elexis 7 years ago.
t3209_keep_victory_condition_minimum.patch (1.1 KB ) - added by elexis 7 years ago.
All that im asking for in this ticket.

Download all attachments as: .zip

Change History (10)

comment:1 by elexis, 7 years ago

This problem is different from #3184. That bug made you play conquest even if you had selected wonder mode in the gamesetup.

The problem I want to address here is that the victory condition is set to conquest each time after changing the map in the gamesetup. So you still have a chance of noticing the changed setting before starting the game (contrary to #3184).

Last edited 7 years ago by elexis (previous) (diff)

comment:2 by elexis, 7 years ago

Keywords: patch review added; simple removed
Milestone: BacklogAlpha 19
Priority: Nice to HaveShould Have
Summary: Changing the map shouldn't reset the victory condition[PATCH] Changing the map shouldn't reset the victory condition

by elexis, 7 years ago

comment:3 by elexis, 7 years ago

This patch changes the following things in that order:

Beautifies code: (1) changes a method name from loadGameAttributes to loadPersistMatchSettings for better reading (2) rearranges the order of settings loaded, to correlate with the order of the settings in the gamesetup

Changes gamesetup: (3) Victory condition is now handled by persist match settings too (4) Victory condition won't be reset when changing the map

comment:4 by elexis, 7 years ago

Summary: [PATCH] Changing the map shouldn't reset the victory condition[PATCH] Changing the map shouldn't reset the victory condition without a reason

L751-756 are not needed since the victory condition is already handled by the persist match settings, the lines don't hurt though. The removal of L980-984 is the relevant part. The rest are beautifications. Let me give you two reasons for the need of a patch:

(1) People have noted on IRC that reseting the victory condition makes sense, since not all maps might support all victory conditions. However the set of selectable victory conditions doesn't change, so changing the selected value has no use at all.

(2) Changing the random map resets the victory condition, but no other settings, in particular: -population capactiy -starting resources -revealed / explored map -map size -number of players & selected civilizations -ceasefire time

Therefore this behaviour is really unexpected and since all settings are persisted, you assume that the victory condition is also the same as with the previously selected map. If you don't know that bug, chances are that you start the game with the wrong setting. Displaying the victory condition ingame (see #3209) doesn't help here, since you have to rehost and usually the same set of players won't join again. Also it can be a lot of work to find the correct settings to satisfy everyone, so rehosting can be a pain.


When looking at the code, I see the need for a complete rewrite of the gamesetup too. But that doesn't mean that we can't fix this problem for the next release by removing 4 lines of code (L980-984) before that will happen.

I also understand that some random maps might not want to support all victory conditions in the future. However this feature is not implemented. If it was implemented then the victory condition shouldn't be reset, unless the choice is not supported by the map.

Last edited 7 years ago by elexis (previous) (diff)

by elexis, 7 years ago

All that im asking for in this ticket.

comment:5 by Itms, 7 years ago

Owner: set to Itms
Resolution: fixed
Status: newclosed

In 16840:

Do not reset victory conditions when changing map selection. The behavior is now coherent with everything else (players, etc.) when selecting random maps.

Patch by elexis, fixes #3209

comment:6 by Itms, 7 years ago

Keywords: patch review removed

comment:7 by elexis, 7 years ago

r17026 finds a compromise between r16238 and r16840. The removed code was needed for a bug described in ticket:3001#comment:6.

comment:8 by elexis, 2 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.