We-the-People-civ4col-mod / Mod

This is the repository where the mod resides.
89 stars 37 forks source link

Removing and adding a citizen from a colony leaves the unit on the plot outside the colony #923

Open Nightinggale opened 11 months ago

Nightinggale commented 11 months ago

I might finally have figured out the cause of NULL units in selection groups.

When loading the attached savegame and ending the turn, unit 1 of player 37 (expert farmer in Fort Christina) will call CvPlot::addUnit twice, but never calls CvPlot::removeUnit. The game then crashes in CvUnit::setInfoBarDirty as the unit instance is the farmer, but since it's not on the map, it's NULL.

Why the farmer is added to the plot twice and why it's not removed when rejoining the city is still unknown.

jhxp_AD-1517-January7.zip

devolution79 commented 11 months ago

I had a look at the save and did an experiment by adding the following to CvPlayerAI::AI_unitUpdate():

                        if (bRemove)
                        {
                            if (getUnit(pRemoveUnit->getID()) == NULL)
                            {
                                // make sure the unit can have the default profession
                                // this fails even if it can if leaving is blocked by disorder
                                //    Nightinggale
                                ProfessionTypes eDefaultProfession = GC.getCivilizationInfo(getCivilizationType()).getDefaultProfession();
                                if (pRemoveUnit->canHaveProfession(eDefaultProfession, false, NULL, true))
                                {
                                    pLoopCity->removePopulationUnit(pRemoveUnit, false, eDefaultProfession);
                                }
                            }
                        }

This prevents the subsequent crash by preventing the removal of the farmer if it already exists. Noe that we do end up triggering a bunch of other asserts so it is not a fix.

I also had a look at the AI player (the Swedish) and I didn't see any signs of strange stuff. Also, no relevant event had triggered recently.

devolution79 commented 11 months ago

I think the bad state has happened prior to this and it may be that the save itself is bit borked, perhaps there is something unusual that triggers this behaviour ?

devolution79 commented 11 months ago

Here's save with the Swedish as the human player (note that the mod folder name is 'wtp')

Johan Printz AD-1517-January.zip

Nightinggale commented 11 months ago

Sounds to me like we have multiple bugs here.

The most critical one is the first one. The others seems to be follow ups, but it's still worth arguing that we should consider fixing all as that will overall make the game more stable now and in the future.

devolution79 commented 11 months ago

I've added a sanity check to determine if a unit is both inside a city and and on the map which is definitely abnormal and it does trigger for the expert farmer! Now that we can detect this state we can fix it but the real question is how to prevent it in the first place.

Nightinggale commented 10 months ago

I got new savegames with units behaving weirdly in colonies. I fixed the reported infinite loop, but then it fails assert with PROFESSION_CARPENTER on a plot. May 1522 savegame, through the zip contains some savegames from previous turns too. jhxp_game_freezes_at_May_1522.zip

Razonatair commented 8 months ago

Just a hypothetical, but maybe the Starvation code is borking the unit sometimes when it kicks it out of a colony?

Nightinggale commented 8 months ago

Just a hypothetical, but maybe the Starvation code is borking the unit sometimes when it kicks it out of a colony?

That is actually a really good question. I don't think anybody can answer that, at least not without further investigation. Given that something is messing up units, it could be possible.

Nightinggale commented 2 months ago

I wonder if this can be linked to the fix I just pushed 8b245f59cdb085a9eb5a9cf5604cef9d0485cb62