TF2-DMB / CBaseNPC

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

Exposing entity factories to plugins #1

Closed KitRifty closed 3 years ago

KitRifty commented 3 years ago
Past issue: Heap corruption when uninstalling a plugin defined factory (solved) Whenever a dependent plugin that creates entity factories is unloaded, the server will crash due to heap corruption. Accelerator minidumps show the problem happening at that instance, specifically when freeing Handles owned by the plugin. ![image](https://user-images.githubusercontent.com/4492504/124217288-f1269600-daac-11eb-9ca7-b783ffdccb3f.png) Analyzing the minidump with WinDbg shows it happening when `free` is called; probably it's being called on something already deleted? ![image](https://user-images.githubusercontent.com/4492504/124217178-b0c71800-daac-11eb-8db2-335c3f7dbb2a.png) ```c++ // public/tier1/utildict.h template void CUtlDict::RemoveAt(I elem) { free( (void *)m_Elements.Key( elem ) ); // crashing here m_Elements.RemoveAt(elem); } ``` Just to note: this is compiled against SDK2013 since that's what the build script is specifically set up for. I didn't touch the SDK at all either, so it's even more confusing. I don't have the ability to test it on Linux so I'm not sure if the same issue happens there too. **Solution:** The problem was an issue regarding memory allocation. `IEntityFactoryDictionary::InstallFactory` is a virtual function, so that means all memory allocation was handled by `server.dll`. `CEntityFactoryDictionaryHack::UninstallFactory` is user-defined by extension, so memory freeing was done by extension CRT. When extension attempted to free memory allocated by `server.dll`, it resulted in a heap corruption error (374). By replacing the virtual call with `m_Factories.Insert`, extension is now the one allocating memory, allowing the uninstallation of plugin-defined factories.
KitRifty commented 3 years ago

Implemented the changes as suggested! Feature-wise, the entity factories are ready, I'd be more comfortable with this if there was a little bit of testing on the Linux side, and I can only hope this compiles on Linux as well. I've done everything on Windows and everything is running well on my end.

EDIT: I spoke too soon. There's currently a bug with input functions where anything that's not a void/integer callback handler, their values aren't getting converted correctly. I'll have to review how variant_t is being passed to the input function. Solved; it was a combination of some mistakes in my testing and float not being converted to cell_t before passed to input function callback. Everything is working nicely now.

Here's a test plugin I whipped up:

#include <sourcemod>
#include <sdktools>
#include <sdkhooks>
#include <cbasenpc>

public void OnPluginStart()
{
    CEntityFactory simpleFactory = new CEntityFactory("test_simple", TestSimple_PostConstructor, TestSimple_OnRemove);
    simpleFactory.DeriveFromBaseEntity();
    simpleFactory.BeginDataMapDesc()
        .DefineIntField("m_iIntegerField")
        .DefineIntField("m_iTestField")
        .DefineFloatField("m_flTestArray", 10)
    .EndDataMapDesc();
    simpleFactory.Install();

    CEntityFactory deriveSimpleFactory = new CEntityFactory("test_simple_derived", TestSimpleDerived_PostConstructor, TestSimpleDerived_OnRemove);
    deriveSimpleFactory.DeriveFromFactory(simpleFactory);
    deriveSimpleFactory.BeginDataMapDesc()
        .DefineIntField("m_iIntKeyField", _, "IntKeyField")
        .DefineStringField("m_szStringKeyField", _, "StringKeyField")
        .DefineVectorField("m_vecKeyField", _, "VectorKeyField")
    .EndDataMapDesc();
    deriveSimpleFactory.Install();

    CEntityFactory abstractFactory = new CEntityFactory("TestAbstractEntity", TestAbstract_PostConstructor, TestAbstract_OnRemove);
    abstractFactory.IsAbstract = true;
    abstractFactory.DeriveFromClass("game_text");
    abstractFactory.BeginDataMapDesc()
        .DefineEntityField("m_hThisEntity")
        .DefineIntField("m_iThisIsFromAbstract")
    .EndDataMapDesc();
    abstractFactory.Install();

    CEntityFactory deriveAbstractFactory = new CEntityFactory("test_derivedfromabstract", TestDeriveFromAbstract_PostConstructor, TestDeriveFromAbstract_OnRemove);
    deriveAbstractFactory.DeriveFromFactory(abstractFactory);
    deriveAbstractFactory.BeginDataMapDesc()
        .DefineIntField("m_iThisIsFromDerived")
        .DefineInputFunc("TestInputVoid", InputFuncValueType_Void, TestDeriveFromAbstract_TestInputVoid)
        .DefineInputFunc("TestInputInt", InputFuncValueType_Integer, TestDeriveFromAbstract_TestInputInteger)
        .DefineInputFunc("TestInputFloat", InputFuncValueType_Float, TestDeriveFromAbstract_TestInputFloat)
        .DefineInputFunc("TestInputString", InputFuncValueType_String, TestDeriveFromAbstract_TestInputString)
    .EndDataMapDesc();
    deriveAbstractFactory.Install();
}

void SpawnTestEntities()
{
    PrintToServer("\nBEGIN CEntityFactory TEST\n");

    AssertEntity("test_simple_derived");
    AssertEntity("test_derivedfromabstract");

    PrintToServer("\nEND CEntityFactory TEST\n");
}

void AssertEntity(const char[] classname)
{
    int ent = CreateEntityByName(classname);
    if (ent == -1)
        ThrowError("assert failed: failed to spawn %s", classname);

    RemoveEntity(ent);
}

void AssertDataPropEqual(int entity, const char[] propName, int value, int element=0)
{
    int testValue = GetEntProp(entity, Prop_Data, propName, _, element);
    if (testValue != value)
        ThrowError("assert failed for dataprop %s[%d]: expected: %d, got: %d", propName, element, value, testValue);
}

void AssertDataPropArraySize(int entity, const char[] propName, int expectedValue)
{
    int testValue = GetEntPropArraySize(entity, Prop_Data, propName);
    if (testValue != expectedValue)
        ThrowError("assert failed for dataprop array %s[]: expected size: %d, got: %d", propName, expectedValue, testValue);
}

void AssertDataPropFloatEqual(int entity, const char[] propName, float value, int element=0)
{
    float testValue = GetEntPropFloat(entity, Prop_Data, propName, element);
    if (testValue != value)
        ThrowError("assert failed for dataprop %s[%d]: expected: %f, got: %f", propName, element, value, testValue);
}

void AssertDataPropEntEqual(int entity, const char[] propName, int expected, int element=0)
{
    int testValue = GetEntPropEnt(entity, Prop_Data, propName, element);
    if (testValue != expected)
        ThrowError("assert failed for dataprop %s[%d]: expected: %d, got: %d", propName, element, expected, testValue);
}

void AssertDataPropStringEqual(int entity, const char[] propName, const char[] value, int element=0)
{
    char testValue[PLATFORM_MAX_PATH];
    GetEntPropString(entity, Prop_Data, propName, testValue, sizeof(testValue), element);
    if (strcmp(testValue, value))
        ThrowError("assert failed for dataprop %s[%d]: expected: \"%s\", got: \"%s\"", propName, element, value, testValue);
}

void AssertDataPropVectorEqual(int entity, const char[] propName, const float value[3], int element=0)
{
    float testValue[3];
    GetEntPropVector(entity, Prop_Data, propName, testValue, element);
    for (int i = 0; i < 3; i++)
    {
        if (testValue[i] != value[i])
        {
            ThrowError("assert failed for dataprop %s[%d]: expected: \"%0.2f %0.2f %0.2f\", got: \"%0.2f %0.2f %0.2f\"", 
                propName, element, value[0], value[1], value[2], testValue[0], testValue[1], testValue[2]);
            return;
        }
    }
}

static void TestSimple_PostConstructor(int entity)
{
    char classname[64];
    GetEntityClassname(entity, classname, sizeof(classname));
    PrintToServer("TestSimple_PostConstructor: %s %d created", classname, entity);
    SetEntProp(entity, Prop_Data, "m_iIntegerField", 10);
    SetEntProp(entity, Prop_Data, "m_iTestField", 125);
    SetEntPropFloat(entity, Prop_Data, "m_flTestArray", 12.0, 0);
    SetEntPropFloat(entity, Prop_Data, "m_flTestArray", 13.0, 1);
    SetEntPropFloat(entity, Prop_Data, "m_flTestArray", 14.0, 2);
    SetEntPropFloat(entity, Prop_Data, "m_flTestArray", 540.0, 9);
}

static void TestSimple_OnRemove(int entity)
{
    char classname[64];
    GetEntityClassname(entity, classname, sizeof(classname));
    PrintToServer("TestSimple_OnRemove: %s %d removed", classname, entity);

    AssertDataPropEqual(entity, "m_iIntegerField", 10);
    AssertDataPropEqual(entity, "m_iTestField", 125);
    AssertDataPropArraySize(entity, "m_flTestArray", 10);
    AssertDataPropFloatEqual(entity, "m_flTestArray", 12.0, 0);
    AssertDataPropFloatEqual(entity, "m_flTestArray", 13.0, 1);
    AssertDataPropFloatEqual(entity, "m_flTestArray", 14.0, 2);
    AssertDataPropFloatEqual(entity, "m_flTestArray", 540.0, 9);
}

static void TestSimpleDerived_PostConstructor(int entity)
{
    char classname[64];
    GetEntityClassname(entity, classname, sizeof(classname));
    PrintToServer("TestSimpleDerived_PostConstructor: %s %d created", classname, entity);

    DispatchKeyValue(entity, "IntKeyField", "56");
    DispatchKeyValue(entity, "StringKeyField", "string test");
    DispatchKeyValue(entity, "VectorKeyField", "25 100 25");
}

static void TestSimpleDerived_OnRemove(int entity)
{
    char classname[64];
    GetEntityClassname(entity, classname, sizeof(classname));
    PrintToServer("TestSimpleDerived_OnRemove: %s %d removed", classname, entity);

    float expected[3] = { 25.0, 100.0, 25.0 };

    AssertDataPropEqual(entity, "m_iIntKeyField", 56);
    AssertDataPropStringEqual(entity, "m_szStringKeyField", "string test");
    AssertDataPropVectorEqual(entity, "m_vecKeyField", expected);
}

static void TestAbstract_PostConstructor(int entity)
{
    char classname[64];
    GetEntityClassname(entity, classname, sizeof(classname));
    PrintToServer("TestAbstract_PostConstructor: %s %d created", classname, entity);

    SetEntPropEnt(entity, Prop_Data, "m_hThisEntity", entity);
    SetEntProp(entity, Prop_Data, "m_iThisIsFromAbstract", 252);
}

static void TestAbstract_OnRemove(entity)
{
    char classname[64];
    GetEntityClassname(entity, classname, sizeof(classname));
    PrintToServer("TestAbstract_OnRemove: %s %d removed", classname, entity);

    AssertDataPropEntEqual(entity, "m_hThisEntity", entity);
    AssertDataPropEqual(entity, "m_iThisIsFromAbstract", 252);
}

static void TestDeriveFromAbstract_PostConstructor(int entity)
{
    char classname[64];
    GetEntityClassname(entity, classname, sizeof(classname));
    PrintToServer("TestDeriveFromAbstract_PostConstructor: %s %d created", classname, entity);

    SetEntProp(entity, Prop_Data, "m_iThisIsFromDerived", 1111);

    AcceptEntityInput(entity, "TestInputVoid");
    SetVariantInt(25);
    AcceptEntityInput(entity, "TestInputInt");
    SetVariantFloat(30.0);
    AcceptEntityInput(entity, "TestInputFloat");
    SetVariantString("55.25");
    AcceptEntityInput(entity, "TestInputString");
}

static void TestDeriveFromAbstract_OnRemove(int entity)
{
    char classname[64];
    GetEntityClassname(entity, classname, sizeof(classname));
    PrintToServer("TestDeriveFromAbstract_OnRemove: %s %d removed", classname, entity);

    AssertDataPropEqual(entity, "m_iThisIsFromDerived", 1111);
}

static void TestDeriveFromAbstract_TestInputVoid(int entity, int activator, int caller)
{
    char classname[64];
    GetEntityClassname(entity, classname, sizeof(classname));
    PrintToServer("TestDeriveFromAbstract_TestInputVoid: %s %d", classname, entity);
}

static void TestDeriveFromAbstract_TestInputInteger(int entity, int activator, int caller, int value)
{
    char classname[64];
    GetEntityClassname(entity, classname, sizeof(classname));
    PrintToServer("TestDeriveFromAbstract_TestInputInteger: %s %d, value: %d", classname, entity, value);

    int expected = 25;
    if (value != expected)
        ThrowError("Input assert failed: expected: %d, got: %d", expected, value);
}

static void TestDeriveFromAbstract_TestInputFloat(int entity, int activator, int caller, float value)
{
    char classname[64];
    GetEntityClassname(entity, classname, sizeof(classname));
    PrintToServer("TestDeriveFromAbstract_TestInputFloat: %s %d, value: %0.2f", classname, entity, value);

    float expected = 30.0;
    if (value != expected)
        ThrowError("Input assert failed: expected: %0.2f, got: %0.2f", expected, value);
}

static void TestDeriveFromAbstract_TestInputString(int entity, int activator, int caller, const char[] value)
{
    char classname[64];
    GetEntityClassname(entity, classname, sizeof(classname));
    PrintToServer("TestDeriveFromAbstract_TestInputString: %s %d, value: %s", classname, entity, value);

    char expected[] = "55.25";
    if (strcmp(value, expected))
        ThrowError("Input assert failed: expected: \"%s\", got: \"%s\"", expected, value);
}

public void OnMapStart()
{
    SpawnTestEntities();
}

Expected output upon map start:

BEGIN CEntityFactory TEST

TestSimple_PostConstructor: test_simple_derived 89 created
TestSimpleDerived_PostConstructor: test_simple_derived 89 created
TestSimpleDerived_OnRemove: test_simple_derived 89 removed
TestSimple_OnRemove: test_simple_derived 89 removed
TestAbstract_PostConstructor: test_derivedfromabstract 90 created
TestDeriveFromAbstract_PostConstructor: test_derivedfromabstract 90 created
TestDeriveFromAbstract_TestInputVoid: test_derivedfromabstract 90
TestDeriveFromAbstract_TestInputInteger: test_derivedfromabstract 90, value: 25
TestDeriveFromAbstract_TestInputFloat: test_derivedfromabstract 90, value: 30.00
TestDeriveFromAbstract_TestInputString: test_derivedfromabstract 90, value: 55.25
TestDeriveFromAbstract_OnRemove: test_derivedfromabstract 90 removed
TestAbstract_OnRemove: test_derivedfromabstract 90 removed

END CEntityFactory TEST
KitRifty commented 3 years ago

I can confirm that this compiles (at least on Ubuntu 18.04 + clang-6.0) and is working on Linux, and passes the tests from the test plugin.