MADEAPPS / newton-dynamics

Newton Dynamics is an integrated solution for real time simulation of physics environments.
http://www.newtondynamics.com
Other
936 stars 182 forks source link

Trying To Delete NULL Pointer #260

Closed kklouzal closed 2 years ago

kklouzal commented 2 years ago

Seems as though you are not allowed to inherit from ndBodyNotify if you also inherit from other types like ndBodyPlayercapsule

class MyCharacterSceneNode : public MySceneNode, public ndBodyPlayerCapsule, public ndBodyNotify

Then later you must create the object like follows:

MyCharacterSceneNode* MeshNode = new CharacterSceneNode(Mesh, localAxis, mass, radius, height, height/4.0f);
...
MeshNode->SetNotifyCallback(MeshNode);
...

When ndWorld does cleanup, it will try to delete MyCharacterSceneNode twice because it first deletes ndBodyPlayerCapsule then also again deletes it becase ndBodyNotify

>   ndNewton_d.dll!dMemory::Free(void * const ptr) Line 66  C++
    ndNewton_d.dll!dFreeListDictionary::Flush(dFreeListHeader * const header) Line 72   C++
    ndNewton_d.dll!dFreeListDictionary::Flush() Line 116    C++
    ndNewton_d.dll!dFreeListAlloc::Flush() Line 190 C++
    ndNewton_d.dll!ndScene::Cleanup() Line 1913 C++
    ndNewton_d.dll!ndScene::~ndScene() Line 212 C++
    [External Code] 
    ndNewton_d.dll!ndWorld::~ndWorld() Line 161 C++
JulioJerez commented 2 years ago

but this code class MyCharacterSceneNode : public MySceneNode, public ndBodyPlayerCapsule, public ndBodyNotify

is a big mistake, a body notifier is a member of ndBody you should make one and assign it to the body using this function bellow D_COLLISION_API void SetNotifyCallback(ndBodyNotify* const notify);

you cna look at the player capsule in the sandbox, there is one notifies class fo it

class ndBasicPlayerCapsuleNotify : public ndDemoEntityNotify
{
    public:
    ndBasicPlayerCapsuleNotify(ndDemoEntityManager* const manager, ndDemoEntity* const entity)
        :ndDemoEntityNotify(manager, entity)
    {
    }

    void OnTransform(dInt32 thread, const ndMatrix& matrix)
    {
        ndDemoEntityNotify::OnTransform(thread, matrix);

        ndWorld* const word = m_manager->GetWorld();
        ndBasicPlayerCapsule* const player = (ndBasicPlayerCapsule*)GetBody();

        dFloat32 timestep = word->GetScene()->GetTimestep();
        //timestep *= 0.25f;
        //timestep = 1.0f/(30.0f * 4.0f);
        timestep *= 0.05f;
        player->m_animBlendTree->Evaluate(player->m_output, timestep);

        for (int i = 0; i < player->m_output.GetCount(); i++)
        {
            const ndAnimKeyframe& keyFrame = player->m_output[i];
            ndDemoEntity* const entity = (ndDemoEntity*)keyFrame.m_userData;
            entity->SetMatrix(keyFrame.m_rotation, keyFrame.m_posit);
        }
    }
};

and it used like this:

ndBasicPlayerCapsule::ndBasicPlayerCapsule(
    ndDemoEntityManager* const scene, const ndDemoEntity* const modelEntity,
    const ndMatrix& localAxis, const ndMatrix& location,
    dFloat32 mass, dFloat32 radius, dFloat32 height, dFloat32 stepHeight, bool isPlayer)
    :ndBodyPlayerCapsule(localAxis, mass, radius, height, stepHeight)
    ,m_scene(scene)
    ,m_isPlayer(isPlayer)
    ,m_output()
    ,m_animBlendTree(nullptr)
{
    ndMatrix matrix(location);
    ndPhysicsWorld* const world = scene->GetWorld();
    ndVector floor(FindFloor(*world, matrix.m_posit + ndVector(0.0f, 100.0f, 0.0f, 0.0f), 200.0f));
    //matrix.m_posit.m_y = floor.m_y + 1.0f;
    matrix.m_posit.m_y = floor.m_y;

    ndDemoEntity* const entity = (ndDemoEntity*)modelEntity->CreateClone();
    entity->ResetMatrix(matrix);

    SetMatrix(matrix);
    world->AddBody(this);
    scene->AddEntity(entity);
    SetNotifyCallback(new ndBasicPlayerCapsuleNotify(scene, entity));

basically, attach a notifier to a body, is not a class member

kklouzal commented 2 years ago

Okay, I had a feeling I needed to do it like this. Would have been convenient to just inherit from ndBodyNotify but no problem :)

JulioJerez commented 2 years ago

your class should be like this class MyCharacterSceneNode : public MySceneNode, public ndBodyPlayerCapsule

them you make one modifier class that you can use as the base class of all your notifications for all your bodies like this

class ndDemoEntityNotify: public ndBodyNotify
{
    public:
    D_CLASS_REFLECTION(ndDemoEntityNotify);
    ndDemoEntityNotify(const ndLoadSaveBase::dLoadDescriptor& desc);
    ndDemoEntityNotify(ndDemoEntityManager* const manager, ndDemoEntity* const entity, ndBodyDynamic* const parentBody = nullptr, dFloat32 gravity = DEMO_GRAVITY);
    virtual ~ndDemoEntityNotify();

    void* GetUserData() const
    {
        return m_entity;
    }

    virtual void OnObjectPick() const;
    virtual void OnTransform(dInt32 threadIndex, const ndMatrix& matrix);
    virtual void OnApplyExternalForce(dInt32 threadIndex, dFloat32 timestep);

    virtual void Save(const ndLoadSaveBase::ndSaveDescriptor& desc) const;

    ndDemoEntity* m_entity;
    ndBodyDynamic* m_parentBody;
    ndDemoEntityManager* m_manager;
};

if you look at that class, it is quite cool because it handles parent child relation, that's one of the differences with 3.14
since the notification is and actual; object it can save parent child information so that is can calculate local transform making easier to make bodies children of some other bodies. this is no impossible with 3.14 but very ugly spaghetti with user data and pointer.

kklouzal commented 2 years ago

Is separated it out like you had suggested here and everything is cleaning up properly now 👍

Ahh I understand, parents can directly call the notify of it's child which in turn will translate it appropriately. It works down the chain of parent->child and everything stays happy 💯