Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash in _map_age_dead_critters #258

Merged
merged 7 commits into from
May 23, 2023

Conversation

roginvs
Copy link
Contributor

@roginvs roginvs commented Apr 13, 2023

This PR fixes crashing in _map_age_dead_critters function because the same object destroyed twice.

I have a save file on Fallout Of Nevada where it happens. Probably reproduceable via keeping corpse on the lowest-index tile and waiting for a while.

It was caused by objectFindFirst/objectFindNext functions which returned the same object twice due to off-by-one error in gObjectFindTile variable.

After calling objectFindFirst() function gObjectFindTile variable holds index of tile where object was found. After few calls to objectFindNext when this tile is exhausted we do postfix increment on gObjectFindTile so we start to fetch objects from the same tile. When tile is exhausted second time then we jump to the next tile because gObjectFindTile already incremented in the first exhausting.

So, for this function we consider gObjectFindTile as a "tile where to lookup when current list ends". It should always be next tile.

Of course it is possible to keep gObjectFindTile meaning as "tile where we currently iterating" but this will lead to more changes in objectFindNext function, not just changing postfix increment into prefix but also checking gObjectFindTile is still in the range.

This PR affects objectFindFirst/objectFindNext and objectFindFirstAtElevation/objectFindNextAtElevation functions. Other function objectFindFirstAtLocation sets gObjectFindTile variable but objectFindNextAtLocation is not using it.

@alexbatalov
Copy link
Owner

While your solution works perfectly fine, there is one hypothetical edge case which it cannot handle (empty map with stack of objects at tile 39999).

@alexbatalov alexbatalov merged commit 42c5410 into alexbatalov:main May 23, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants