yairm210 / Unciv

Open-source Android/Desktop remake of Civ V
Mozilla Public License 2.0
8.37k stars 1.56k forks source link

Crash: Unit Missionary at Tile @(10.0,-4.0), Grassland, Hill, Deer, Railroad, Giant Death Robot - Mitla can't be put in tile (10.0,-4.0)! #9636

Closed marek22k closed 1 year ago

marek22k commented 1 year ago

Platform: Android Version: 4.7.2 (Build 881) Rulesets: [Policy mod, Great Farmer, The Demon World, Extra Buildings, Extra Units, Barbarian xp farm, The North Pole, Warfare Expanded Lite, Better Units and Natural Wonders, Civ V - Vanilla, Resource Recyclers, AbsoluteUnits, Modern Joke Religions, Better Pantheons Lite, Extra Wonders, Barbarians but Better, CS Rework, Better Pantheons, Civ V - Gods & Kings, CS Unique Components , 5Hex Tileset, Logicians, The Barbarians, Extra Promotions, Better Workers, Better ruins, Ruins Extended Classic Edition] Last Screen: com.unciv.ui.screens.worldscreen.WorldScreen


Device Model: FP4 API Level: 33


Message:

java.lang.Exception: Unit Missionary at Tile @(10.0,-4.0), Grassland, Hill, Deer, Railroad, Giant Death Robot - Mitla can't be put in tile (10.0,-4.0)!
    at com.unciv.logic.map.mapunit.MapUnit.putInTile(MapUnit.kt:639)
    at com.unciv.logic.map.mapunit.UnitMovement.canUnitSwapToReachableTile(UnitMovement.kt:464)
    at com.unciv.logic.map.mapunit.UnitMovement.access$canUnitSwapToReachableTile(UnitMovement.kt:15)
    at com.unciv.logic.map.mapunit.UnitMovement$getUnitSwappableTiles$1.invoke(UnitMovement.kt:436)
    at com.unciv.logic.map.mapunit.UnitMovement$getUnitSwappableTiles$1.invoke(UnitMovement.kt:436)
    at kotlin.sequences.FilteringSequence$iterator$1.calcNext(Sequences.kt:171)
    at kotlin.sequences.FilteringSequence$iterator$1.hasNext(Sequences.kt:194)
    at kotlin.sequences.SequencesKt___SequencesKt.none(_Sequences.kt:2130)
    at com.unciv.ui.screens.worldscreen.unit.actions.UnitActions.addSwapAction(UnitActions.kt:117)
    at com.unciv.ui.screens.worldscreen.unit.actions.UnitActions.getAdditionalActions(UnitActions.kt:94)
    at com.unciv.ui.screens.worldscreen.unit.actions.UnitActions.getUnitActions(UnitActions.kt:36)
    at com.unciv.ui.screens.worldscreen.unit.actions.UnitActionsTable.update(UnitActionsTable.kt:28)
    at com.unciv.ui.screens.worldscreen.WorldScreen.update(WorldScreen.kt:368)
    at com.unciv.ui.screens.worldscreen.WorldScreen.render(WorldScreen.kt:721)
    at com.badlogic.gdx.Game.render(Game.java:48)
    at com.unciv.UncivGame.access$render$s2211858(UncivGame.kt:125)
    at com.unciv.UncivGame$wrappedCrashHandlingRender$1.invoke(UncivGame.kt:143)
    at com.unciv.UncivGame$wrappedCrashHandlingRender$1.invoke(UncivGame.kt:143)
    at com.unciv.ui.crashhandling.CrashHandlingExtensionsKt$wrapCrashHandling$1.invoke(CrashHandlingExtensions.kt:17)
    at com.unciv.ui.crashhandling.CrashHandlingExtensionsKt$wrapCrashHandlingUnit$1.invoke(CrashHandlingExtensions.kt:33)
    at com.unciv.ui.crashhandling.CrashHandlingExtensionsKt$wrapCrashHandlingUnit$1.invoke(CrashHandlingExtensions.kt:33)
    at com.unciv.UncivGame.render(UncivGame.kt:456)
    at com.badlogic.gdx.backends.android.AndroidGraphics.onDrawFrame(AndroidGraphics.java:474)
    at android.opengl.GLSurfaceView$GLThread.guardedRun(GLSurfaceView.java:1573)
    at android.opengl.GLSurfaceView$GLThread.run(GLSurfaceView.java:1272)

Save Mods:

[CS Rework, Great Farmer, Resource Recyclers, Civ V - Gods & Kings, Modern Joke Religions, CS Unique Components , Extra Buildings, 5Hex Tileset, Extra Wonders, Extra Promotions, Extra Units, Logicians]

Save Data: https://gist.github.com/marek22k/e2a9aa2a41ef9a9bd86e070ac932f692

SomeTroglodyte commented 1 year ago

What a can of Worms... 1 - How can that blank get in there: "gameParameters":{"difficulty":"Immortal","maxNumberOfCityStates":9,"maxTurns":900,"minNumberOfCityStates":4,"mods":["5Hex Tileset","CS Rework","CS Unique Components ","Extra Buildings","Extra Promotions","Extra Units","Great Farmer","Logicians","Modern Joke Religions","Resource Recyclers","Extra Wonders"],"noBarbarians":true,"nuclearWeaponsEnabled":false,"numberOfCityStates":3,"players":[{"chosenCiv":"Logicians","playerType":"Human"},{},{}],"speed":"Epic","victoryTypes":["Domination"]} 2 - This will obviously trip the missing mod downloader, possibly hidden on Linux but hard on Windoze. 3 - The failure during zip extraction and copying shows crashhandling unable to handle the crash - fails to switch to a GL context to be able to show the CrashScreen

4 - OP didn't mention which units exactly they were trying to swap - a Missionary and...?

marek22k commented 1 year ago

4 - OP didn't mention which units exactly they were trying to swap - a Missionary and...?

Was that a question for me? I did not use the "Swap Units" function during the game. After I restarted the game, it worked again - as far as I can tell.

SomeTroglodyte commented 1 year ago

Was that a question for me

Indirectly, a little. See that getUnitSwappableTiles in the stack trace? My 50:50 guess would have been that's only called after using the swap button, but maybe not - if you say you didn't...

But thank you for finding those vulnerabilities in the mod management.

Interesting that the code seems entirely immune to that repo name on 'droid, nobody trying to trim, so the folder name with a trailing blank must cleanly propagate everywhere including file system... As I said, on Windows that's not possible. Dozy can do trailing blanks in filesystem names if the FS supports them, but you must go through the Wider-char version of the CreateFile API and prepend a \\?\ to all paths - and use fully qualified paths only. The Gdx-java.io doesn't do that.

SomeTroglodyte commented 1 year ago

For the curious: The error that crashhanlding fails to display is Could not load the missing mods! Unhandled problem, GdxRuntimeException Error copying source file: mods\temp--20ccb061\CS-Unique-Components--main\game.png (Local) To destination: mods\CS Unique Components \game.png (Local)

SomeTroglodyte commented 1 year ago

Coders: handleLoadGameException runs on the non-daemon pool thread, so its !isUserFixablepart will clearly fail - you can't instantiate (actually instatiate yes but adding font-using widgets no) a Popup without GL context, let alone show it. Clearly the runOnGLThread a little later is misplaced.

.... But why does that topple the entire process, why is the crashhandling wrapper (which is in place) not recover? That runs downstream of the try-catch block in CoroutineScope.launchCrashHandling but the catch doesn't catch says the debugger. 🤔 ???????

yairm210 commented 1 year ago

What this looks like is that the GDR unit was in a tile that - by code - it shouldn't have been able to enter in the first place

So we have 2 open questions: A. How did this happen? B. Do we want to 'patch' such problem on load game by 'picking each unit up and putting it back down'? My instinct says "no, because then we won't discover other errors like this one"

SomeTroglodyte commented 1 year ago

that the GDR unit was in a tile that - by code - it shouldn't have been able to enter in the first place

Not sure what you mean. (10,-4) is this: image

.. That's the GDR mentioned, and the tile is just fine?

yairm210 commented 1 year ago

In the error message, there was a missionary attempting the swap who was in that tile. Clicking 'swap' with any unit does not crash Creating a missionary, moving it near, clicking 'swap' does not crash

I am entirely unable to recreate this crash, and the code looks good, without a save file this is unworkable :/ @marek22k can you provide a save file that crashes?

marek22k commented 1 year ago

@marek22k can you provide a save file that crashes?

No, unfortunately not. As I said, after I restarted the game, it worked again.

SomeTroglodyte commented 1 year ago

there was a missionary

Since that save seems after the fact like most, I assumed the AI next-turn automation had one - but then maybe AI isn't able to use swap at all??? Oh right, WorldScreen.update - so it was a human action, and we do not have the full information. Oh well, we'll catch it sooner or later.

yairm210 commented 1 year ago

Unfortunately I'll have to close this bug as unworkable - cannot reproduce and cannot find the error without a reproduction

Last ditch attempt - @marek22k is there a chance you clicked the 'swap' button while 'move automated units' was active?

marek22k commented 1 year ago

Unfortunately I'll have to close this bug as unworkable - cannot reproduce and cannot find the error without a reproduction

That's fine. We can reopen the issue if this happens to me again and I have saved multiple rounds.

Last ditch attempt - @marek22k is there a chance you clicked the 'swap' button while 'move automated units' was active?

No, I generally don't use this feature (currently) in my games. The only thing I could imagine is that I accidentally pressed it.