wzapi: change removeObject API function to defer object removal
#3736
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
past-due
reviewed
Apr 15, 2024
ManManson
force-pushed
the
fix_alpha11_crash_remove_droid_from_script
branch
from
06cb72e to
41ec6c6
Compare
|
Found a few other places we should probably call
|
ManManson
force-pushed
the
fix_alpha11_crash_remove_droid_from_script
branch
2 times, most recently
from
2283c39 to
256a3d4
Compare
past-due
reviewed
Apr 18, 2024
ManManson
force-pushed
the
fix_alpha11_crash_remove_droid_from_script
branch
from
c0b5107 to
3c5000d
Compare
Signed-off-by: Pavel Solodovnikov <[email protected]>
Introduce `scriptQueuedObjectRemovals()` vector to hold
deferred object removal requests from `wzapi::removeObject` API.
`wzapi::removeObject` API call adds the game objects to this list.
They will eventually be released during calls to `processScriptQueuedObjectRemovals()`
during the following stages of the main game loop:
1. `recvMessage()` - there are a few functions which distribute
resources of the defeated players among others and may trigger script events.
2. `updateScripts()` - processes queued timer functions.
3. `droidUpdate()`, `missionDroidUpdate()`, `structureUpdate()` - main
routines for updating the state of in-game objects. These would potentially
trigger the majority of script events.
Each pair represents `<GameObject, NeedToApplyEffectsOnDestruction>` tuple.
The sole reason for this breaking API change is to guard against
cases like this one:
1. `gameStateUpdate()` processes droid state updates inside
a `mutating_list_iterate`.
2. A droid update triggers some user script, which calls
`wzapi::removeObject` several times in a row.
3. This may potentially destroy `itNext` iterator saved inside the
`mutating_list_iterate` from of the upper stack frames, which
would lead to invalid memory access.
So, instead of removing multiple objects inside of
`mutating_list_iterate`, which is unsafe, we just queue removal
requests and execute actual destruction routines once we are out of
the `mutating_list_iterate` function.
Signed-off-by: Pavel Solodovnikov <[email protected]>
These places include not only those which involve triggers upon `droidUpdate()` inside the main execution sequence of the game loop, but also those events that execute prior to and after the main things in the game loop, e.g.: * Level start. * Load/Save game. * Start playing videos. * Render loop. Some of these places may be just extra work and probably won't have even a slight possibility of calling `wzapi::removeObject` from inside them, but it's always better to be safe than sorry. Also, introduce a check for already-dead entries in `processScriptQueuedObjectRemovals()` and skip them. This is a way to deal with duplicate entries to ensure we don't destroy a single object more than once. These things can occur when, e.g., several units happen to move near enough to an artifact lying on the ground, so that they would pick up the artifact simultaneously within the same game loop iteration (in the `checkLocalFeatures()` function). In such case, each of these droids will trigger the "artifact picked up" event, so JS handler (`cam_eventPickup`) will be called multiple times for the same object, each trying to place it into `scriptQueuedObjectRemovals()` list. Signed-off-by: Pavel Solodovnikov <[email protected]>
…`__camPickupArtifact` Signed-off-by: Pavel Solodovnikov <[email protected]>
ManManson
force-pushed
the
fix_alpha11_crash_remove_droid_from_script
branch
from
3c5000d to
e4927fb
Compare
past-due
approved these changes
Apr 22, 2024
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.
Introduce
scriptQueuedObjectRemovals()vector to hold deferred object removal requests fromwzapi::removeObjectAPI.wzapi::removeObjectAPI call adds the game objects to this list. They will eventually be released during calls toprocessScriptQueuedObjectRemovals()during the following stages of the main game loop:recvMessage()- there are a few functions which distribute resources of the defeated players among others and may trigger script events.updateScripts()- processes queued timer functions.droidUpdate(),missionDroidUpdate(),structureUpdate()- main routines for updating the state of in-game objects. These would potentially trigger the majority of script events.Each pair represents
<GameObject, NeedToApplyEffectsOnDestruction>tuple.The sole reason for this breaking API change is to guard against cases like this one:
gameStateUpdate()processes droid state updates inside amutating_list_iterate.wzapi::removeObjectseveral times in a row.itNextiterator saved inside themutating_list_iteratefrom of the upper stack frames, which would lead to invalid memory access.So, instead of removing multiple objects inside of
mutating_list_iterate, which is unsafe, we just queue removal requests and execute actual destruction routines once we are out of themutating_list_iteratefunction.Signed-off-by: Pavel Solodovnikov [email protected]