From d277b0055ebfc1a1e849b794a9c3f7709ca06dde Mon Sep 17 00:00:00 2001 From: Yair Morgenstern Date: Sun, 15 Sep 2024 21:48:46 +0300 Subject: [PATCH] Fix broken start new game (#12215) * Revert "perf: Decrease getUniques(uniqueType) by adding a EnumMap to store uniques by type, with basically zero lookup time :star:" This reverts commit 0533347371a5b566ef40cf894d5f80080d024a6f. * Fix new game screen crashing if you have no scenarios --- changelog.md | 4 -- .../com/unciv/models/ruleset/unique/Unique.kt | 37 ++++------- .../screens/newgamescreen/MapOptionsTable.kt | 63 ++++++++++++------- 3 files changed, 51 insertions(+), 53 deletions(-) diff --git a/changelog.md b/changelog.md index 60d04ba9ff487..4e90f7098de72 100644 --- a/changelog.md +++ b/changelog.md @@ -1,9 +1,5 @@ ## 4.13.8 -Average speed: 28.0 turns/s -Average game duration: 10.714975s -Total time: 7m 8.599s - Allowed starting Scenarios, including Multiplayer, through the "New Game" screen! :D WLTK + continuous rendering no longer cause city tiles to be dimmed diff --git a/core/src/com/unciv/models/ruleset/unique/Unique.kt b/core/src/com/unciv/models/ruleset/unique/Unique.kt index ada7df7e42bec..abd8f794c6306 100644 --- a/core/src/com/unciv/models/ruleset/unique/Unique.kt +++ b/core/src/com/unciv/models/ruleset/unique/Unique.kt @@ -12,7 +12,6 @@ import com.unciv.models.translations.getModifiers import com.unciv.models.translations.getPlaceholderParameters import com.unciv.models.translations.getPlaceholderText import com.unciv.models.translations.removeConditionals -import java.util.EnumMap class Unique(val text: String, val sourceObjectType: UniqueTarget? = null, val sourceObjectName: String? = null) { @@ -244,27 +243,22 @@ class LocalUniqueCache(val cache: Boolean = true) { } } -open class UniqueMap() { - protected val innerUniqueMap = HashMap>() - - // *shares* the list of uniques with the other map, to save on memory and allocations - // This is a memory/speed tradeoff, since there are *600 unique types*, - // 750 including deprecated, and EnumMap creates a N-sized array where N is the number of objects in the enum - val typedUniqueMap = EnumMap>(UniqueType::class.java) +class UniqueMap() { + //todo Once all untyped Uniques are converted, this should be HashMap + // For now, we can have both map types "side by side" each serving their own purpose, + // and gradually this one will be deprecated in favor of the other + + private val innerUniqueMap = HashMap>() constructor(uniques: Sequence) : this() { addUniques(uniques.asIterable()) } /** Adds one [unique] unless it has a ConditionalTimedUnique conditional */ - open fun addUnique(unique: Unique) { + fun addUnique(unique: Unique) { val existingArrayList = innerUniqueMap[unique.placeholderText] if (existingArrayList != null) existingArrayList.add(unique) else innerUniqueMap[unique.placeholderText] = arrayListOf(unique) - - if (unique.type == null) return - if (typedUniqueMap[unique.type] != null) return - typedUniqueMap[unique.type] = innerUniqueMap[unique.placeholderText] } /** Calls [addUnique] on each item from [uniques] */ @@ -287,21 +281,13 @@ open class UniqueMap() { fun hasTagUnique(tagUnique: String) = innerUniqueMap.containsKey(tagUnique) - // 160ms vs 1000-1250ms/30s - fun getUniques(uniqueType: UniqueType) = typedUniqueMap[uniqueType] - ?.asSequence() - ?: emptySequence() + fun getUniques(uniqueType: UniqueType) = + innerUniqueMap[uniqueType.placeholderText]?.asSequence() ?: emptySequence() fun getMatchingUniques(uniqueType: UniqueType, state: StateForConditionals = StateForConditionals.EmptyState) = getUniques(uniqueType) - // Same as .filter | .flatMap, but more cpu/mem performant (7.7 GB vs ?? for test) - .flatMap { - when { - it.isTimedTriggerable -> emptySequence() - !it.conditionalsApply(state) -> emptySequence() - else -> it.getMultiplied(state) - } - } + .filter { it.conditionalsApply(state) && !it.isTimedTriggerable } + .flatMap { it.getMultiplied(state) } fun hasMatchingUnique(uniqueType: UniqueType, state: StateForConditionals = StateForConditionals.EmptyState) = getUniques(uniqueType).any { it.conditionalsApply(state) } @@ -315,6 +301,7 @@ open class UniqueMap() { } } + class TemporaryUnique() : IsPartOfGameInfoSerialization { constructor(uniqueObject: Unique, turns: Int) : this() { diff --git a/core/src/com/unciv/ui/screens/newgamescreen/MapOptionsTable.kt b/core/src/com/unciv/ui/screens/newgamescreen/MapOptionsTable.kt index a0a07313ec35d..bcb4dbe44bf0d 100644 --- a/core/src/com/unciv/ui/screens/newgamescreen/MapOptionsTable.kt +++ b/core/src/com/unciv/ui/screens/newgamescreen/MapOptionsTable.kt @@ -6,12 +6,14 @@ import com.unciv.Constants import com.unciv.logic.GameInfoPreview import com.unciv.logic.map.MapGeneratedMainType import com.unciv.logic.map.MapParameters +import com.unciv.models.ruleset.Ruleset import com.unciv.ui.components.extensions.toLabel import com.unciv.ui.components.input.onChange import com.unciv.ui.components.widgets.TranslatedSelectBox import com.unciv.ui.screens.basescreen.BaseScreen +import com.unciv.utils.Concurrency -class ScenarioSelectTable(newGameScreen: NewGameScreen, mapParameters: MapParameters) : Table() { +class ScenarioSelectTable(val newGameScreen: NewGameScreen, mapParameters: MapParameters) : Table() { data class ScenarioData(val name:String, val file: FileHandle){ var preview: GameInfoPreview? = null @@ -19,34 +21,46 @@ class ScenarioSelectTable(newGameScreen: NewGameScreen, mapParameters: MapParame val scenarios = HashMap() lateinit var selectedScenario: ScenarioData + var scenarioSelectBox: TranslatedSelectBox? = null init { - // TODO make this async so it's not slow - val scenarioFiles = newGameScreen.game.files.getScenarioFiles() - for ((file, ruleset) in scenarioFiles) - scenarios[file.name()] = ScenarioData(file.name(), file) - - val scenarioSelectBox = TranslatedSelectBox(scenarios.keys, scenarios.keys.first()) - - fun selectScenario(){ - val scenario = scenarios[scenarioSelectBox.selected.value]!! - val preload = if (scenario.preview != null) scenario.preview!! else { - val preview = newGameScreen.game.files.loadGamePreviewFromFile(scenario.file) - scenario.preview = preview - preview + // Only the first so it's fast + val firstScenarioFile = newGameScreen.game.files.getScenarioFiles().firstOrNull() + if (firstScenarioFile != null) { + createScenarioSelectBox(listOf(firstScenarioFile)) + Concurrency.run { + val scenarioFiles = newGameScreen.game.files.getScenarioFiles().toList() + Concurrency.runOnGLThread { + createScenarioSelectBox(scenarioFiles) + } } - newGameScreen.gameSetupInfo.gameParameters.players = preload.gameParameters.players - .apply { removeAll { it.chosenCiv == Constants.spectator } } - newGameScreen.gameSetupInfo.gameParameters.baseRuleset = preload.gameParameters.baseRuleset - newGameScreen.gameSetupInfo.gameParameters.mods = preload.gameParameters.mods - newGameScreen.tryUpdateRuleset(true) - newGameScreen.playerPickerTable.update() - selectedScenario = scenario } - - scenarioSelectBox.onChange { selectScenario() } + } + + private fun createScenarioSelectBox(scenarioFiles: List>) { + for ((file, ruleset) in scenarioFiles) + scenarios[file.name()] = ScenarioData(file.name(), file) + + scenarioSelectBox = TranslatedSelectBox(scenarios.keys.sorted(), scenarios.keys.first()) + scenarioSelectBox!!.onChange { selectScenario() } + clear() add(scenarioSelectBox) - selectScenario() + } + + fun selectScenario(){ + val scenario = scenarios[scenarioSelectBox!!.selected.value]!! + val preload = if (scenario.preview != null) scenario.preview!! else { + val preview = newGameScreen.game.files.loadGamePreviewFromFile(scenario.file) + scenario.preview = preview + preview + } + newGameScreen.gameSetupInfo.gameParameters.players = preload.gameParameters.players + .apply { removeAll { it.chosenCiv == Constants.spectator } } + newGameScreen.gameSetupInfo.gameParameters.baseRuleset = preload.gameParameters.baseRuleset + newGameScreen.gameSetupInfo.gameParameters.mods = preload.gameParameters.mods + newGameScreen.tryUpdateRuleset(true) + newGameScreen.playerPickerTable.update() + selectedScenario = scenario } } @@ -94,6 +108,7 @@ class MapOptionsTable(private val newGameScreen: NewGameScreen) : Table() { MapGeneratedMainType.scenario -> { mapParameters.name = "" mapTypeSpecificTable.add(scenarioOptionsTable) + scenarioOptionsTable.selectScenario() newGameScreen.lockTables() } }