Mods with maps bug new game creation

[Caballero-Arepa]

Is there an existing issue for this?

• [X] I have searched the existing issues

Game Version

4.10.21

Describe the bug

It has been happening for a long time now. I believe it has something to do with the code that automatically sets a recently saved map [from the Map Editor] in the New Game settup.

It mostly happens when you have few maps or is your first time downloading the mod. For some it is fixed by closing and opening the game, but for others it stays.

The bug is that the mod with the maps is forced to be played, even if you manually change the map to a generated map, or not select the mod, and even using the Reset to Default, once you start the match, the app will deselect all the other mods, activate the map mod and use a custom map.

Steps to Reproduce

1. Open the game
2. Download Latin American Civs
3. Click "start new game"
4. Check the ruleset mod's list
5. Gods and Kings is selected
6. Latin American Civs mod is not selected
7. The map is automatically selected, and is a map of the Americas
8. Check the available civs
9. See Argentina, Bolivia, Chile, different Brazil, Colombia, Costa Rica, Cuba etc... (all Latin American Civs mod's civs) as available civs
10. Try adding more mods and set a generated map
11. Start game
12. Realize it's a custom map and only the Latin American civs mod is active

Screenshots

No response

Link to save file

No response

Operating System

Windows

Additional Information

No response

[Caballero-Arepa]

Thanks a lot!

[yairm210]

Hmmm going through the description this is not actually 100% solved, didn't check the 'reset to default' &c

[SomeTroglodyte]

... and that commit sounds like a misunderstanding. One shouldn't throw away the UI elements and rebuild them, but update their state ... ~modCheckboxes.setBaseRuleset(gameParameters.baseRuleset) is one step better,~ but the problem is more likely gameParameters being changed without notifying these widgets.

...(hours later)...

Considering this patch:

```patch Index: core/src/com/unciv/ui/screens/newgamescreen/NewGameScreen.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/core/src/com/unciv/ui/screens/newgamescreen/NewGameScreen.kt b/core/src/com/unciv/ui/screens/newgamescreen/NewGameScreen.kt --- a/core/src/com/unciv/ui/screens/newgamescreen/NewGameScreen.kt (revision 0474755a0d0d2d8eca8bedb9002b49d9cee0987b) +++ b/core/src/com/unciv/ui/screens/newgamescreen/NewGameScreen.kt (date 1712219455379) @@ -61,7 +61,7 @@ init { val isPortrait = isNarrowerThan4to3() - tryUpdateRuleset() // must come before playerPickerTable so mod nations from fromSettings + tryUpdateRuleset(updateUI = false) // must come before playerPickerTable so mod nations from fromSettings // remove the victory types which are not in the rule set (e.g. were in the recently disabled mod) gameSetupInfo.gameParameters.victoryTypes.removeAll { it !in ruleset.victories.keys } @@ -365,7 +365,7 @@ * * @return Success - failure means gameSetupInfo was reset to defaults and the Ruleset was reverted to G&K */ - fun tryUpdateRuleset(): Boolean { + fun tryUpdateRuleset(updateUI: Boolean): Boolean { var success = true fun handleFailure(message: String): Ruleset { success = false @@ -388,6 +388,8 @@ ruleset.add(newRuleset) ImageGetter.setNewRuleset(ruleset) game.musicController.setModList(gameSetupInfo.gameParameters.getModsAndBaseRuleset()) + + if (updateUI) newGameOptionsTable.updateRuleset(ruleset) return success } Index: core/src/com/unciv/ui/screens/newgamescreen/GameOptionsTable.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/core/src/com/unciv/ui/screens/newgamescreen/GameOptionsTable.kt b/core/src/com/unciv/ui/screens/newgamescreen/GameOptionsTable.kt --- a/core/src/com/unciv/ui/screens/newgamescreen/GameOptionsTable.kt (revision 0474755a0d0d2d8eca8bedb9002b49d9cee0987b) +++ b/core/src/com/unciv/ui/screens/newgamescreen/GameOptionsTable.kt (date 1712221106427) @@ -11,6 +11,7 @@ import com.unciv.models.metadata.BaseRuleset import com.unciv.models.metadata.GameParameters import com.unciv.models.metadata.Player +import com.unciv.models.ruleset.Ruleset import com.unciv.models.ruleset.RulesetCache import com.unciv.models.ruleset.nation.Nation import com.unciv.models.ruleset.unique.UniqueType @@ -42,9 +43,12 @@ private val updatePlayerPickerRandomLabel: () -> Unit ) : Table(BaseScreen.skin) { var gameParameters = previousScreen.gameSetupInfo.gameParameters - val ruleset = previousScreen.ruleset + var ruleset = previousScreen.ruleset var locked = false + private var baseRulesetHash = gameParameters.baseRuleset.hashCode() + private var selectedModsHash = gameParameters.mods.hashCode() + /** Holds the UI for the Extension Mods * * Attention: This Widget is a little tricky due to the UI changes to support portrait mode: @@ -53,7 +57,7 @@ * * The second reason this is public: [NewGameScreen] accesses [ModCheckboxTable.savedModcheckResult] for display. */ - var modCheckboxes = getModCheckboxes(isPortrait = isPortrait) + val modCheckboxes = getModCheckboxes(isPortrait = isPortrait) // Remember this so we can unselect it when the pool dialog returns an empty pool private var randomNationsPoolCheckbox: CheckBox? = null @@ -69,8 +73,18 @@ fun update() { clear() - // Mods may have changed - modCheckboxes = getModCheckboxes(isPortrait = isPortrait) + + // Mods may have changed (e.g. custom map selection) + val newBaseRulesetHash = gameParameters.baseRuleset.hashCode() + if (newBaseRulesetHash != baseRulesetHash) { + baseRulesetHash = newBaseRulesetHash + modCheckboxes.setBaseRuleset(gameParameters.baseRuleset) + } + val newSelectedModsHash = gameParameters.mods.hashCode() + if (newSelectedModsHash != selectedModsHash) { + selectedModsHash = newSelectedModsHash + modCheckboxes.updateSelection(gameParameters.mods) + } add(Table().apply { defaults().pad(5f) @@ -430,6 +444,12 @@ add(victoryConditionsTable).colspan(2).row() } + fun updateRuleset(ruleset: Ruleset) { + this.ruleset = ruleset + gameParameters.acceptedModCheckErrors = "" + modCheckboxes.setBaseRuleset(gameParameters.baseRuleset) + } + fun resetRuleset() { val rulesetName = BaseRuleset.Civ_V_GnK.fullName gameParameters.baseRuleset = rulesetName @@ -446,6 +466,7 @@ ruleset.mods += gameParameters.baseRuleset ruleset.mods += gameParameters.mods ruleset.modOptions = newRuleset.modOptions + gameParameters.acceptedModCheckErrors = "" ImageGetter.setNewRuleset(ruleset) UncivGame.Current.musicController.setModList(gameParameters.getModsAndBaseRuleset()) Index: core/src/com/unciv/ui/screens/newgamescreen/MapOptionsTable.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/core/src/com/unciv/ui/screens/newgamescreen/MapOptionsTable.kt b/core/src/com/unciv/ui/screens/newgamescreen/MapOptionsTable.kt --- a/core/src/com/unciv/ui/screens/newgamescreen/MapOptionsTable.kt (revision 0474755a0d0d2d8eca8bedb9002b49d9cee0987b) +++ b/core/src/com/unciv/ui/screens/newgamescreen/MapOptionsTable.kt (date 1712221930085) @@ -38,6 +38,7 @@ MapGeneratedMainType.custom -> { mapParameters.type = MapGeneratedMainType.custom mapTypeSpecificTable.add(savedMapOptionsTable) + savedMapOptionsTable.onSelectBoxChange() newGameScreen.unlockTables() } MapGeneratedMainType.generated -> { Index: core/src/com/unciv/ui/screens/newgamescreen/ModCheckboxTable.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/core/src/com/unciv/ui/screens/newgamescreen/ModCheckboxTable.kt b/core/src/com/unciv/ui/screens/newgamescreen/ModCheckboxTable.kt --- a/core/src/com/unciv/ui/screens/newgamescreen/ModCheckboxTable.kt (revision 0474755a0d0d2d8eca8bedb9002b49d9cee0987b) +++ b/core/src/com/unciv/ui/screens/newgamescreen/ModCheckboxTable.kt (date 1712222173881) @@ -47,6 +47,7 @@ private var disableChangeEvents = false private val expanderPadTop = if (isPortrait) 0f else 16f + private val expanderPadOther = if (isPortrait) 0f else 10f init { val modRulesets = RulesetCache.values.filter { @@ -67,6 +68,18 @@ setBaseRuleset(initialBaseRuleset) } + fun updateSelection(newMods: Collection) { + savedModcheckResult = null + mods.clear() + mods.addAll(newMods) + disableChangeEvents = true + for (mod in modWidgets) { + mod.widget.isChecked = mod.mod.name in mods + } + disableChangeEvents = false + deselectIncompatibleMods(null) + } + fun setBaseRuleset(newBaseRuleset: String) { baseRulesetName = newBaseRuleset savedModcheckResult = null @@ -88,7 +101,7 @@ for (mod in compatibleMods) { it.add(mod.widget).row() } - }).pad(10f).padTop(expanderPadTop).growX().row() + }).pad(expanderPadOther).padTop(expanderPadTop).growX().row() disableIncompatibleMods() @@ -103,6 +116,7 @@ mods.clear() disableChangeEvents = false + savedModcheckResult = null disableIncompatibleMods() onUpdate("-") // should match no mod } @@ -176,11 +190,10 @@ /** Deselect incompatible mods after [skipCheckBox] was selected. * - * Note: Inactive - we don'n even allow a conflict to be turned on using [disableIncompatibleMods]. + * Note: Inactive - we don't even allow a conflict to be turned on using [disableIncompatibleMods]. * But if we want the alternative UX instead - use this in [checkBoxChanged] near `mods.add` and skip disabling... */ - @Suppress("unused") - private fun deselectIncompatibleMods(skipCheckBox: CheckBox) { + private fun deselectIncompatibleMods(skipCheckBox: CheckBox?) { disableChangeEvents = true for (modWidget in modWidgets) { if (modWidget.widget == skipCheckBox) continue Index: core/src/com/unciv/ui/screens/newgamescreen/MapFileSelectTable.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/core/src/com/unciv/ui/screens/newgamescreen/MapFileSelectTable.kt b/core/src/com/unciv/ui/screens/newgamescreen/MapFileSelectTable.kt --- a/core/src/com/unciv/ui/screens/newgamescreen/MapFileSelectTable.kt (revision 0474755a0d0d2d8eca8bedb9002b49d9cee0987b) +++ b/core/src/com/unciv/ui/screens/newgamescreen/MapFileSelectTable.kt (date 1712221930094) @@ -131,8 +131,9 @@ // Do NOT try to put back the "bad" preselection! } - private fun onSelectBoxChange() { + fun onSelectBoxChange() { cancelBackgroundJobs() + if (mapFileSelectBox.selection.isEmpty) return val mapFile = mapFileSelectBox.selected.fileHandle mapParameters.name = mapFile.name() newGameScreen.gameSetupInfo.mapFile = mapFile @@ -140,7 +141,7 @@ newGameScreen.gameSetupInfo.gameParameters.mods = LinkedHashSet(mapMods.second) newGameScreen.gameSetupInfo.gameParameters.baseRuleset = mapMods.first.firstOrNull() ?: mapFileSelectBox.selected.mapParameters.baseRuleset - val success = newGameScreen.tryUpdateRuleset() + val success = newGameScreen.tryUpdateRuleset(updateUI = true) newGameScreen.updateTables() hideMiniMap() if (success) { ```

As you can see less trivial and fly-by fixing some other inconsistencies. I'm just a little unsure whether some of that (e.g. the if (newBaseRulesetHash != baseRulesetHash) thing) is really necessary.

[SomeTroglodyte]

@Caballero-Arepa since I'm not 100% confident about that merged PR, could you test some more?

[Caballero-Arepa]

Sure! As always, we appreciate your work

[SomeTroglodyte]

Uh... Already found two R E A L L Y embarrassing blunders... See #11398

[Caballero-Arepa]

Unfortunately, the bug is not solved.

I got a good screen recorder and I'll show you. https://youtu.be/_rNy5_Op-1A

Important note: The mouse is hidden, but when I go to the Main menu, and then back inside New Game, the Latam mod is turned on BY ITSELF! I didn't check it.

[SomeTroglodyte]

Lat-am is supposed to activate if a map using it automatically activates - other than that, the bugs fixed in #11398 will confuse things to a degree it will seem mysterious. Probably most of what that video shows is due to checkbox visuals not matching internal on/off state.

[Caballero-Arepa]

Lat-am is supposed to activate if a map using it automatically activates

Of course, but as you can see, I change the map to a generated one, and it still forces me to a Custom map

[SomeTroglodyte]

as you can see

Wasn't too clear in the vid - but neither is it clear that the recent fix merged in 4.11.2 would fix such a problem. Maybe - or not. Re-testing in order.

Edit: Seems OK - I update Lat-am, go new game, custom & a map is preselected, change to generated and go -> it's a fresh pangaea... Reopen if you find a sequence of steps to fail.

Edit 2: Nope, there's still a wrong assumption in there: GameOptionsTable.updateRuleset should call modCheckboxes.updateSelection before modCheckboxes.setBaseRuleset - I assumed the latter would include the former but no, it actually relied on the checkboxes instead of on gameParameters.mods. Also, need to look into why mod-specific players stay when their mod is deselected - I thought there was a cleanup code for that?

Edit 3: Aaand - there's the thing with mutability. The mods reference ModCheckboxTable keeps does get out of date when GameOptionsTable's var gameParameters is overwritten.

But - @Caballero-Arepa - your maps do not consistently select Lat-am as mod (what the map editor saves as mod selection): America XXL no, America XL yes. And there's too many starting locations for non-Latam civs to really appreciate a "take nations from map" button. Aaand - I think we'll need to spruce up starting locations in the editor to have a "On new game: ignore / AI / human" attribute that can control such a button. Aaaand - wouldn't it be nice to have a mod option to specify which map will be preselected (unzip on Linux won't preserve timestamps from the zip, even if github did meaningful ones, as we do a move that overwrites them)? Some futures...