diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index 75da6cbb35..5d42d8a7bd 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -1,4 +1,4 @@ -// +/3/ // Avatar.cpp // interface/src/avatar // @@ -108,6 +108,7 @@ Avatar::Avatar(RigPointer rig) : } Avatar::~Avatar() { + assert(isDead()); // mark dead before calling the dtor for(auto attachment : _unusedAttachments) { delete attachment; } diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 6075c43520..6993fb6ae1 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -76,6 +76,10 @@ AvatarManager::AvatarManager(QObject* parent) : packetReceiver.registerListener(PacketType::AvatarBillboard, this, "processAvatarBillboardPacket"); } +AvatarManager::~AvatarManager() { + _myAvatar->die(); +} + void AvatarManager::init() { _myAvatar->init(); { diff --git a/interface/src/avatar/AvatarManager.h b/interface/src/avatar/AvatarManager.h index 6fac652969..72fcb3f862 100644 --- a/interface/src/avatar/AvatarManager.h +++ b/interface/src/avatar/AvatarManager.h @@ -34,6 +34,8 @@ public: /// Registers the script types associated with the avatar manager. static void registerMetaTypes(QScriptEngine* engine); + virtual ~AvatarManager(); + void init(); MyAvatar* getMyAvatar() { return _myAvatar.get(); } diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 97043a635d..fb773ee89b 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -85,6 +85,7 @@ EntityItem::EntityItem(const EntityItemID& entityItemID) : } EntityItem::~EntityItem() { + assert(isDead()); // mark as dead before calling dtor // clear out any left-over actions EntityTreePointer entityTree = _element ? _element->getTree() : nullptr; EntitySimulation* simulation = entityTree ? entityTree->getSimulation() : nullptr; diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index 8250064326..b543d9d75b 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -48,9 +48,10 @@ bool entityTreeIsLocked() { #endif -EntityMotionState::EntityMotionState(btCollisionShape* shape, EntityItem* entity) : +EntityMotionState::EntityMotionState(btCollisionShape* shape, EntityItemPointer entity) : ObjectMotionState(shape), - _entity(entity), + _entityPtr(entity), + _entity(entity.get()), _sentInactive(true), _lastStep(0), _serverPosition(0.0f), diff --git a/libraries/physics/src/EntityMotionState.h b/libraries/physics/src/EntityMotionState.h index 578dc427a6..da78513b58 100644 --- a/libraries/physics/src/EntityMotionState.h +++ b/libraries/physics/src/EntityMotionState.h @@ -12,11 +12,11 @@ #ifndef hifi_EntityMotionState_h #define hifi_EntityMotionState_h +#include #include #include "ObjectMotionState.h" -class EntityItem; // From the MotionState's perspective: // Inside = physics simulation @@ -25,7 +25,7 @@ class EntityItem; class EntityMotionState : public ObjectMotionState { public: - EntityMotionState(btCollisionShape* shape, EntityItem* item); + EntityMotionState(btCollisionShape* shape, EntityItemPointer item); virtual ~EntityMotionState(); void updateServerPhysicsVariables(const QUuid& sessionID); @@ -73,7 +73,7 @@ public: virtual QUuid getSimulatorID() const; virtual void bump(quint8 priority); - EntityItem* getEntity() const { return _entity; } + EntityItemPointer getEntity() const { return _entityPtr.lock(); } void resetMeasuredBodyAcceleration(); void measureBodyAcceleration(); @@ -96,7 +96,15 @@ protected: virtual btCollisionShape* computeNewShape(); virtual void setMotionType(MotionType motionType); - EntityItem* _entity { nullptr }; // do NOT use smartpointer here + // In the glorious future (when entities lib depends on physics lib) the EntityMotionState will be + // properly "owned" by the EntityItem and will be deleted by it in the dtor. In pursuit of that + // state of affairs we can't keep a real EntityItemPointer as data member (it would produce a + // recursive dependency). Instead we keep a EntityItemWeakPointer to break that dependency while + // still granting us the capability to generate EntityItemPointers as necessary (for external data + // structures that use the MotionState to get to the EntityItem). + EntityItemWeakPointer _entityPtr; + // Meanwhile we also keep a raw EntityItem* for internal stuff where the pointer is guaranteed valid. + EntityItem* _entity; bool _sentInactive; // true if body was inactive when we sent last update diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index 1281edb205..4116ddeda0 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -122,17 +122,15 @@ void PhysicalEntitySimulation::clearEntitiesInternal() { // first disconnect each MotionStates from its Entity for (auto stateItr : _physicalObjects) { EntityMotionState* motionState = static_cast(&(*stateItr)); - EntityItem* entity = motionState->getEntity(); - assert(entity); - _entitiesToDelete.insert(EntityItemPointer(entity)); + _entitiesToDelete.insert(motionState->getEntity()); } - // then remove the objects from physics (aka MotionStates) + // then remove the objects (aka MotionStates) from physics _physicsEngine->removeObjects(_physicalObjects); - // delete the objects (aka MotionStates) - // Someday when we invert the entities/physics lib dependencies we can let EntityItem delete its own PhysicsInfo - // rather than do it here + // delete the MotionStates + // TODO: after we invert the entities/physics lib dependencies we will let EntityItem delete + // its own PhysicsInfo rather than do it here for (auto entity : _entitiesToDelete) { EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); if (motionState) { @@ -182,15 +180,15 @@ void PhysicalEntitySimulation::getObjectsToAddToPhysics(VectorOfMotionStates& re QMutexLocker lock(&_mutex); SetOfEntities::iterator entityItr = _entitiesToAddToPhysics.begin(); while (entityItr != _entitiesToAddToPhysics.end()) { - EntityItem* entity = (*entityItr).get(); + EntityItemPointer entity = (*entityItr); assert(!entity->getPhysicsInfo()); if (entity->isDead()) { - prepareEntityForDelete(EntityItemPointer(entity)); + prepareEntityForDelete(entity); } else if (!entity->shouldBePhysical()) { // this entity should no longer be on the internal _entitiesToAddToPhysics entityItr = _entitiesToAddToPhysics.erase(entityItr); if (entity->isMoving()) { - _simpleKinematicEntities.insert(EntityItemPointer(entity)); + _simpleKinematicEntities.insert(entity); } } else if (entity->isReadyToComputeShape()) { ShapeInfo shapeInfo; @@ -236,12 +234,12 @@ void PhysicalEntitySimulation::handleOutgoingChanges(const VectorOfMotionStates& ObjectMotionState* state = &(*stateItr); if (state && state->getType() == MOTIONSTATE_TYPE_ENTITY) { EntityMotionState* entityState = static_cast(state); - EntityItem* entity = entityState->getEntity(); - assert(entity); + EntityItemPointer entity = entityState->getEntity(); + assert(entity.get()); if (entityState->isCandidateForOwnership(sessionID)) { _outgoingChanges.insert(entityState); } - _entitiesToSort.insert(EntityItemPointer(entity)); + _entitiesToSort.insert(entity); } } diff --git a/libraries/shared/src/SpatiallyNestable.h b/libraries/shared/src/SpatiallyNestable.h index 915d247b92..dc38671091 100644 --- a/libraries/shared/src/SpatiallyNestable.h +++ b/libraries/shared/src/SpatiallyNestable.h @@ -34,7 +34,7 @@ enum class NestableType { class SpatiallyNestable : public std::enable_shared_from_this { public: SpatiallyNestable(NestableType nestableType, QUuid id); - virtual ~SpatiallyNestable() { assert(_isDead); } + virtual ~SpatiallyNestable() { } virtual const QUuid& getID() const { return _id; } virtual void setID(const QUuid& id) { _id = id; }