make hitpoint sexps safer to use in debriefing #6514
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Goober5000
added
fix
Goober5000
force-pushed
the
hitpoint_fixes
branch
from
394a6eb to
0159799
Compare
There is a rather large design flaw in FreeSpace debriefings... debriefing SEXPs can refer to in-mission ship data, while all ships (and all objects) are deleted as soon as the mission completes. Specifically, `obj_delete()` is called which sets all objects to `OBJ_NONE` and marks them as freed, although the data still exists in the various arrays. This is the use-after-free problem, but applied to the in-game object system, rather than memory accessed by pointers. It's probably best to defer fixes to an as-needed basis. As for hitpoint SEXPs, this change updates them to use the object and ship references from the ship registry, since the ship registry is not cleared by the time the debriefing is loaded. (Using `objp->instance` and `shipp->objnum` at this point would be problematic.) Since subsystems are also cleared at this point, this also updates the warning messages in those functions. Fixes an Assert seen in debug mode in Inferno.
This has been broken ever since commit 6c17ef0 in 2005. Since `get_hull_pct()` did not return negative values, `percent_killed` was always 0. Allow negative values in this case.
Goober5000
force-pushed
the
hitpoint_fixes
branch
from
0159799 to
cecfd9f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There is a rather large design flaw in FreeSpace debriefings... debriefing SEXPs can refer to in-mission ship data, while all ships (and all objects) are deleted as soon as the mission completes. Specifically,
obj_delete()is called which sets all objects toOBJ_NONEand marks them as freed, although the data still exists in the various arrays. This is the use-after-free problem, but applied to the in-game object system, rather than memory accessed by pointers.It's probably best to defer fixes to an as-needed basis. As for hitpoint SEXPs, this change updates them to use the object and ship references from the ship registry, since the ship registry is not cleared by the time the debriefing is loaded. (Using
objp->instanceandshipp->objnumat this point would be problematic.) Since subsystems are also cleared at this point, this also updates the warning messages in those functions.Fixes an Assert seen in debug mode in Inferno.
Relatedly, fix the calculation of
percent_killed. This has been broken ever since commit 6c17ef0 in 2005. Sinceget_hull_pct()did not return negative values,percent_killedwas always 0. Allow negative values in this case.