Skip to content

Commit

Permalink
wzapi: change removeObject API function to defer object removal
Browse files Browse the repository at this point in the history 
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]>
  • Loading branch information
@ManManson
ManManson committed Apr 15, 2024
1 parent 5f98ae8 commit 06cb72e
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 44 deletions.
54 changes: 35 additions & 19 deletions src/loop.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
#include "clparse.h"
#include "gamehistorylogger.h"
#include "profiling.h"
#include "wzapi.h"

#include "warzoneconfig.h"

Expand Down Expand Up @@ -490,6 +491,14 @@ void countUpdate(bool synch)
}
}

template <typename Fn>
static void executeFnAndProcessScriptQueuedRemovals(Fn fn)
{
ASSERT(wzapi::scriptQueuedObjectRemovals().empty(), "Leftover script-queued object removals detected!");
fn();
wzapi::processScriptQueuedObjectRemovals();
}

static void gameStateUpdate()
{
WZ_PROFILE_SCOPE(gameStateUpdate);
Expand All @@ -515,7 +524,7 @@ static void gameStateUpdate()

if (!paused && !scriptPaused())
{
updateScripts();
executeFnAndProcessScriptQueuedRemovals([]() { updateScripts(); });
}

// Update abandoned structures
Expand Down Expand Up @@ -544,27 +553,34 @@ static void gameStateUpdate()
//update the current power available for a player
updatePlayerPower(i);

mutating_list_iterate(apsDroidLists[i], [](DROID* d)
{
droidUpdate(d);
return IterationResult::CONTINUE_ITERATION;
executeFnAndProcessScriptQueuedRemovals([i]() {
mutating_list_iterate(apsDroidLists[i], [](DROID* d)
{
droidUpdate(d);
return IterationResult::CONTINUE_ITERATION;
});
});
mutating_list_iterate(mission.apsDroidLists[i], [](DROID* d)
{
missionDroidUpdate(d);
return IterationResult::CONTINUE_ITERATION;
executeFnAndProcessScriptQueuedRemovals([i]() {
mutating_list_iterate(mission.apsDroidLists[i], [](DROID* d)
{
missionDroidUpdate(d);
return IterationResult::CONTINUE_ITERATION;
});
});

// FIXME: These for-loops are code duplication
mutating_list_iterate(apsStructLists[i], [](STRUCTURE* s)
{
structureUpdate(s, false);
return IterationResult::CONTINUE_ITERATION;
executeFnAndProcessScriptQueuedRemovals([i]() {
mutating_list_iterate(apsStructLists[i], [](STRUCTURE* s)
{
structureUpdate(s, false);
return IterationResult::CONTINUE_ITERATION;
});
});
mutating_list_iterate(mission.apsStructLists[i], [](STRUCTURE* s)
{
structureUpdate(s, true); // update for mission
return IterationResult::CONTINUE_ITERATION;
executeFnAndProcessScriptQueuedRemovals([i]() {
mutating_list_iterate(mission.apsStructLists[i], [](STRUCTURE* s)
{
structureUpdate(s, true); // update for mission
return IterationResult::CONTINUE_ITERATION;
});
});
}

Expand Down Expand Up @@ -641,7 +657,7 @@ GAMECODE gameLoop()
{
// Receive NET_BLAH messages.
// Receive GAME_BLAH messages, and if it's time, process exactly as many GAME_BLAH messages as required to be able to tick the gameTime.
recvMessage();
executeFnAndProcessScriptQueuedRemovals([]() { recvMessage(); });

bool selectedPlayerIsSpectator = bMultiPlayer && NetPlay.players[selectedPlayer].isSpectator;
bool multiplayerHostDisconnected = bMultiPlayer && !NetPlay.isHostAlive && NetPlay.bComms && !NetPlay.isHost; // do not fast-forward after the host has disconnected
Expand Down
76 changes: 52 additions & 24 deletions src/wzapi.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2930,36 +2930,22 @@ bool wzapi::removeStruct(WZAPI_PARAMS(STRUCTURE *psStruct)) WZAPI_DEPRECATED

//-- ## removeObject(gameObject[, sfx])
//--
//-- Remove the given game object with special effects. Returns a boolean that is true on success.
//-- Queue the given game object for removal with or without special effects. Returns a boolean that is true on success.
//-- A second, optional boolean parameter specifies whether special effects are to be applied. (3.2+ only)
//--
//-- BREAKING CHANGE (4.5+): the effect of this function is not immediate anymore, the object will be
//-- queued for later removal instead of destroying it right away.
//-- User scripts should not rely on this function having immediate side-effects.
//--
bool wzapi::removeObject(WZAPI_PARAMS(BASE_OBJECT *psObj, optional<bool> _sfx))
{
SCRIPT_ASSERT(false, context, psObj, "No valid object provided");
bool sfx = _sfx.value_or(false);
SCRIPT_ASSERT(false, context,
psObj->type == OBJ_STRUCTURE || psObj->type == OBJ_DROID || psObj->type == OBJ_FEATURE,
"Wrong game object type");

bool retval = false;
if (sfx)
{
switch (psObj->type)
{
case OBJ_STRUCTURE: destroyStruct((STRUCTURE *)psObj, gameTime); break;
case OBJ_DROID: retval = destroyDroid((DROID *)psObj, gameTime); break;
case OBJ_FEATURE: retval = destroyFeature((FEATURE *)psObj, gameTime); break;
default: SCRIPT_ASSERT(false, context, false, "Wrong game object type"); break;
}
}
else
{
switch (psObj->type)
{
case OBJ_STRUCTURE: retval = removeStruct((STRUCTURE *)psObj, true); break;
case OBJ_DROID: retval = removeDroidBase((DROID *)psObj); break;
case OBJ_FEATURE: retval = removeFeature((FEATURE *)psObj); break;
default: SCRIPT_ASSERT(false, context, false, "Wrong game object type"); break;
}
}
return retval;
scriptQueuedObjectRemovals().emplace_back(psObj, _sfx.value_or(false));
return true;
}

//-- ## setScrollLimits(x1, y1, x2, y2)
Expand Down Expand Up @@ -4600,3 +4586,45 @@ nlohmann::json wzapi::constructMapTilesArray()
}
return mapTileArray;
}

wzapi::QueuedObjectRemovalsVector& wzapi::scriptQueuedObjectRemovals()
{
static QueuedObjectRemovalsVector instance = []()
{
static constexpr size_t initialCapacity = 16;
QueuedObjectRemovalsVector ret;
ret.reserve(initialCapacity);
return ret;
}();
return instance;
}

void wzapi::processScriptQueuedObjectRemovals()
{
auto& queuedObjRemovals = scriptQueuedObjectRemovals();
for (auto& objWithSfxFlag : queuedObjRemovals)
{
BASE_OBJECT* psObj = objWithSfxFlag.first;
if (objWithSfxFlag.second)
{
switch (psObj->type)
{
case OBJ_STRUCTURE: destroyStruct((STRUCTURE*)psObj, gameTime); break;
case OBJ_DROID: destroyDroid((DROID*)psObj, gameTime); break;
case OBJ_FEATURE: destroyFeature((FEATURE*)psObj, gameTime); break;
default: ASSERT(false, "Wrong game object type"); break;
}
}
else
{
switch (psObj->type)
{
case OBJ_STRUCTURE: removeStruct((STRUCTURE*)psObj, true); break;
case OBJ_DROID: removeDroidBase((DROID*)psObj); break;
case OBJ_FEATURE: removeFeature((FEATURE*)psObj); break;
default: ASSERT(false, "Wrong game object type"); break;
}
}
}
queuedObjRemovals.clear();
}
30 changes: 29 additions & 1 deletion src/wzapi.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1098,6 +1098,14 @@ namespace wzapi
no_return_value makeComponentAvailable(WZAPI_PARAMS(std::string componentName, int player));
bool allianceExistsBetween(WZAPI_PARAMS(int player1, int player2));
bool removeStruct(WZAPI_PARAMS(STRUCTURE *psStruct)) WZAPI_DEPRECATED;
/// <summary>
/// Queues the given game object for removal with or without special effects.
///
/// The current behavior is in effect since version 4.5+.
/// </summary>
/// <param name="psObj">Game object to be queued for removal.</param>
/// <param name="_sfx">Optional boolean parameter that specifies whether special effects are to be applied.</param>
/// <returns>`true` if `psObj` has been successfully queued for destruction.</returns>
bool removeObject(WZAPI_PARAMS(BASE_OBJECT *psObj, optional<bool> _sfx));
no_return_value setScrollLimits(WZAPI_PARAMS(int x1, int y1, int x2, int y2));
scr_area getScrollLimits(WZAPI_NO_PARAMS);
Expand Down Expand Up @@ -1126,6 +1134,26 @@ namespace wzapi
nlohmann::json constructStaticPlayerData();
std::vector<PerPlayerUpgrades> getUpgradesObject();
nlohmann::json constructMapTilesArray();
}

// `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.
using QueuedObjectRemovalsVector = std::vector<std::pair<BASE_OBJECT*, bool>>;

QueuedObjectRemovalsVector& scriptQueuedObjectRemovals();
/// <summary>
/// Walks `scriptQueuedObjectRemovals()` list, destroys every object in that list
/// and clears the container.
/// </summary>
void processScriptQueuedObjectRemovals();
} // namespace wzapi

#endif

0 comments on commit 06cb72e

Please sign in to comment.