TF2-DMB / CBaseNPC

Provides a friendly interface for plugins to use in order to create custom NPCs on the game Team Fortress 2
36 stars 5 forks source link

Allow use of RemoveEntity in Actions #16

Closed KitRifty closed 2 years ago

KitRifty commented 2 years ago

Currently, using RemoveEntity() in an action callback results in undefined behavior, since CBaseNPC behavior (and by extension the actions) is released in UpdateOnRemove() which is directly invoked from the native. This also violates the atomic principle that the Actions were originally built upon (OnEnd() ends up getting called in the invoking action callback, which shouldn't happen). The dirty workaround to this is delaying the removal via AcceptEntityInput() or RequestFrame().

This PR fixes that case, allowing RemoveEntity() to be used in action callbacks without crashes. Object destruction is now handled via destructor hooks and not in UpdateOnRemove(), making objects behave more normally by C++ standards.

As a result, uninstalling factories now removes entities IMMEDIATELY instead of waiting for next frame to prevent invalid memory access, since removing entities immediately calls UpdateOnRemove() and destroys the entity in the same frame. Of course, this may be unstable anyways since destroying entities during a frame isn't recommended, but since this case should only happen in a dev environment it's a drawback I'm willing to live with.

Some factory install/uninstall code had to be changed as a result of this fix. To ensure that RemoveEntityImmediate() can take place, entities will no longer be automatically removed in the OnCoreMapEnd() functions. Factory entities will now be removed at the same time as any other entity in the game upon map change, which unfortunately happens after SM has performed its map cleanup which includes Timer handles. Warnings were added to let developers know that Timer handles with the TIMER_FLAG_NO_MAPCHANGE flag may have been cleaned up already by the time a CEntityFactory's onRemove callback or NextBotActionFactory's OnEnd() callback is fired.

Tested on Windows with the following code:

#pragma newdecls required
#pragma semicolon 1
#include <sourcemod>
#include <sdktools>
#include <sdkhooks>
#include <dhooks>
#include <cbasenpc>

NextBotActionFactory testActionFactory;
CEntityFactory testFactory;

public void OnPluginStart()
{
    testActionFactory = new NextBotActionFactory("TestAction");
    testActionFactory.SetCallback(NextBotActionCallbackType_OnEnd, OnTestActionEnd);
    testActionFactory.SetEventCallback(EventResponderType_OnKilled, OnTestActionKilled);

    testFactory = new CEntityFactory("test_entity_npc", OnCreateTestEntity, OnRemoveTestEntity);
    testFactory.DeriveFromNPC();
    testFactory.SetInitialActionFactory(testActionFactory);
    testFactory.BeginDataMapDesc();
    testFactory.DefineIntField("m_iMyValue");
    testFactory.DefineStringField("m_vecMyValue");
    testFactory.EndDataMapDesc();
    testFactory.Install();
}

static void OnTestActionEnd(NextBotAction action, int actor)
{
    PrintToServer("Test entity %d: OnTestActionEnd (entity is valid: %d)", actor, IsValidEntity(actor));
}

static int OnTestActionKilled(NextBotAction action, int actor, int attacker, int inflictor, float damage, int damagetype)
{
    RemoveEntity(actor);

    PrintToServer("Test entity %d: OnTestActionKilled (entity is valid: %d)", actor, IsValidEntity(actor));

    return action.TryDone(RESULT_CRITICAL);
}

static void OnCreateTestEntity(int ent)
{
    PrintToServer("Test entity %d: OnCreate", ent);

    SetEntityModel(ent, "models/player/scout.mdl");
}

static void OnRemoveTestEntity(int ent)
{
    PrintToServer("Test entity %d: OnRemove (entity is valid: %d)", ent, IsValidEntity(ent));
}

Results:

// Killing
Test entity 98: OnCreate
NextBot tickrate changed from 0 (0.000ms) to 7 (0.105ms)
Test entity 98: OnRemove (entity is valid: 1)
Test entity 98: OnTestActionKilled (entity is valid: 1)
Test entity 98: OnTestActionEnd (entity is valid: 1)

// Reloading plugin manually
Test entity 97: OnCreate
sm plugins reload test_entity
Test entity 97: OnRemove (entity is valid: 1)
Test entity 97: OnTestActionEnd (entity is valid: 1)
L 12/25/2021 - 10:39:39: [CBASENPC] Uninstalled test_entity_npc entity factory
L 12/25/2021 - 10:39:39: [CBASENPC] Installed test_entity_npc entity factory
[SM] Plugin test_entity reloaded successfully.

Test entity 97: OnCreate
// Plugin file recompiled, will be reloaded automatically on map change
sm_map ctf_turbine
[SM] Changing map to ctf_turbine...
L 12/25/2021 - 10:40:10: [basecommands.smx] "Console<0><Console><Console>" changed map to "ctf_turbine"
L 12/25/2021 - 10:40:13: [SM] Changed map to "ctf_turbine"
---- Host_Changelevel ----
Test entity 97: OnRemove (entity is valid: 1)
Test entity 97: OnTestActionEnd (entity is valid: 1)
L 12/25/2021 - 10:40:13: [CBASENPC] Uninstalled test_entity_npc entity factory
L 12/25/2021 - 10:40:15: -------- Mapchange to ctf_turbine --------
L 12/25/2021 - 10:40:15: [CBASENPC] Installed test_entity_npc entity factory
Executing dedicated server config file server.cfg