From 9f3f6ea0ca0def20e8b6dd54227a5fb598cef026 Mon Sep 17 00:00:00 2001 From: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com> Date: Thu, 26 Jun 2025 22:46:06 +0200 Subject: [PATCH] Improve ModChecker UI (#13507) * Minor linting * Move error severity icon to where the color is, fixing "Orange-level bad" being a "positive" Checkmark * Move 'open unique builder' to helper, fixing the premature expensive instantiation of another copy of the combined ruleset * Move the per-mod UI code to helper function for readability * Make background coroutine cancellable and use that * Fix base out of sync between SelectBox and actual check after autoUpdateUniques * Make changes to search while a check is running work properly * Use LoadingImage instead of moving label to indicate in progress * Skip mods that we know won't work with the current base selection * Default base to check against: determined automatically * Add lost check for changed search while a check is still running * Cleaner Job Cleanup --- .../jsons/translations/template.properties | 3 + .../ruleset/validation/ModCompatibility.kt | 8 + .../ruleset/validation/RulesetErrorList.kt | 12 +- .../unciv/ui/popups/options/ModCheckTab.kt | 280 ++++++++++++------ 4 files changed, 206 insertions(+), 97 deletions(-) diff --git a/android/assets/jsons/translations/template.properties b/android/assets/jsons/translations/template.properties index 2caa257035..87c955bd00 100644 --- a/android/assets/jsons/translations/template.properties +++ b/android/assets/jsons/translations/template.properties @@ -943,7 +943,10 @@ Hit the desired key now = Locate mod errors = Check extension mods based on: = -none- = +-declared requirements- = +Requirements could not be determined.\nChoose a base to check this Mod. = Reload mods = +# Currently unused Checking mods for errors... = No problems found. = Autoupdate mod uniques = diff --git a/core/src/com/unciv/models/ruleset/validation/ModCompatibility.kt b/core/src/com/unciv/models/ruleset/validation/ModCompatibility.kt index ee465cbaaf..ed9ea60276 100644 --- a/core/src/com/unciv/models/ruleset/validation/ModCompatibility.kt +++ b/core/src/com/unciv/models/ruleset/validation/ModCompatibility.kt @@ -56,6 +56,14 @@ object ModCompatibility { && mod.name.isNotBlank() && !mod.modOptions.hasUnique(UniqueType.ModIsAudioVisualOnly) + fun isConstantsOnly(mod: Ruleset): Boolean { + val folder = mod.folderLocation ?: return false + if (folder.list("atlas").isNotEmpty()) return false + val jsonFolder = folder.child("jsons") + if (!jsonFolder.exists() || !jsonFolder.isDirectory) return false + return jsonFolder.list().map { it.name() } == listOf("ModOptions.json") + } + fun modNameFilter(modName: String, filter: String): Boolean { if (modName == filter) return true if (filter.length < 3 || !filter.startsWith('*') || !filter.endsWith('*')) return false diff --git a/core/src/com/unciv/models/ruleset/validation/RulesetErrorList.kt b/core/src/com/unciv/models/ruleset/validation/RulesetErrorList.kt index b02d648d0d..af87eb2eb5 100644 --- a/core/src/com/unciv/models/ruleset/validation/RulesetErrorList.kt +++ b/core/src/com/unciv/models/ruleset/validation/RulesetErrorList.kt @@ -9,15 +9,15 @@ import com.unciv.models.ruleset.unique.UniqueType class RulesetError(val text: String, val errorSeverityToReport: RulesetErrorSeverity) -enum class RulesetErrorSeverity(val color: Color) { +enum class RulesetErrorSeverity(val color: Color, val iconName: String) { /** Only appears in mod checker - used for possible misspellings, etc */ - OK(Color.GREEN), + OK(Color.GREEN, "OtherIcons/Checkmark"), /** Only appears in mod checker */ - WarningOptionsOnly(Color.YELLOW), - Warning(Color.YELLOW), + WarningOptionsOnly(Color.YELLOW, "OtherIcons/ExclamationMark"), + Warning(Color.YELLOW, "OtherIcons/ExclamationMark"), /** Only appears in mod checker */ - ErrorOptionsOnly(Color.ORANGE), - Error(Color.RED), + ErrorOptionsOnly(Color.ORANGE, "OtherIcons/ExclamationMark"), + Error(Color.RED, "OtherIcons/Stop"), } /** diff --git a/core/src/com/unciv/ui/popups/options/ModCheckTab.kt b/core/src/com/unciv/ui/popups/options/ModCheckTab.kt index 0882ff3662..eb606093a3 100644 --- a/core/src/com/unciv/ui/popups/options/ModCheckTab.kt +++ b/core/src/com/unciv/ui/popups/options/ModCheckTab.kt @@ -5,18 +5,25 @@ import com.badlogic.gdx.graphics.Color import com.badlogic.gdx.scenes.scene2d.ui.Label import com.badlogic.gdx.scenes.scene2d.ui.Table import com.badlogic.gdx.utils.Align +import com.unciv.Constants import com.unciv.UncivGame import com.unciv.models.ruleset.Ruleset import com.unciv.models.ruleset.RulesetCache +import com.unciv.models.ruleset.unique.UniqueType +import com.unciv.models.ruleset.validation.ModCompatibility +import com.unciv.models.ruleset.validation.RulesetErrorList import com.unciv.models.ruleset.validation.RulesetErrorSeverity import com.unciv.models.ruleset.validation.UniqueAutoUpdater import com.unciv.models.translations.tr +import com.unciv.ui.components.UncivTooltip.Companion.addTooltip +import com.unciv.ui.components.extensions.setSize import com.unciv.ui.components.extensions.surroundWithCircle import com.unciv.ui.components.extensions.toLabel import com.unciv.ui.components.extensions.toTextButton import com.unciv.ui.components.input.onChange import com.unciv.ui.components.input.onClick import com.unciv.ui.components.widgets.ExpanderTab +import com.unciv.ui.components.widgets.LoadingImage import com.unciv.ui.components.widgets.TabbedPager import com.unciv.ui.components.widgets.TranslatedSelectBox import com.unciv.ui.components.widgets.UncivTextField @@ -26,9 +33,12 @@ import com.unciv.ui.screens.UniqueBuilderScreen import com.unciv.ui.screens.basescreen.BaseScreen import com.unciv.utils.Concurrency import com.unciv.utils.launchOnGLThread +import kotlinx.coroutines.Job +import kotlinx.coroutines.isActive private const val MOD_CHECK_WITHOUT_BASE = "-none-" +private const val MOD_CHECK_DYNAMIC_BASE = "-declared requirements-" class ModCheckTab( val screen: BaseScreen @@ -41,38 +51,44 @@ class ModCheckTab( private var modCheckBaseSelect: TranslatedSelectBox? = null private val searchModsTextField: UncivTextField private val modCheckResultTable = Table() - /** Used to repolulate modCheckResultTable when searching */ + private val loadingImage = LoadingImage(48f, LoadingImage.Style(loadingColor = Color.SCARLET)) + /** Used to repopulate modCheckResultTable when searching */ private val modResultExpanderTabs = ArrayList() + private var runningCheck: Job? = null + + /** The search filter the check was started with*/ + private var checkedFilter = "" + /** The current search filter, kept up to date in the `TextField.onChange` event*/ + private var currentFilter = "" + + private val emptyRuleset = Ruleset() + init { defaults().pad(10f).align(Align.top) fixedContent.defaults().pad(10f).align(Align.top) val reloadModsButton = "Reload mods".toTextButton().onClick(::runAction) - fixedContent.add(reloadModsButton).row() - - searchModsTextField = UncivTextField("Search mods") - searchModsTextField.onChange { - modCheckResultTable.clear() - for (expanderTab in modResultExpanderTabs) { - if (expanderTab.title.lowercase().contains(searchModsTextField.text.lowercase())) - modCheckResultTable.add(expanderTab).row() - } - } + fixedContent.add().width(48f) // placeholder to balance out loadingImage + fixedContent.add(reloadModsButton).center().expandX() + fixedContent.add(loadingImage).row() - if (RulesetCache.values.count() > 10) - fixedContent.add(searchModsTextField).fillX().row() + searchModsTextField = UncivTextField("Search mods") + searchModsTextField.onChange { changeSearch() } + + if (RulesetCache.size > 10) + fixedContent.add(searchModsTextField).colspan(3).fillX().row() val labeledBaseSelect = Table().apply { add("Check extension mods based on:".toLabel()).padRight(10f) - val baseMods = listOf(MOD_CHECK_WITHOUT_BASE) + RulesetCache.getSortedBaseRulesets() - modCheckBaseSelect = TranslatedSelectBox(baseMods, MOD_CHECK_WITHOUT_BASE).apply { - selectedIndex = 0 + val baseMods = listOf(MOD_CHECK_WITHOUT_BASE, MOD_CHECK_DYNAMIC_BASE) + RulesetCache.getSortedBaseRulesets() + modCheckBaseSelect = TranslatedSelectBox(baseMods, MOD_CHECK_DYNAMIC_BASE).apply { + selectedIndex = 1 onChange { runAction() } } add(modCheckBaseSelect) } - fixedContent.add(labeledBaseSelect).row() + fixedContent.add(labeledBaseSelect).colspan(3).row() add(modCheckResultTable) } @@ -88,113 +104,195 @@ class ModCheckTab( runAction() } - private fun runModChecker(base: String = MOD_CHECK_WITHOUT_BASE) { + override fun deactivated(index: Int, caption: String, pager: TabbedPager) { + cancelJob() + } + + private fun cancelJob() { + val job = runningCheck ?: return + endJob() + job.cancel() + } + + private fun endJob() { + runningCheck = null + loadingImage.hide() + } + + private fun runModChecker(base: String = MOD_CHECK_DYNAMIC_BASE) { + cancelJob() modCheckFirstRun = false if (modCheckBaseSelect == null) return - val openedExpanderTitles = modCheckResultTable.children.filterIsInstance() + loadingImage.show() + + val openedExpanderTitles = modResultExpanderTabs .filter { it.isOpen }.map { it.title }.toSet() modCheckResultTable.clear() modResultExpanderTabs.clear() - val rulesetErrors = RulesetCache.loadRulesets() - if (rulesetErrors.isNotEmpty()) { + val loadingErrors = RulesetCache.loadRulesets() + if (loadingErrors.isNotEmpty()) { val errorTable = Table().apply { defaults().pad(2f) } - for (rulesetError in rulesetErrors) - errorTable.add(rulesetError.toLabel()).width(stage.width / 2).row() + for (loadingError in loadingErrors) + errorTable.add(loadingError.toLabel()).width(stage.width / 2).row() modCheckResultTable.add(errorTable) } - modCheckResultTable.add("Checking mods for errors...".toLabel()).row() - modCheckBaseSelect!!.isDisabled = true - - Concurrency.run("ModChecker") { - for (mod in RulesetCache.values - .filter { it.name.lowercase().contains(searchModsTextField.text.lowercase()) } - .sortedBy { it.name }.sortedByDescending { it.name in openedExpanderTitles }) { - if (base != MOD_CHECK_WITHOUT_BASE && mod.modOptions.isBaseRuleset) continue + runningCheck = Concurrency.run("ModChecker") { + checkedFilter = searchModsTextField.text + currentFilter = checkedFilter + val modsToCheck = RulesetCache.values + .filter { it.name.filterApplies() } + .sortedWith( + compareByDescending { it.name in openedExpanderTitles } + .thenBy { it.name } + ) + for (mod in modsToCheck) { + val baseForThisMod = if (base != MOD_CHECK_DYNAMIC_BASE) base else getBaseForMod(mod) + if (baseForThisMod == null) { + // Don't check, but since this is the default view, show greyed out, so people don't wonder where their mods are + launchOnGLThread { + addDisabledPlaceholder(mod) + } + continue + } + if (!shouldCheckMod(mod, baseForThisMod)) continue + if (!isActive) break val modLinks = - if (base == MOD_CHECK_WITHOUT_BASE) mod.getErrorList(tryFixUnknownUniques = true) - else RulesetCache.checkCombinedModLinks(linkedSetOf(mod.name), base, tryFixUnknownUniques = true) + if (baseForThisMod == MOD_CHECK_WITHOUT_BASE) mod.getErrorList(tryFixUnknownUniques = true) + else RulesetCache.checkCombinedModLinks(linkedSetOf(mod.name), baseForThisMod, tryFixUnknownUniques = true) + if (!isActive) break + modLinks.sortByDescending { it.errorSeverityToReport } - val noProblem = !modLinks.isNotOK() if (modLinks.isNotEmpty()) modLinks.add("", RulesetErrorSeverity.OK, sourceObject = null) - if (noProblem) modLinks.add("No problems found.".tr(), RulesetErrorSeverity.OK, sourceObject = null) + if (!modLinks.isNotOK()) modLinks.add("No problems found.".tr(), RulesetErrorSeverity.OK, sourceObject = null) launchOnGLThread { - // When the options popup is already closed before this postRunnable is run, - // Don't add the labels, as otherwise the game will crash - if (stage == null) return@launchOnGLThread - // Don't just render text, since that will make all the conditionals in the mod replacement messages move to the end, which makes it unreadable - // Don't use .toLabel() either, since that activates translations as well, which is what we're trying to avoid, - // Instead, some manual work needs to be put in. - - val iconColor = modLinks.getFinalSeverity().color - val iconName = when (iconColor) { - Color.RED -> "OtherIcons/Stop" - Color.YELLOW -> "OtherIcons/ExclamationMark" - else -> "OtherIcons/Checkmark" - } - val icon = ImageGetter.getImage(iconName) - .apply { color = ImageGetter.CHARCOAL } - .surroundWithCircle(30f, color = iconColor) - - val expanderTab = ExpanderTab(mod.name, icon = icon, startsOutOpened = mod.name in openedExpanderTitles) { - it.defaults().align(Align.left) - it.defaults().pad(10f) - - val openUniqueBuilderButton = "Open unique builder".toTextButton() - val ruleset = if (base == MOD_CHECK_WITHOUT_BASE) mod - else RulesetCache.getComplexRuleset(linkedSetOf(mod.name), base) - openUniqueBuilderButton.onClick { UncivGame.Current.pushScreen(UniqueBuilderScreen(ruleset)) } - it.add(openUniqueBuilderButton).row() - - if (!noProblem && mod.folderLocation != null) { - val replaceableUniques = UniqueAutoUpdater.getDeprecatedReplaceableUniques(mod) - if (replaceableUniques.isNotEmpty()) - it.add("Autoupdate mod uniques".toTextButton() - .onClick { autoUpdateUniques(screen, mod, replaceableUniques) }).row() - } - for (line in modLinks) { - val label = Label(line.text, BaseScreen.skin) - .apply { color = line.errorSeverityToReport.color } - label.wrap = true - it.add(label).width(stage.width / 2).row() - } - if (!noProblem) - it.add("Copy to clipboard".toTextButton().onClick { - Gdx.app.clipboard.contents = modLinks - .joinToString("\n") { line -> line.text } - }).row() - } - expanderTab.header.left() - modResultExpanderTabs.add(expanderTab) - - val loadingLabel = modCheckResultTable.children.last() - modCheckResultTable.removeActor(loadingLabel) - modCheckResultTable.add(expanderTab).row() - modCheckResultTable.add(loadingLabel).row() + addModResult(mod, baseForThisMod, modLinks, mod.name in openedExpanderTitles) } } // done with all mods! launchOnGLThread { - modCheckResultTable.removeActor(modCheckResultTable.children.last()) - modCheckBaseSelect!!.isDisabled = false + endJob() } } } + private fun changeSearch() { + currentFilter = searchModsTextField.text + if (currentFilter.contains(checkedFilter, ignoreCase = true)) { + // The last check, whether finished or not, included all mods we want to filter + synchronized(modCheckResultTable) { + modCheckResultTable.clear() + for (expanderTab in modResultExpanderTabs) { + if (expanderTab.title.filterApplies()) + modCheckResultTable.add(expanderTab).row() + } + } + } else { + // the filter is wider than the last check - rerun + runAction() + } + } + + private fun String.filterApplies() = contains(currentFilter, ignoreCase = true) + + /** Use the declarative mod compatibility Uniques to omit meaningless check combos */ + private fun shouldCheckMod(mod: Ruleset, base: String): Boolean { + if (mod.modOptions.isBaseRuleset) return base == MOD_CHECK_WITHOUT_BASE + if (ModCompatibility.isAudioVisualMod(mod)) return true + val baseRuleset = RulesetCache[base] ?: emptyRuleset // MOD_CHECK_WITHOUT_BASE compares compatibility against an empty Ruleset + return ModCompatibility.meetsBaseRequirements(mod, baseRuleset) // yes this returns true for mods ignoring declarative compatibility + } + + private fun getBaseForMod(mod: Ruleset): String? { + if (mod.modOptions.isBaseRuleset || ModCompatibility.isAudioVisualMod(mod) || ModCompatibility.isConstantsOnly(mod)) + return MOD_CHECK_WITHOUT_BASE + if (!mod.modOptions.hasUnique(UniqueType.ModRequires)) return null + return RulesetCache.values + .filter { it.modOptions.isBaseRuleset } + .firstOrNull { ModCompatibility.meetsBaseRequirements(mod, it) } + ?.name + } + + private fun addModResult(mod: Ruleset, base: String, modLinks: RulesetErrorList, startsOutOpened: Boolean) { + // When the options popup is already closed before this postRunnable is run, + // Don't add the labels, as otherwise the game will crash + if (stage == null) return + // Don't just render text, since that will make all the conditionals in the mod replacement messages move to the end, which makes it unreadable + // Don't use .toLabel() either, since that activates translations as well, which is what we're trying to avoid, + // Instead, some manual work needs to be put in. + + val severity = modLinks.getFinalSeverity() + val icon = ImageGetter.getImage(severity.iconName) + .apply { color = ImageGetter.CHARCOAL } + .surroundWithCircle(30f, color = severity.color) + + val expanderTab = ExpanderTab(mod.name, icon = icon, startsOutOpened = startsOutOpened) { + it.defaults().align(Align.left) + it.defaults().pad(10f) + + val openUniqueBuilderButton = "Open unique builder".toTextButton() + openUniqueBuilderButton.onClick { openUniqueBuilder(mod, base) } + it.add(openUniqueBuilderButton).row() + + if (severity != RulesetErrorSeverity.OK && mod.folderLocation != null) { + val replaceableUniques = UniqueAutoUpdater.getDeprecatedReplaceableUniques(mod) + if (replaceableUniques.isNotEmpty()) + it.add("Autoupdate mod uniques".toTextButton() + .onClick { autoUpdateUniques(screen, mod, replaceableUniques) }).row() + } + for (line in modLinks) { + val label = Label(line.text, BaseScreen.skin) + .apply { color = line.errorSeverityToReport.color } + label.wrap = true + it.add(label).width(stage.width / 2).row() + } + if (severity != RulesetErrorSeverity.OK) + it.add("Copy to clipboard".toTextButton().onClick { + Gdx.app.clipboard.contents = modLinks + .joinToString("\n") { line -> line.text } + }).row() + } + expanderTab.header.left() + + synchronized(modCheckResultTable) { + modResultExpanderTabs.add(expanderTab) + if (mod.name.filterApplies()) + modCheckResultTable.add(expanderTab).row() + } + } + + private fun addDisabledPlaceholder(mod: Ruleset) { + if (stage == null) return + val table = Table(BaseScreen.skin).apply { + defaults().pad(8f) + background = BaseScreen.skinStrings.getUiBackground("General/DisabledBox", tintColor = Color.DARK_GRAY) + add(ImageGetter.getImage("OtherIcons/Question").apply { setSize(33f) }).padRight(10f).growY() + add(mod.name.toLabel(Color.LIGHT_GRAY, Constants.headingFontSize, alignment = Align.left)).left().grow() + addTooltip("Requirements could not be determined.\nChoose a base to check this Mod.", 16f, targetAlign = Align.top) + } + synchronized(modCheckResultTable) { + modCheckResultTable.add(table).growX().row() + } + } + + private fun openUniqueBuilder(mod: Ruleset, base: String) { + val ruleset = if (base == MOD_CHECK_WITHOUT_BASE) mod + else RulesetCache.getComplexRuleset(linkedSetOf(mod.name), base) + UncivGame.Current.pushScreen(UniqueBuilderScreen(ruleset)) + } private fun autoUpdateUniques(screen: BaseScreen, mod: Ruleset, replaceableUniques: HashMap) { UniqueAutoUpdater.autoupdateUniques(mod, replaceableUniques) val toastText = "Uniques updated!" ToastPopup(toastText, screen) - runModChecker() + runAction() } } -