Crash for unique modded unit text?

yairm210 commented 2 years ago

Not sure what mod or unit this is

java.lang.NullPointerException: at com.unciv.models.ruleset.Nation.addUniqueUnitsText (Nation.kt:259) at com.unciv.models.ruleset.Nation.getCivilopediaTextLines (Nation.kt:141) at com.unciv.ui.newgamescreen.NationTable. (NationTable.kt:51) at com.unciv.ui.newgamescreen.NationPickerPopup.setNationDetails (PlayerPickerTable.kt:346) at com.unciv.ui.newgamescreen.NationPickerPopup. (PlayerPickerTable.kt:306) at com.unciv.ui.newgamescreen.PlayerPickerTable.popupNationPicker (PlayerPickerTable.kt:224) at com.unciv.ui.newgamescreen.PlayerPickerTable.access$popupNationPicker (PlayerPickerTable.kt:40) at com.unciv.ui.newgamescreen.PlayerPickerTable$getNationTable$1.invoke (PlayerPickerTable.kt:212) at com.unciv.ui.newgamescreen.PlayerPickerTable$getNationTable$1.invoke (PlayerPickerTable.kt:211) at com.unciv.ui.utils.ExtensionFunctionsKt$onClick$1.invoke (ExtensionFunctions.kt:64) at com.unciv.ui.utils.ExtensionFunctionsKt$onClick$1.invoke (ExtensionFunctions.kt:64) at com.unciv.ui.utils.ExtensionFunctionsKt$onClickEvent$1.clicked (ExtensionFunctions.kt:57) at com.badlogic.gdx.scenes.scene2d.utils.ClickListener.touchUp (ClickListener.java:88) at com.badlogic.gdx.scenes.scene2d.InputListener.handle (InputListener.java:71) at com.badlogic.gdx.scenes.scene2d.Stage.touchUp (Stage.java:355) at com.badlogic.gdx.backends.android.DefaultAndroidInput.processEvents (DefaultAndroidInput.java:425) at com.badlogic.gdx.backends.android.AndroidGraphics.onDrawFrame (AndroidGraphics.java:469) at android.opengl.GLSurfaceView$GLThread.guardedRun (GLSurfaceView.java:1582) at android.opengl.GLSurfaceView$GLThread.run (GLSurfaceView.java:1281)

xlenstra commented 2 years ago

This on first sight looks like a unit has a promotion that doesn't exist in the ruleset, and this leads to a crash when the nation is selected in the start game screen nation picker. Isn't this just a mod problem then?

yairm210 commented 2 years ago

Is that's the case we should error the mod in checkRuleset, not crash the game

xlenstra commented 2 years ago

I've been able to reproduce a crash with basically the same stack trace through the following steps:

Create two extension mods, A and B.
Mod B contains a promotion
Mod A contains a nation with a unique unit using this promotion. Note that this unique unit should replace another unit for the crash to occur
Start a new game
Activate Mod B, then activate Mod A, then disable Mod B
Open the nation picker
Crash ensues, with stack trace below; except for a click instead of a tap to open the picker, the stack trace appears identical

I've included some example mods here as well modA.tar.gz modB.tar.gz

Exception in thread "main" java.lang.NullPointerException
    at com.unciv.models.ruleset.Nation.addUniqueUnitsText(Nation.kt:259)
    at com.unciv.models.ruleset.Nation.getCivilopediaTextLines(Nation.kt:141)
    at com.unciv.ui.newgamescreen.NationTable.<init>(NationTable.kt:51)
    at com.unciv.ui.newgamescreen.NationPickerPopup.setNationDetails(PlayerPickerTable.kt:346)
    at com.unciv.ui.newgamescreen.NationPickerPopup.<init>(PlayerPickerTable.kt:306)
    at com.unciv.ui.newgamescreen.PlayerPickerTable.popupNationPicker(PlayerPickerTable.kt:224)
    at com.unciv.ui.newgamescreen.PlayerPickerTable.access$popupNationPicker(PlayerPickerTable.kt:40)
    at com.unciv.ui.newgamescreen.PlayerPickerTable$getNationTable$1.invoke(PlayerPickerTable.kt:212)
    at com.unciv.ui.newgamescreen.PlayerPickerTable$getNationTable$1.invoke(PlayerPickerTable.kt:211)
    at com.unciv.ui.utils.ExtensionFunctionsKt$onClick$1.invoke(ExtensionFunctions.kt:64)
    at com.unciv.ui.utils.ExtensionFunctionsKt$onClick$1.invoke(ExtensionFunctions.kt:64)
    at com.unciv.ui.utils.ExtensionFunctionsKt$onClickEvent$1.clicked(ExtensionFunctions.kt:57)
    at com.badlogic.gdx.scenes.scene2d.utils.ClickListener.touchUp(ClickListener.java:88)
    at com.badlogic.gdx.scenes.scene2d.InputListener.handle(InputListener.java:71)
    at com.badlogic.gdx.scenes.scene2d.Stage.touchUp(Stage.java:355)
    at com.badlogic.gdx.InputEventQueue.drain(InputEventQueue.java:70)
    at com.badlogic.gdx.backends.lwjgl3.DefaultLwjgl3Input.update(DefaultLwjgl3Input.java:189)
    at com.badlogic.gdx.backends.lwjgl3.Lwjgl3Window.update(Lwjgl3Window.java:394)
    at com.badlogic.gdx.backends.lwjgl3.Lwjgl3Application.loop(Lwjgl3Application.java:143)
    at com.badlogic.gdx.backends.lwjgl3.Lwjgl3Application.<init>(Lwjgl3Application.java:116)
    at com.unciv.app.desktop.DesktopLauncher.main(DesktopLauncher.kt:61)

The question now is, what should be done? To me it seems like extension mods should only depend on base ruleset mods and not on other extension mods. Do we want to enforce that? Alternatively, do we just also want to check mod links after disabling an extension mod, and reverse the disabling if mod links break?

yairm210 commented 2 years ago

Oh interesting! The answer is, we should check mod compatibility when removing mods as well as when adding mods in the screen I don't think we ever thought that removing a mod from a ruleset could cause problems

yairm210 / Unciv

Crash for unique modded unit text? #5790