From 7aa7b8a4b224c08f3caf2dd8d9b109a5dbdfaeb1 Mon Sep 17 00:00:00 2001
From: Yair Morgenstern <yairm210@hotmail.com>
Date: Fri, 17 Dec 2021 08:47:07 +0200
Subject: [PATCH] Resolved #5790 - kudos to @xlenstra for finding the problem
 and providing reproduction! (#5812)

---
 .../ui/newgamescreen/ModCheckboxTable.kt      | 50 ++++++++++++++-----
 .../com/unciv/ui/utils/ExtensionFunctions.kt  |  4 +-
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/core/src/com/unciv/ui/newgamescreen/ModCheckboxTable.kt b/core/src/com/unciv/ui/newgamescreen/ModCheckboxTable.kt
index ac67989364..04bde0e0f4 100644
--- a/core/src/com/unciv/ui/newgamescreen/ModCheckboxTable.kt
+++ b/core/src/com/unciv/ui/newgamescreen/ModCheckboxTable.kt
@@ -2,6 +2,7 @@ package com.unciv.ui.newgamescreen
 
 import com.badlogic.gdx.scenes.scene2d.ui.CheckBox
 import com.badlogic.gdx.scenes.scene2d.ui.Table
+import com.badlogic.gdx.scenes.scene2d.utils.ChangeListener
 import com.unciv.models.ruleset.Ruleset
 import com.unciv.models.ruleset.RulesetCache
 import com.unciv.models.translations.tr
@@ -23,7 +24,7 @@ class ModCheckboxTable(
         for (mod in modRulesets.sortedBy { it.name }) {
             val checkBox = mod.name.toCheckBox(mod.name in mods)
             checkBox.onChange {
-                if (checkBoxChanged(checkBox, mod)) {
+                if (checkBoxChanged(checkBox, it!!, mod)) {
                     //todo: persist ExpanderTab states here
                     onUpdate(mod.name)
                 }
@@ -48,7 +49,22 @@ class ModCheckboxTable(
         }
     }
 
-    private fun checkBoxChanged(checkBox: CheckBox, mod: Ruleset): Boolean {
+
+    fun popupToastError(rulesetErrorList:Ruleset.RulesetErrorList) {
+        val initialText =
+            if (rulesetErrorList.isError()) "The mod combination you selected is incorrectly defined!".tr()
+            else "{The mod combination you selected has problems.}\n{You can play it, but don't expect everything to work!}".tr()
+        val toastMessage =  "$initialText\n\n${rulesetErrorList.getErrorText()}"
+
+        lastToast?.close()
+        lastToast = ToastPopup(toastMessage, screen, 5000L)
+    }
+
+    private fun checkBoxChanged(
+        checkBox: CheckBox,
+        changeEvent: ChangeListener.ChangeEvent,
+        mod: Ruleset
+    ): Boolean {
         if (checkBox.isChecked) {
             // First the quick standalone check
             val modLinkErrors = mod.checkModLinks()
@@ -57,7 +73,7 @@ class ModCheckboxTable(
                 val toastMessage =
                     "The mod you selected is incorrectly defined!".tr() + "\n\n${modLinkErrors.getErrorText()}"
                 lastToast = ToastPopup(toastMessage, screen, 5000L)
-                checkBox.isChecked = false
+                changeEvent.cancel() // Cancel event to reset to previous state - see Button.setChecked()
                 return false
             }
 
@@ -66,23 +82,31 @@ class ModCheckboxTable(
             // Check over complete combination of selected mods
             val complexModLinkCheck = RulesetCache.checkCombinedModLinks(mods, baseRuleset)
             if (complexModLinkCheck.isWarnUser()) {
-                lastToast?.close()
-                val toastMessage = (
-                    if (complexModLinkCheck.isError()) "The mod combination you selected is incorrectly defined!".tr()
-                    else "{The mod combination you selected has problems.}\n{You can play it, but don't expect everything to work!}".tr()
-                ) + 
-                    "\n\n${complexModLinkCheck.getErrorText()}"
-                
-                lastToast = ToastPopup(toastMessage, screen, 5000L)
-
+                popupToastError(complexModLinkCheck)
                 if (complexModLinkCheck.isError()) {
-                    checkBox.isChecked = false
+                    changeEvent.cancel() // Cancel event to reset to previous state - see Button.setChecked()
+                    mods.remove(mod.name)
                     return false
                 }
             }
 
         } else {
+            /**
+             * Turns out we need to check ruleset when REMOVING a mod as well, since if mod A references something in mod B (like a promotion),
+             *   and then we remove mod B, then the entire ruleset is now broken!
+             */
+
             mods.remove(mod.name)
+            val complexModLinkCheck = RulesetCache.checkCombinedModLinks(mods, baseRuleset)
+            if (complexModLinkCheck.isWarnUser()) {
+                popupToastError(complexModLinkCheck)
+                if (complexModLinkCheck.isError()) {
+                    changeEvent.cancel() // Cancel event to reset to previous state - see Button.setChecked()
+                    mods.add(mod.name)
+                    return false
+                }
+            }
+
         }
 
         return true
diff --git a/core/src/com/unciv/ui/utils/ExtensionFunctions.kt b/core/src/com/unciv/ui/utils/ExtensionFunctions.kt
index c9a1cdd6e5..213705863b 100644
--- a/core/src/com/unciv/ui/utils/ExtensionFunctions.kt
+++ b/core/src/com/unciv/ui/utils/ExtensionFunctions.kt
@@ -69,10 +69,10 @@ fun Actor.onClick(function: () -> Unit): Actor {
     return this
 }
 
-fun Actor.onChange(function: () -> Unit): Actor {
+fun Actor.onChange(function: (event: ChangeListener.ChangeEvent?) -> Unit): Actor {
     this.addListener(object : ChangeListener() {
         override fun changed(event: ChangeEvent?, actor: Actor?) {
-            function()
+            function(event)
         }
     })
     return this