LoneGazebo / Community-Patch-DLL

Community Patch for Civilization V - Brave New World
Other
285 stars 157 forks source link

Crash when civilian tries to withdraw from captured unit #9674

Open Maksym-Nosatskyi opened 1 year ago

Maksym-Nosatskyi commented 1 year ago

1. Mod Version (X.Y.Z). Current Version: 3.1.1

3.1.1

2. DirectX Version

10/11

3. List of Other Mods

Here is a zip with the minimum amount of mods that have to be enabled for the save to load properly (the crash persists) and attached is another one with the rest of the mods I used. I haven't actually tried disabling all of the extra civs, but just trying a few one by one caused the save to not load, so I assume the same will happen with the rest as well. MODS_rest.zip

4. Describe the Issue

Crash on Attila's turn 205.


5. Save Game From 1 Turn Before (ALWAYS ATTACH THIS IF POSSIBLE)

Attached is a save on turn 205 right before clicking next turn and an autosave at the start of turn 204. saves.zip

6. Logs (ALWAYS ATTACH THESE IF POSSIBLE)

Logs.zip

7. CvMiniDump.dmp File (ATTACH IF REPORTING A GAME CRASH)

CvMiniDump.zip

8. Steps to reproduce the Issue (Optional)

Click next turn and wait for the Hun turn to start.

9. Screenshots of the Issue (Optional)

Maksym-Nosatskyi commented 1 year ago

Also in this game I attacked Genoa to steal some land with Hex Conquer and made peace back in Ancient. Since then the amount of turns that they won't give me quests has been ticking up instead of down. I've never actively attacked a CS before without conquering it, so I don't actually know if it's supposed to be this long and it's just a UI bug, or something went wrong with the actual mechanic. Either way, I can befriend them with a single envoy, so it's not a big deal.

Maksym-Nosatskyi commented 1 year ago

Update. So the save in the OP is actually the second time this has happened. The first time was on the Hunnic turn 190-something. I tried to fix it by disabling mods, ended up breaking the save (all the buildings not from base VP were shuffled around) and restarting from autosave_initial. Then on that restart the OP happened, then I reloaded like 15 turns back on a hunch and did some things differently and there was no crash. It came back later on the Inuit turn 267. Since these two are two eras apart, but both during wars, my entirely uneducated guess would be a tactical AI bug? New saves, logs (separated into the ones last updated right before the crash and the rest) and dump attached. By the way, the Genoa bug turned out to be just in the UI after all. saves2.zip Logs_latest.zip Logs_rest.zip CvMiniDump.zip

RecursiveVision commented 1 year ago

09b37832f7ccf5ed25acf790abbb4c22

A captured unit is created, then the following code is run (attacker is NULL, therefore plot is NULL, causing a nullptr error).

No idea how to fix.

ilteroi commented 1 year ago

well, i attacker is a reference so it can't be zero. but the plot might be zero. no idea how this can happen but maybe adding the following lines at the beginning of GetWithdrawChance() would help:

    if (!onMap() || !attacker.onMap())
        return 0;
RecursiveVision commented 1 year ago

I'll try that. Maybe it's because it's called when the unit is being created and presumably hasn't been placed on the map yet.

Maksym-Nosatskyi commented 1 year ago

I've got another crash in the same game, so replying here to not upload the mod list again. It seems to be a different one as this time the culprit is Brazil, who is at peace, and there are no barbarians around any of their units with withdraw chance, as far as I can tell. An event from Community Events fires for them on this turn, but on a previous run it happened a few turns earlier and the crash was later. It may have been fixed in one of the new versions, but still. I don't think I can reopen this issue, so if nobody responds I'll just open a new one. Gandhi_0293 AD-1365.zip Logs.zip CvMiniDump.zip

Maksym-Nosatskyi commented 1 year ago

Actually, sorry, nevermind, I tested this after posting when I didn't have internet and forgot to reply later, this is caused by Hex Conquer and the crash is actually on the Inuit turn, I'll report it in that mod's forum thread.

axatin commented 1 year ago

@RecursiveVision Regarding the first bug:

Attila attacks a tile on which both a military and a civilian unit are. The military unit is captured, so a new unit is initialized and it's position is set . Now, in CvUnit::setXY, before actually setting the new position of the unit, a check for enemy units on the new tile is done. The civilian unit which is still there has a promotion that allows it to withdraw, so fallback options are evaluated for it. The possible tiles to withdraw to are based on the current position of the attacking unit to make sure a withdrawing unit doesn't appear on or next to the tile the attacker came from.

In this case, setXY is called for a unit that's newly created, so it doesn't have a position from where it came. It's quite a corner case. We can follow ilteroi's suggestion and make withdrawal impossible. Or we change CvUnit::DoFallBack and allow units to fall back in all 6 directions if the attacker is not on the map (normally only the three directions opposite of the attacker are possible).

axatin commented 11 months ago

With the fix that has been applied the game does not crash anymore. Again, the situation here is that a military unit is being captured while there's a civilian unit with a withdrawal promotion on the same tile. The current behavior is that the civilian is captured as well. I don't see a way to make the unit retreat normally (including the information about where the attacker came from) without rewriting a lot of code, and that doesn't feel appropriate given that this is really a corner case. I'd suggest closing this as completed.

The other option is to make the unit withdraw and evaluate all 6 possible directions. Since the attacking unit is still on its original tile and since units prefer not to withdraw to tiles with adjacent enemies, in most cases the civilian would probably withdraw to one of the tiles opposite to the attacker, consistent with the usual withdrawal behavior. @RecursiveVision Which solution would you prefer?

azum4roll commented 7 months ago

How does it work now with the capture rework?