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

Callbacks call twice #53

Closed MAGNAT2645 closed 4 months ago

MAGNAT2645 commented 4 months ago

For some reason, CEntityFactory callbacks are called twice, post constructor and destructor. I create new factories in OnAllPluginsLoaded because there is a base factory in one plugin (created in OnPluginStart) that I'm deriving from to create extended factories in other plugins. It seems that due to this, my SDK hooks (created in post constructor callbacks) are called twice too. I will post some examples later.

MAGNAT2645 commented 4 months ago

These are from main plugin that has base factory.

L 19.05.2024 - 22:35:58: OnItemEntityCreated(52)
L 19.05.2024 - 22:35:58: OnItemEntityCreated(52)
L 19.05.2024 - 22:42:12: OnItemEntityCreated(89)
L 19.05.2024 - 22:42:12: OnItemEntityCreated(89)
L 19.05.2024 - 22:42:58: OnItemEntityCreated(90)
L 19.05.2024 - 22:42:58: OnItemEntityCreated(90)
L 19.05.2024 - 22:43:02: OnItemEntityRemove: entity = 90, m_Attributes = 0xE4300AC0
L 19.05.2024 - 22:43:02: OnItemEntityRemove: entity = 90, m_Attributes = 0x00000000
L 19.05.2024 - 22:43:32: OnItemEntityRemove: entity = 89, m_Attributes = 0x00000000
L 19.05.2024 - 22:43:32: OnItemEntityRemove: entity = 89, m_Attributes = 0x00000000

And these are in other plugin that has extended factory. OnTestItemEntStartedTouching is the SDKHook_StartTouchPost that's hooked in OnTestItemEntCreate.

L 19.05.2024 - 22:42:12: OnTestItemEntCreate: entityIdx = 89
L 19.05.2024 - 22:42:12: OnTestItemEntCreate: entityIdx = 89
L 19.05.2024 - 22:43:02: OnTestItemEntStartedTouching: entityIdx = 89, otherEntityIdx = 90
L 19.05.2024 - 22:43:02: OnTestItemEntStartedTouching: entityIdx = 89, otherEntityIdx = 90
KitRifty commented 4 months ago

Hi, thanks for the report.

Can you also attach the plugin source used to test this? Or at least a minimum reproducible example?

MAGNAT2645 commented 4 months ago

Main plugin:

#define ITEM_ENT_CLASSNAME "collectable_item"

public void OnPluginStart()
{
    g_hItemBaseEntFactory = new CEntityFactory( ITEM_ENT_CLASSNAME, OnItemEntityCreated, OnItemEntityRemove );
    if ( !g_hItemBaseEntFactory )
    {
        SetFailState( "Couldn't create CEntityFactory " ... ITEM_ENT_CLASSNAME ... "!" );
    }

    g_hItemBaseEntFactory.DeriveFromClass( "prop_physics_multiplayer" );
    g_hItemBaseEntFactory.BeginDataMapDesc( "DT_CollectableItem" )
        .DefineIntField( "m_nItem" )
        .DefineIntField( "m_iQuantity" )
        .DefineIntField( "m_nAcquireMethod" )
        .DefineIntField( "m_nExpiryDate" )
        .DefineIntField( "m_Attributes" ) // Handle to JSON_Object, null by default
        .DefineEntityField( "m_hDroppedBy" )
    .EndDataMapDesc();
    g_hItemBaseEntFactory.Install();
}

void OnItemEntityCreated(CBaseEntity entity)
{
#if defined DEBUG_ENTITIES_CREATING
    PrintToServer( "OnItemEntityCreated(%i)", entity );
#endif

    //entity.SetModelName( "models/props_halloween/halloween_gift.mdl" );
    entity.SetModelName( "models/props_junk/cardboard_box004a.mdl" );
    entity.SetProp( Prop_Data, "m_nItem", INVALID_ITEM );
    entity.SetProp( Prop_Data, "m_iQuantity", 1 );
    entity.SetProp( Prop_Data, "m_nAcquireMethod", AcquireMethod_Unknown );
    entity.SetProp( Prop_Data, "m_nExpiryDate", ZERO_DATETIME );
    entity.SetProp( Prop_Data, "m_Attributes", 0 ); // Compiler does not let use null here, so leave 0 then
    entity.SetPropEnt( Prop_Data, "m_hDroppedBy", -1 );
    entity.KeyValue( "physicsmode", "1" ); // Solid, Server-side (Solid, pushes the player away.)
    entity.KeyValue( "spawnflags", "8448" ); // 8448 = 256 + 8192
                                             // 256: Generate output on +use (so SDKHook_Use will trigger)
                                             // 8192: Force server-side
    entity.KeyValue( "solid", "6" ); // Collisions: 6 = VPhysics

    int entityIdx = entity.index;
    //SDKHook( entityIdx, SDKHook_SpawnPost, OnItemEntitySpawned );
    SDKHook( entityIdx, SDKHook_Use, OnItemEntityUsed );

    CEntityFactory factory = CEntityFactory.GetFactoryOfEntity( entityIdx );
    if ( !factory || factory == g_hItemBaseEntFactory )
    {
        return;
    }

    char className[64];
    factory.GetClassname( STR_OUT( className ) );

    // this is for extended factories so their constructors will be called before spawning entity
    ItemBasedEntityFactory_t factoryData;
    if ( g_hItemBasedEntFactories.GetArray( className, factoryData, sizeof factoryData )
         && factoryData.PostConstructor != INVALID_FUNCTION
       )
    {
        Call_StartFunction( factoryData.Owner, factoryData.PostConstructor );
        Call_PushCell( entity );
        Call_Finish();
    }
}

void OnItemEntityRemove(CBaseEntity entity)
{
    CEntityFactory factory = CEntityFactory.GetFactoryOfEntity( entity.index );
    if ( factory && factory != g_hItemBaseEntFactory )
    {
        char ClassName[64];
        factory.GetClassname( STR_OUT( ClassName ) );

        // this is for extended factories so their destructors will be called before removing entity
        ItemBasedEntityFactory_t FactoryData;
        if ( g_hItemBasedEntFactories.GetArray( ClassName, FactoryData, sizeof FactoryData )
             && FactoryData.Destructor != INVALID_FUNCTION
           )
        {
            Call_StartFunction( FactoryData.Owner, FactoryData.Destructor );
            Call_PushCell( entity );
            Call_Finish();
        }
    }

    CItemEntity ItemEntity = CItemEntity( entity ); // CItemEntity is basically a wrapper around CBaseEntity
    JSON_Object attributes = ItemEntity.Attributes;
#if defined DEBUG_ENTITIES_REMOVING
    PrintToServer(
        "OnItemEntityRemove: entity = %i, m_Attributes = 0x%08X",
        entity,
        attributes
    );
#endif
    json_cleanup( attributes );
    ItemEntity.Attributes = null; // To avoid "Invalid Handle (error 3)" in case of OnItemEntityRemove calling twice
}

Separate plugin:

public void OnAllPluginsLoaded()
{
    // CItemEntityFactory is a wrapper for CEntityFactory to allow setting different constructors and destructors
    // for extended factories based (derived from) on g_hItemBaseEntFactory
    // and for some offtopic features
    CItemEntityFactory factory = new CItemEntityFactory( "collectable_test1", .onCreate = OnTestItemEntCreate );
    if ( factory )
    {
        factory.BeginDataMapDesc( "DT_CollectableTest" )
            .DefineIntField( "m_iTestCount" )
        .EndDataMapDesc();

        factory.Install();
    }
}

void OnTestItemEntCreate(int entityIdx)
{
#if DEBUG
    PrintToServer( "OnTestItemEntCreate: entityIdx = %i", entityIdx );
#endif
    SDKHook( entityIdx, SDKHook_StartTouchPost, OnTestItemEntStartedTouching );
}

static void OnTestItemEntStartedTouching(int entityIdx, int otherEntityIdx)
{
    if ( CEntityFactory.GetFactoryOfEntity( otherEntityIdx ) != g_ConsumableItemEntFactory ) //ignore this
    {
        return;
    }

    /* ignore this
    CItemEntity otherEntity = CItemEntity( otherEntityIdx );
    StoreItem item = otherEntity.Item;
    if ( item == INVALID_ITEM || g_RequiredItem == INVALID_ITEM || item != g_RequiredItem )
    {
        return;
    }
    */

#if DEBUG
    PrintToServer( "OnTestItemEntStartedTouching: entityIdx = %i, otherEntityIdx = %i", entityIdx, otherEntityIdx );
#endif
    ...
}

Let me know if I should provide more code but I dont really want to post all of it here and everything above is actually how it is done (except native functions code).

MAGNAT2645 commented 4 months ago

This is the continuation of OnTestItemEntStartedTouching with some lines removed. And the point here is that instead of counting to 10 before resetting scale it resets scale after 5 times. So that proves that callbacks are called twice.

    CItemEntity entity = CItemEntity( entityIdx );
    float scale = entity.GetPropFloat( Prop_Send, "m_flModelScale" );
    int count = entity.GetProp( Prop_Data, "m_iTestCount" );
    if ( ++count >= 10 )
    {
        count = 0;
        scale = 1.0;
    }
    else
    {
        scale += 0.05;
    }
    entity.SetProp( Prop_Data, "m_iTestCount", count );
    entity.SetPropFloat( Prop_Send, "m_flModelScale", scale );
KitRifty commented 4 months ago

Hi! Thanks for providing some example code. After testing, I confirmed the callbacks are working as designed. I've cut out chunks of your code to only include the parts that are relevant to the issue.

#include <sourcemod>
#include <sdkhooks>
#include <tf2_stocks>
#include <cbasenpc>

CEntityFactory g_hItemBaseEntFactory;
CEntityFactory factory;

public void OnPluginStart()
{
    g_hItemBaseEntFactory = new CEntityFactory("collectable_item", OnItemEntityCreated, OnItemEntityRemove);
    g_hItemBaseEntFactory.DeriveFromClass("prop_physics_multiplayer");
    g_hItemBaseEntFactory.Install();

    factory = new CEntityFactory("collectable_test1", OnTestItemEntCreate);
    factory.DeriveFromFactory(g_hItemBaseEntFactory );
    factory.Install();
}

static void OnItemEntityCreated(CBaseEntity entity) 
{
    PrintToServer("OnItemEntityCreated(%d)", entity.index);
}

static void OnItemEntityRemove(CBaseEntity entity)
{
    PrintToServer("OnItemEntityRemove(%d)", entity.index);
}

static void OnTestItemEntCreate(CBaseEntity entity) 
{
    PrintToServer("OnTestItemEntCreate(%d)", entity.index);
}

public void OnMapStart()
{
    int ent1 = CreateEntityByName("collectable_test1");
    int ent2 = CreateEntityByName("collectable_test1");
    RemoveEntity(ent1);
    RemoveEntity(ent2);
}

Output:

OnItemEntityCreated(47)
OnTestItemEntCreate(47)
OnItemEntityCreated(48)
OnTestItemEntCreate(48)
OnItemEntityRemove(47)
OnItemEntityRemove(48)

When creating an entity, the base factory's onCreate function will be called first before the derived factory's. On removal it's the reverse: the derived factory's onRemove function will be called first before the base factory's. If anything, I suspect the duplication behavior is stemming from invoking callbacks from g_hItemBasedEntFactories.

For what it's worth, this behavior should've been documented which I don't mind addressing in a PR to help prevent confusion in the future.

MAGNAT2645 commented 4 months ago

Oh, so it does call derived factory callbacks. When I was looking at the ext source code I could not find part where it does this so I wrote my own "callback caller" (if it had its own name) for such factories. The thing here is that OnTestItemEntCreated is not attached to the factory itself, it is called from OnItemEntityCreated. BUT now I understand why it does call them twice. In the CItemEntityFactory constructor I add callbacks to the g_hItemBasedEntFactories and set OnItemEntityCreated as the constructor to the underlying CEntityFactory. That's why it first calls base factory constructor and then for derived factory which is same as base one.

MAGNAT2645 commented 4 months ago

I'll close this for now and reopen if something will be wrong.

MAGNAT2645 commented 4 months ago

There's also a problem with quit command.

47