Fix NullPointerException on founding a pantheon

[dHannasch]

Unciv crashes at https://github.com/yairm210/Unciv/blob/master/core/src/com/unciv/logic/civilization/managers/ReligionManager.kt#L378 because religion is needed to be non-null and is null (because it has not yet been created).

Maybe setting ReligionState needs to be down at https://github.com/yairm210/Unciv/blob/master/core/src/com/unciv/logic/civilization/managers/ReligionManager.kt#L390, I don't know, I don't really follow the logic there. But I think foundPantheon needs to happen before religion is used, because that's what makes religion non-null.

2024-01-07T21:58:35.775775Z [main] [UncivGame] [ERROR] Uncaught throwable | java.lang.NullPointerException
        at com.unciv.logic.civilization.managers.ReligionManager.chooseBeliefs(ReligionManager.kt:379)
        at com.unciv.ui.screens.pickerscreens.PantheonPickerScreen$3.invoke(PantheonPickerScreen.kt:36)
        at com.unciv.ui.screens.pickerscreens.PantheonPickerScreen$3.invoke(PantheonPickerScreen.kt:35)
        at com.unciv.ui.screens.pickerscreens.ReligionPickerScreenCommon$setOKAction$1.invoke(ReligionPickerScreenCommon.kt:68)
        at com.unciv.ui.screens.pickerscreens.ReligionPickerScreenCommon$setOKAction$1.invoke(ReligionPickerScreenCommon.kt:67)
        at com.unciv.ui.components.input.ActivationActionMap.activate(ActivationActionMap.kt:56)
        at com.unciv.ui.components.input.ActorAttachments.activate(ActorAttachments.kt:42)
        at com.unciv.ui.components.input.ActivationExtensionsKt.activate(ActivationExtensions.kt:17)
        at com.unciv.ui.components.input.ActivationListener.tap(ActivationListener.kt:15)
        at com.badlogic.gdx.scenes.scene2d.utils.ActorGestureListener$1.tap(ActorGestureListener.java:52)
        at com.badlogic.gdx.input.GestureDetector.touchUp(GestureDetector.java:206)
        at com.badlogic.gdx.scenes.scene2d.utils.ActorGestureListener.handle(ActorGestureListener.java:125)
        at com.badlogic.gdx.scenes.scene2d.Stage.touchUp(Stage.java:354)
        at com.unciv.ui.screens.basescreen.UncivStage.access$touchUp$s80204510(UncivStage.kt:17)
        at com.unciv.ui.screens.basescreen.UncivStage$touchUp$1.invoke(UncivStage.kt:87)
        at com.unciv.ui.screens.basescreen.UncivStage$touchUp$1.invoke(UncivStage.kt:87)
        at com.unciv.ui.crashhandling.CrashHandlingExtensionsKt$wrapCrashHandling$1.invoke(CrashHandlingExtensions.kt:17)
        at com.unciv.ui.screens.basescreen.UncivStage.touchUp(UncivStage.kt:87)
        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:378)
        at com.badlogic.gdx.backends.lwjgl3.Lwjgl3Application.loop(Lwjgl3Application.java:193)
        at com.badlogic.gdx.backends.lwjgl3.Lwjgl3Application.<init>(Lwjgl3Application.java:167)
        at com.unciv.app.desktop.HardenGdxAudio.<init>(HardenGdxAudio.kt:46)
        at com.unciv.app.desktop.DesktopLauncher.main(DesktopLauncher.kt:76)

This branch makes the NullPointerException not happen.

[SomeTroglodyte]

Must have been a regression by #10877 - please cross-check?

[dHannasch]

@SomeTroglodyte Sorry, what does "cross-check" mean?

[SomeTroglodyte]

It means please read that PR, and check if it indeed broke this, and if it did why and whether its intention is preserved by this.

[dHannasch]

Thank you!

Yes, the NullPointerException did indeed start when #10877 was merged. The reason why is because the foundPantheon call creates the religion, specifically at https://github.com/yairm210/Unciv/blob/master/core/src/com/unciv/logic/civilization/managers/ReligionManager.kt#L134, and the religion needs to be non-null at https://github.com/yairm210/Unciv/blob/master/core/src/com/unciv/logic/civilization/managers/ReligionManager.kt#L378. Presumably this was the reason for the comment line

        // add beliefs (religion exists at this point)

Prior to #10877, that comment line was immediately below the block that creates the religion by calling foundPantheon. They were originally added together by https://github.com/yairm210/Unciv/blob/master/core/src/com/unciv/logic/civilization/managers/ReligionManager.kt#L134.

I've fiddled with https://github.com/yairm210/Unciv/blob/master/core/src/com/unciv/logic/civilization/managers/ReligionManager.kt#L390 for purely textual symmetry, making all three clauses in the when look the same. (This meant removing the religionState setting out of foundPantheon, but if it's fine to move the religionState setting for the other cases, I'm guessing it will be fine for this? To tell the truth I really do not understand what the religionState is for.) Beyond that, I don't really know the intention behind moving the foundPantheon call so I don't know whether it is preserved.

[SomeTroglodyte]

Uh, haven't got the energy to Guess intentions - Xander's originally or @SeventhM 's. I got to that comment you showed, and the conclusion the original author (guessing all of that must have been xlenstra - not @ing, they're busy afaik) left it because they knew they were relying on a specific workflow order, but from there to why the reorder actually touched that and how to get that back on track... Brain fail. Can you please try - and/or talk to 7M?

[dHannasch]

@SeventhM This branch moves the foundPantheon call back up above where religion is needed non-null, and moves only

                religionState = ReligionState.Pantheon
                for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponFoundingPantheon))
                    UniqueTriggerActivation.triggerCivwideUnique(unique, civInfo)

down to https://github.com/yairm210/Unciv/blob/master/core/src/com/unciv/logic/civilization/managers/ReligionManager.kt#L390, same as the other two clauses in the when.

This deletes those lines from the foundPantheon() function, same as https://github.com/yairm210/Unciv/pull/10877/files deleted those same lines from foundReligion() and useProphetForEnhancingReligion().

Does this preserve the intent of #10877?

[SeventhM]

Also, something tells me foundPantheon should also be in the pantheon picker screen now that I think about it, not just here. Can't believe I overlooked it, especially when it took needed it's triggers moved

[yairm210]

Release patch

[yairm210]

Reopening because we get a lot of these

[yairm210]

Oh whoops this needs to be on an issue