diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 4a166d9b12..65d9ae1ba7 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1084,8 +1084,8 @@ Application::~Application() { // remove avatars from physics engine DependencyManager::get()->clearOtherAvatars(); VectorOfMotionStates motionStates; - DependencyManager::get()->getObjectsToDelete(motionStates); - _physicsEngine->deleteObjects(motionStates); + DependencyManager::get()->getObjectsToRemoveFromPhysics(motionStates); + _physicsEngine->removeObjects(motionStates); DependencyManager::destroy(); DependencyManager::destroy(); @@ -3085,11 +3085,11 @@ void Application::update(float deltaTime) { PerformanceTimer perfTimer("physics"); static VectorOfMotionStates motionStates; - _entitySimulation.getObjectsToDelete(motionStates); - _physicsEngine->deleteObjects(motionStates); + _entitySimulation.getObjectsToRemoveFromPhysics(motionStates); + _physicsEngine->removeObjects(motionStates); getEntities()->getTree()->withWriteLock([&] { - _entitySimulation.getObjectsToAdd(motionStates); + _entitySimulation.getObjectsToAddToPhysics(motionStates); _physicsEngine->addObjects(motionStates); }); @@ -3102,9 +3102,9 @@ void Application::update(float deltaTime) { _entitySimulation.applyActionChanges(); AvatarManager* avatarManager = DependencyManager::get().data(); - avatarManager->getObjectsToDelete(motionStates); - _physicsEngine->deleteObjects(motionStates); - avatarManager->getObjectsToAdd(motionStates); + avatarManager->getObjectsToRemoveFromPhysics(motionStates); + _physicsEngine->removeObjects(motionStates); + avatarManager->getObjectsToAddToPhysics(motionStates); _physicsEngine->addObjects(motionStates); avatarManager->getObjectsToChange(motionStates); _physicsEngine->changeObjects(motionStates); @@ -4083,7 +4083,7 @@ bool Application::nearbyEntitiesAreReadyForPhysics() { }); foreach (EntityItemPointer entity, entities) { - if (!entity->isReadyToComputeShape()) { + if (entity->shouldBePhysical() && !entity->isReadyToComputeShape()) { static QString repeatedMessage = LogHandler::getInstance().addRepeatedMessageRegex("Physics disabled until entity loads: .*"); qCDebug(interfaceapp) << "Physics disabled until entity loads: " << entity->getID() << entity->getName(); diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index f532798c2d..75da6cbb35 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -108,7 +108,13 @@ Avatar::Avatar(RigPointer rig) : } Avatar::~Avatar() { - assert(_motionState == nullptr); + for(auto attachment : _unusedAttachments) { + delete attachment; + } + if (_motionState) { + delete _motionState; + _motionState = nullptr; + } } const float BILLBOARD_LOD_DISTANCE = 40.0f; diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 2ef3956d99..6075c43520 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -165,7 +165,12 @@ void AvatarManager::simulateAvatarFades(float deltaTime) { avatar->setTargetScale(avatar->getUniformScale() * SHRINK_RATE); if (avatar->getTargetScale() <= MIN_FADE_SCALE) { avatar->removeFromScene(*fadingIterator, scene, pendingChanges); - fadingIterator = _avatarFades.erase(fadingIterator); + // only remove from _avatarFades if we're sure its motionState has been removed from PhysicsEngine + if (_motionStatesToRemoveFromPhysics.empty()) { + fadingIterator = _avatarFades.erase(fadingIterator); + } else { + ++fadingIterator; + } } else { avatar->simulate(deltaTime); ++fadingIterator; @@ -193,20 +198,6 @@ AvatarSharedPointer AvatarManager::addAvatar(const QUuid& sessionUUID, const QWe return newAvatar; } -// protected -void AvatarManager::removeAvatarMotionState(AvatarSharedPointer avatar) { - auto rawPointer = std::static_pointer_cast(avatar); - AvatarMotionState* motionState = rawPointer->getMotionState(); - if (motionState) { - // clean up physics stuff - motionState->clearObjectBackPointer(); - rawPointer->setMotionState(nullptr); - _avatarMotionStates.remove(motionState); - _motionStatesToAdd.remove(motionState); - _motionStatesToDelete.push_back(motionState); - } -} - // virtual void AvatarManager::removeAvatar(const QUuid& sessionUUID) { QWriteLocker locker(&_hashLock); @@ -220,8 +211,16 @@ void AvatarManager::removeAvatar(const QUuid& sessionUUID) { void AvatarManager::handleRemovedAvatar(const AvatarSharedPointer& removedAvatar) { AvatarHashMap::handleRemovedAvatar(removedAvatar); - removedAvatar->die(); - removeAvatarMotionState(removedAvatar); + Avatar* avatar = static_cast(removedAvatar.get()); + avatar->die(); + + AvatarMotionState* motionState = avatar->getMotionState(); + if (motionState) { + _motionStatesThatMightUpdate.remove(motionState); + _motionStatesToAddToPhysics.remove(motionState); + _motionStatesToRemoveFromPhysics.push_back(motionState); + } + _avatarFades.push_back(removedAvatar); } @@ -274,22 +273,22 @@ AvatarData* AvatarManager::getAvatar(QUuid avatarID) { } -void AvatarManager::getObjectsToDelete(VectorOfMotionStates& result) { +void AvatarManager::getObjectsToRemoveFromPhysics(VectorOfMotionStates& result) { result.clear(); - result.swap(_motionStatesToDelete); + result.swap(_motionStatesToRemoveFromPhysics); } -void AvatarManager::getObjectsToAdd(VectorOfMotionStates& result) { +void AvatarManager::getObjectsToAddToPhysics(VectorOfMotionStates& result) { result.clear(); - for (auto motionState : _motionStatesToAdd) { + for (auto motionState : _motionStatesToAddToPhysics) { result.push_back(motionState); } - _motionStatesToAdd.clear(); + _motionStatesToAddToPhysics.clear(); } void AvatarManager::getObjectsToChange(VectorOfMotionStates& result) { result.clear(); - for (auto state : _avatarMotionStates) { + for (auto state : _motionStatesThatMightUpdate) { if (state->_dirtyFlags > 0) { result.push_back(state); } @@ -344,8 +343,8 @@ void AvatarManager::addAvatarToSimulation(Avatar* avatar) { // we don't add to the simulation now, we put it on a list to be added later AvatarMotionState* motionState = new AvatarMotionState(avatar, shape); avatar->setMotionState(motionState); - _motionStatesToAdd.insert(motionState); - _avatarMotionStates.insert(motionState); + _motionStatesToAddToPhysics.insert(motionState); + _motionStatesThatMightUpdate.insert(motionState); } } diff --git a/interface/src/avatar/AvatarManager.h b/interface/src/avatar/AvatarManager.h index 8494ce0b40..6fac652969 100644 --- a/interface/src/avatar/AvatarManager.h +++ b/interface/src/avatar/AvatarManager.h @@ -59,8 +59,8 @@ public: Q_INVOKABLE AvatarData* getAvatar(QUuid avatarID); - void getObjectsToDelete(VectorOfMotionStates& motionStates); - void getObjectsToAdd(VectorOfMotionStates& motionStates); + void getObjectsToRemoveFromPhysics(VectorOfMotionStates& motionStates); + void getObjectsToAddToPhysics(VectorOfMotionStates& motionStates); void getObjectsToChange(VectorOfMotionStates& motionStates); void handleOutgoingChanges(const VectorOfMotionStates& motionStates); void handleCollisionEvents(const CollisionEvents& collisionEvents); @@ -80,7 +80,6 @@ private: // virtual overrides virtual AvatarSharedPointer newSharedAvatar(); virtual AvatarSharedPointer addAvatar(const QUuid& sessionUUID, const QWeakPointer& mixerWeakPointer); - void removeAvatarMotionState(AvatarSharedPointer avatar); virtual void removeAvatar(const QUuid& sessionUUID); virtual void handleRemovedAvatar(const AvatarSharedPointer& removedAvatar); @@ -93,9 +92,9 @@ private: bool _shouldShowReceiveStats = false; - SetOfAvatarMotionStates _avatarMotionStates; - SetOfMotionStates _motionStatesToAdd; - VectorOfMotionStates _motionStatesToDelete; + SetOfAvatarMotionStates _motionStatesThatMightUpdate; + SetOfMotionStates _motionStatesToAddToPhysics; + VectorOfMotionStates _motionStatesToRemoveFromPhysics; }; Q_DECLARE_METATYPE(AvatarManager::LocalLight) diff --git a/interface/src/avatar/AvatarMotionState.cpp b/interface/src/avatar/AvatarMotionState.cpp index 9acbe6c3d4..f61533a5e3 100644 --- a/interface/src/avatar/AvatarMotionState.cpp +++ b/interface/src/avatar/AvatarMotionState.cpp @@ -25,20 +25,17 @@ AvatarMotionState::AvatarMotionState(Avatar* avatar, btCollisionShape* shape) : } AvatarMotionState::~AvatarMotionState() { + assert(_avatar); _avatar = nullptr; } // virtual uint32_t AvatarMotionState::getIncomingDirtyFlags() { - uint32_t dirtyFlags = 0; - if (_body && _avatar) { - dirtyFlags = _dirtyFlags; - } - return dirtyFlags; + return _body ? _dirtyFlags : 0; } void AvatarMotionState::clearIncomingDirtyFlags() { - if (_body && _avatar) { + if (_body) { _dirtyFlags = 0; } } @@ -50,12 +47,9 @@ MotionType AvatarMotionState::computeObjectMotionType() const { // virtual and protected btCollisionShape* AvatarMotionState::computeNewShape() { - if (_avatar) { - ShapeInfo shapeInfo; - _avatar->computeShapeInfo(shapeInfo); - return getShapeManager()->getShape(shapeInfo); - } - return nullptr; + ShapeInfo shapeInfo; + _avatar->computeShapeInfo(shapeInfo); + return getShapeManager()->getShape(shapeInfo); } // virtual @@ -65,9 +59,6 @@ bool AvatarMotionState::isMoving() const { // virtual void AvatarMotionState::getWorldTransform(btTransform& worldTrans) const { - if (!_avatar) { - return; - } worldTrans.setOrigin(glmToBullet(getObjectPosition())); worldTrans.setRotation(glmToBullet(getObjectRotation())); if (_body) { @@ -78,9 +69,6 @@ void AvatarMotionState::getWorldTransform(btTransform& worldTrans) const { // virtual void AvatarMotionState::setWorldTransform(const btTransform& worldTrans) { - if (!_avatar) { - return; - } // HACK: The PhysicsEngine does not actually move OTHER avatars -- instead it slaves their local RigidBody to the transform // as specified by a remote simulation. However, to give the remote simulation time to respond to our own objects we tie // the other avatar's body to its true position with a simple spring. This is a HACK that will have to be improved later. @@ -159,10 +147,3 @@ int16_t AvatarMotionState::computeCollisionGroup() { return COLLISION_GROUP_OTHER_AVATAR; } -// virtual -void AvatarMotionState::clearObjectBackPointer() { - ObjectMotionState::clearObjectBackPointer(); - _avatar = nullptr; -} - - diff --git a/interface/src/avatar/AvatarMotionState.h b/interface/src/avatar/AvatarMotionState.h index b5101d2c70..8f60c8e607 100644 --- a/interface/src/avatar/AvatarMotionState.h +++ b/interface/src/avatar/AvatarMotionState.h @@ -68,8 +68,7 @@ public: protected: virtual bool isReadyToComputeShape() { return true; } virtual btCollisionShape* computeNewShape(); - virtual void clearObjectBackPointer(); - Avatar* _avatar; + Avatar* _avatar; // do NOT use smartpointer here uint32_t _dirtyFlags; }; diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 83195364bf..542b0598ce 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -344,9 +344,6 @@ public: glm::vec3 getClientGlobalPosition() { return _globalPosition; } - void die() { _isDead = true; } - bool isDead() const { return _isDead; } - public slots: void sendAvatarDataPacket(); void sendIdentityPacket(); @@ -423,8 +420,6 @@ protected: // updates about one avatar to another. glm::vec3 _globalPosition; - bool _isDead { false }; - private: friend void avatarStateFromFrame(const QByteArray& frameData, AvatarData* _avatar); static QUrl _defaultFullAvatarModelUrl; diff --git a/libraries/entities-renderer/src/RenderablePolyVoxEntityItem.h b/libraries/entities-renderer/src/RenderablePolyVoxEntityItem.h index 0411945ede..efd9b4afda 100644 --- a/libraries/entities-renderer/src/RenderablePolyVoxEntityItem.h +++ b/libraries/entities-renderer/src/RenderablePolyVoxEntityItem.h @@ -77,7 +77,7 @@ public: glm::mat4 localToVoxelMatrix() const; virtual ShapeType getShapeType() const; - virtual bool shouldBePhysical() const { return true; } + virtual bool shouldBePhysical() const { return !isDead(); } virtual bool isReadyToComputeShape(); virtual void computeShapeInfo(ShapeInfo& info); diff --git a/libraries/entities/src/BoxEntityItem.h b/libraries/entities/src/BoxEntityItem.h index 351feb7e54..6196346b9a 100644 --- a/libraries/entities/src/BoxEntityItem.h +++ b/libraries/entities/src/BoxEntityItem.h @@ -53,7 +53,7 @@ public: } virtual ShapeType getShapeType() const { return SHAPE_TYPE_BOX; } - virtual bool shouldBePhysical() const { return true; } + virtual bool shouldBePhysical() const { return !isDead(); } virtual void debugDump() const; diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index 4d7106b858..83f2ad164e 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -302,7 +302,7 @@ public: virtual bool contains(const glm::vec3& point) const; - virtual bool isReadyToComputeShape() { return true; } + virtual bool isReadyToComputeShape() { return !isDead(); } virtual void computeShapeInfo(ShapeInfo& info); virtual float getVolumeEstimate() const { return getDimensions().x * getDimensions().y * getDimensions().z; } diff --git a/libraries/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp index 62912b8954..35d5b18fda 100644 --- a/libraries/entities/src/EntitySimulation.cpp +++ b/libraries/entities/src/EntitySimulation.cpp @@ -38,16 +38,17 @@ void EntitySimulation::updateEntities() { sortEntitiesThatMoved(); } -void EntitySimulation::getEntitiesToDelete(VectorOfEntities& entitiesToDelete) { +void EntitySimulation::takeEntitiesToDelete(VectorOfEntities& entitiesToDelete) { QMutexLocker lock(&_mutex); for (auto entity : _entitiesToDelete) { - // this entity is still in its tree, so we insert into the external list + // push this entity onto the external list entitiesToDelete.push_back(entity); } _entitiesToDelete.clear(); } void EntitySimulation::removeEntityInternal(EntityItemPointer entity) { + // remove from all internal lists except _entitiesToDelete _mortalEntities.remove(entity); _entitiesToUpdate.remove(entity); _entitiesToSort.remove(entity); @@ -59,6 +60,7 @@ void EntitySimulation::removeEntityInternal(EntityItemPointer entity) { void EntitySimulation::prepareEntityForDelete(EntityItemPointer entity) { assert(entity); assert(entity->isSimulated()); + assert(entity->isDead()); entity->clearActions(this); removeEntityInternal(entity); _entitiesToDelete.insert(entity); @@ -89,6 +91,7 @@ void EntitySimulation::expireMortalEntities(const quint64& now) { quint64 expiry = entity->getExpiry(); if (expiry < now) { itemItr = _mortalEntities.erase(itemItr); + entity->die(); prepareEntityForDelete(entity); } else { if (expiry < _nextExpiry) { @@ -134,6 +137,7 @@ void EntitySimulation::sortEntitiesThatMoved() { if (success && !domainBounds.touches(newCube)) { qCDebug(entities) << "Entity " << entity->getEntityItemID() << " moved out of domain bounds."; itemItr = _entitiesToSort.erase(itemItr); + entity->die(); prepareEntityForDelete(entity); } else { moveOperator.addEntityToMoveList(entity, newCube); @@ -193,6 +197,7 @@ void EntitySimulation::changeEntity(EntityItemPointer entity) { AACube newCube = entity->getQueryAACube(success); if (success && !domainBounds.touches(newCube)) { qCDebug(entities) << "Entity " << entity->getEntityItemID() << " moved out of domain bounds."; + entity->die(); prepareEntityForDelete(entity); wasRemoved = true; } @@ -226,14 +231,15 @@ void EntitySimulation::clearEntities() { _entitiesToUpdate.clear(); _entitiesToSort.clear(); _simpleKinematicEntities.clear(); - _entitiesToDelete.clear(); clearEntitiesInternal(); - for (auto entityItr : _allEntities) { - entityItr->setSimulated(false); + for (auto entity : _allEntities) { + entity->setSimulated(false); + entity->die(); } _allEntities.clear(); + _entitiesToDelete.clear(); } void EntitySimulation::moveSimpleKinematics(const quint64& now) { diff --git a/libraries/entities/src/EntitySimulation.h b/libraries/entities/src/EntitySimulation.h index f1548b50e9..442ff4a74b 100644 --- a/libraries/entities/src/EntitySimulation.h +++ b/libraries/entities/src/EntitySimulation.h @@ -73,7 +73,7 @@ public: EntityTreePointer getEntityTree() { return _entityTree; } - void getEntitiesToDelete(VectorOfEntities& entitiesToDelete); + virtual void takeEntitiesToDelete(VectorOfEntities& entitiesToDelete); /// \param entity pointer to EntityItem that needs to be put on the entitiesToDelete list and removed from others. virtual void prepareEntityForDelete(EntityItemPointer entity); diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index af6e1f5ef1..9861252d63 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -442,6 +442,7 @@ void EntityTree::processRemovedEntities(const DeleteEntityOperator& theOperator) const RemovedEntities& entities = theOperator.getEntities(); foreach(const EntityToDeleteDetails& details, entities) { EntityItemPointer theEntity = details.entity; + theEntity->die(); if (getIsServer()) { // set up the deleted entities ID @@ -1005,7 +1006,7 @@ void EntityTree::update() { withWriteLock([&] { _simulation->updateEntities(); VectorOfEntities pendingDeletes; - _simulation->getEntitiesToDelete(pendingDeletes); + _simulation->takeEntitiesToDelete(pendingDeletes); if (pendingDeletes.size() > 0) { // translate into list of ID's diff --git a/libraries/entities/src/ModelEntityItem.cpp b/libraries/entities/src/ModelEntityItem.cpp index 709cd67ef5..bef4406f71 100644 --- a/libraries/entities/src/ModelEntityItem.cpp +++ b/libraries/entities/src/ModelEntityItem.cpp @@ -377,7 +377,7 @@ void ModelEntityItem::setAnimationFPS(float value) { // virtual bool ModelEntityItem::shouldBePhysical() const { - return getShapeType() != SHAPE_TYPE_NONE; + return !isDead() && getShapeType() != SHAPE_TYPE_NONE; } void ModelEntityItem::resizeJointArrays(int newSize) { diff --git a/libraries/entities/src/SphereEntityItem.h b/libraries/entities/src/SphereEntityItem.h index 941d5a167c..fda5eab009 100644 --- a/libraries/entities/src/SphereEntityItem.h +++ b/libraries/entities/src/SphereEntityItem.h @@ -52,7 +52,7 @@ public: } virtual ShapeType getShapeType() const { return SHAPE_TYPE_SPHERE; } - virtual bool shouldBePhysical() const { return true; } + virtual bool shouldBePhysical() const { return !isDead(); } virtual bool supportsDetailedRayIntersection() const { return true; } virtual bool findDetailedRayIntersection(const glm::vec3& origin, const glm::vec3& direction, diff --git a/libraries/entities/src/ZoneEntityItem.h b/libraries/entities/src/ZoneEntityItem.h index 19206f8acc..bf323248c0 100644 --- a/libraries/entities/src/ZoneEntityItem.h +++ b/libraries/entities/src/ZoneEntityItem.h @@ -57,7 +57,7 @@ public: static bool getDrawZoneBoundaries() { return _drawZoneBoundaries; } static void setDrawZoneBoundaries(bool value) { _drawZoneBoundaries = value; } - virtual bool isReadyToComputeShape() { return true; } + virtual bool isReadyToComputeShape() { return false; } void updateShapeType(ShapeType type) { _shapeType = type; } virtual ShapeType getShapeType() const; diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index cd07b4112f..8250064326 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -34,7 +34,7 @@ const quint64 USECS_BETWEEN_OWNERSHIP_BIDS = USECS_PER_SECOND / 5; #ifdef WANT_DEBUG_ENTITY_TREE_LOCKS bool EntityMotionState::entityTreeIsLocked() const { - EntityTreeElementPointer element = _entity ? _entity->getElement() : nullptr; + EntityTreeElementPointer element = _entity->getElement(); EntityTreePointer tree = element ? element->getTree() : nullptr; if (!tree) { return true; @@ -48,7 +48,7 @@ bool entityTreeIsLocked() { #endif -EntityMotionState::EntityMotionState(btCollisionShape* shape, EntityItemPointer entity) : +EntityMotionState::EntityMotionState(btCollisionShape* shape, EntityItem* entity) : ObjectMotionState(shape), _entity(entity), _sentInactive(true), @@ -69,14 +69,14 @@ EntityMotionState::EntityMotionState(btCollisionShape* shape, EntityItemPointer _loopsWithoutOwner(0) { _type = MOTIONSTATE_TYPE_ENTITY; - assert(_entity != nullptr); + assert(_entity); assert(entityTreeIsLocked()); setMass(_entity->computeMass()); } EntityMotionState::~EntityMotionState() { - // be sure to clear _entity before calling the destructor - assert(!_entity); + assert(_entity); + _entity = nullptr; } void EntityMotionState::updateServerPhysicsVariables(const QUuid& sessionID) { @@ -138,11 +138,6 @@ bool EntityMotionState::handleHardAndEasyChanges(uint32_t& flags, PhysicsEngine* return ObjectMotionState::handleHardAndEasyChanges(flags, engine); } -void EntityMotionState::clearObjectBackPointer() { - ObjectMotionState::clearObjectBackPointer(); - _entity = nullptr; -} - MotionType EntityMotionState::computeObjectMotionType() const { if (!_entity) { return MOTION_TYPE_STATIC; @@ -222,21 +217,15 @@ void EntityMotionState::setWorldTransform(const btTransform& worldTrans) { // virtual and protected bool EntityMotionState::isReadyToComputeShape() { - if (_entity) { - return _entity->isReadyToComputeShape(); - } - return false; + return _entity->isReadyToComputeShape(); } // virtual and protected btCollisionShape* EntityMotionState::computeNewShape() { - if (_entity) { - ShapeInfo shapeInfo; - assert(entityTreeIsLocked()); - _entity->computeShapeInfo(shapeInfo); - return getShapeManager()->getShape(shapeInfo); - } - return nullptr; + ShapeInfo shapeInfo; + assert(entityTreeIsLocked()); + _entity->computeShapeInfo(shapeInfo); + return getShapeManager()->getShape(shapeInfo); } bool EntityMotionState::isCandidateForOwnership(const QUuid& sessionID) const { @@ -553,26 +542,17 @@ void EntityMotionState::clearIncomingDirtyFlags() { // virtual quint8 EntityMotionState::getSimulationPriority() const { - if (_entity) { - return _entity->getSimulationPriority(); - } - return NO_PRORITY; + return _entity->getSimulationPriority(); } // virtual QUuid EntityMotionState::getSimulatorID() const { - if (_entity) { - assert(entityTreeIsLocked()); - return _entity->getSimulatorID(); - } - return QUuid(); + assert(entityTreeIsLocked()); + return _entity->getSimulatorID(); } -// virtual void EntityMotionState::bump(quint8 priority) { - if (_entity) { - setOutgoingPriority(glm::max(VOLUNTEER_SIMULATION_PRIORITY, --priority)); - } + setOutgoingPriority(glm::max(VOLUNTEER_SIMULATION_PRIORITY, --priority)); } void EntityMotionState::resetMeasuredBodyAcceleration() { @@ -624,18 +604,12 @@ void EntityMotionState::setMotionType(MotionType motionType) { // virtual QString EntityMotionState::getName() { - if (_entity) { - assert(entityTreeIsLocked()); - return _entity->getName(); - } - return ""; + assert(entityTreeIsLocked()); + return _entity->getName(); } // virtual int16_t EntityMotionState::computeCollisionGroup() { - if (!_entity) { - return COLLISION_GROUP_STATIC; - } if (_entity->getIgnoreForCollisions()) { return COLLISION_GROUP_COLLISIONLESS; } diff --git a/libraries/physics/src/EntityMotionState.h b/libraries/physics/src/EntityMotionState.h index c666f87221..578dc427a6 100644 --- a/libraries/physics/src/EntityMotionState.h +++ b/libraries/physics/src/EntityMotionState.h @@ -25,7 +25,7 @@ class EntityItem; class EntityMotionState : public ObjectMotionState { public: - EntityMotionState(btCollisionShape* shape, EntityItemPointer item); + EntityMotionState(btCollisionShape* shape, EntityItem* item); virtual ~EntityMotionState(); void updateServerPhysicsVariables(const QUuid& sessionID); @@ -73,7 +73,7 @@ public: virtual QUuid getSimulatorID() const; virtual void bump(quint8 priority); - EntityItemPointer getEntity() const { return _entity; } + EntityItem* getEntity() const { return _entity; } void resetMeasuredBodyAcceleration(); void measureBodyAcceleration(); @@ -94,10 +94,9 @@ protected: virtual bool isReadyToComputeShape(); virtual btCollisionShape* computeNewShape(); - virtual void clearObjectBackPointer(); virtual void setMotionType(MotionType motionType); - EntityItemPointer _entity; + EntityItem* _entity { nullptr }; // do NOT use smartpointer here bool _sentInactive; // true if body was inactive when we sent last update diff --git a/libraries/physics/src/ObjectMotionState.cpp b/libraries/physics/src/ObjectMotionState.cpp index 4d7df9b43c..c434f67ad2 100644 --- a/libraries/physics/src/ObjectMotionState.cpp +++ b/libraries/physics/src/ObjectMotionState.cpp @@ -71,9 +71,9 @@ ObjectMotionState::ObjectMotionState(btCollisionShape* shape) : } ObjectMotionState::~ObjectMotionState() { - // adebug TODO: move shape release out of PhysicsEngine and into into the ObjectMotionState dtor assert(!_body); - assert(!_shape); + releaseShape(); + _type = MOTIONSTATE_TYPE_INVALID; } void ObjectMotionState::setBodyLinearVelocity(const glm::vec3& velocity) const { diff --git a/libraries/physics/src/ObjectMotionState.h b/libraries/physics/src/ObjectMotionState.h index 8f97b25dcc..2e30269efc 100644 --- a/libraries/physics/src/ObjectMotionState.h +++ b/libraries/physics/src/ObjectMotionState.h @@ -153,9 +153,6 @@ protected: void setMotionType(MotionType motionType); void updateCCDConfiguration(); - // clearObjectBackPointer() overrrides should call the base method, then actually clear the object back pointer. - virtual void clearObjectBackPointer() { _type = MOTIONSTATE_TYPE_INVALID; } - void setRigidBody(btRigidBody* body); MotionStateType _type = MOTIONSTATE_TYPE_INVALID; // type of MotionState diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index 010969d3c9..1281edb205 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -44,6 +44,7 @@ void PhysicalEntitySimulation::updateEntitiesInternal(const quint64& now) { void PhysicalEntitySimulation::addEntityInternal(EntityItemPointer entity) { assert(entity); + assert(!entity->isDead()); if (entity->shouldBePhysical()) { EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); if (!motionState) { @@ -60,8 +61,6 @@ void PhysicalEntitySimulation::removeEntityInternal(EntityItemPointer entity) { EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); if (motionState) { - motionState->clearObjectBackPointer(); - entity->setPhysicsInfo(nullptr); _outgoingChanges.remove(motionState); _entitiesToRemoveFromPhysics.insert(entity); } else { @@ -69,6 +68,23 @@ void PhysicalEntitySimulation::removeEntityInternal(EntityItemPointer entity) { } } +void PhysicalEntitySimulation::takeEntitiesToDelete(VectorOfEntities& entitiesToDelete) { + QMutexLocker lock(&_mutex); + for (auto entity : _entitiesToDelete) { + // this entity is still in its tree, so we insert into the external list + entitiesToDelete.push_back(entity); + + // Someday when we invert the entities/physics lib dependencies we can let EntityItem delete its own PhysicsInfo + // rather than do it here + EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); + if (motionState) { + delete motionState; + entity->setPhysicsInfo(nullptr); + } + } + _entitiesToDelete.clear(); +} + void PhysicalEntitySimulation::changeEntityInternal(EntityItemPointer entity) { // queue incoming changes: from external sources (script, EntityServer, etc) to physics engine assert(entity); @@ -106,17 +122,26 @@ void PhysicalEntitySimulation::clearEntitiesInternal() { // first disconnect each MotionStates from its Entity for (auto stateItr : _physicalObjects) { EntityMotionState* motionState = static_cast(&(*stateItr)); - EntityItemPointer entity = motionState->getEntity(); - if (entity) { - entity->setPhysicsInfo(nullptr); - } - motionState->clearObjectBackPointer(); + EntityItem* entity = motionState->getEntity(); + assert(entity); + _entitiesToDelete.insert(EntityItemPointer(entity)); } - // then delete the objects (aka MotionStates) - _physicsEngine->deleteObjects(_physicalObjects); + // then remove the objects from physics (aka MotionStates) + _physicsEngine->removeObjects(_physicalObjects); - // finally clear all lists (which now have only dangling pointers) + // 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 + for (auto entity : _entitiesToDelete) { + EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); + if (motionState) { + delete motionState; + entity->setPhysicsInfo(nullptr); + } + } + + // finally clear all lists maintained by this class _physicalObjects.clear(); _entitiesToRemoveFromPhysics.clear(); _entitiesToAddToPhysics.clear(); @@ -128,16 +153,13 @@ void PhysicalEntitySimulation::clearEntitiesInternal() { void PhysicalEntitySimulation::prepareEntityForDelete(EntityItemPointer entity) { assert(entity); assert(entity->isSimulated()); + assert(entity->isDead()); entity->clearActions(this); removeEntityInternal(entity); - - // the PhysicalEntitySimulation must pull the corresponding object out of the PhysicsEngine - // before the Entity is ready to delete so we first put them on this list - _entitiesToRemoveFromPhysics.insert(entity); } // end EntitySimulation overrides -void PhysicalEntitySimulation::getObjectsToDelete(VectorOfMotionStates& result) { +void PhysicalEntitySimulation::getObjectsToRemoveFromPhysics(VectorOfMotionStates& result) { result.clear(); QMutexLocker lock(&_mutex); for (auto entity: _entitiesToRemoveFromPhysics) { @@ -145,28 +167,30 @@ void PhysicalEntitySimulation::getObjectsToDelete(VectorOfMotionStates& result) if (motionState) { _pendingChanges.remove(motionState); _physicalObjects.remove(motionState); - motionState->clearObjectBackPointer(); result.push_back(motionState); } _entitiesToAddToPhysics.remove(entity); - entity->setPhysicsInfo(nullptr); - _entitiesToDelete.insert(entity); + if (entity->isDead()) { + _entitiesToDelete.insert(entity); + } } _entitiesToRemoveFromPhysics.clear(); } -void PhysicalEntitySimulation::getObjectsToAdd(VectorOfMotionStates& result) { +void PhysicalEntitySimulation::getObjectsToAddToPhysics(VectorOfMotionStates& result) { result.clear(); QMutexLocker lock(&_mutex); SetOfEntities::iterator entityItr = _entitiesToAddToPhysics.begin(); while (entityItr != _entitiesToAddToPhysics.end()) { - EntityItemPointer entity = *entityItr; + EntityItem* entity = (*entityItr).get(); assert(!entity->getPhysicsInfo()); - if (!entity->shouldBePhysical()) { + if (entity->isDead()) { + prepareEntityForDelete(EntityItemPointer(entity)); + } else if (!entity->shouldBePhysical()) { // this entity should no longer be on the internal _entitiesToAddToPhysics entityItr = _entitiesToAddToPhysics.erase(entityItr); if (entity->isMoving()) { - _simpleKinematicEntities.insert(entity); + _simpleKinematicEntities.insert(EntityItemPointer(entity)); } } else if (entity->isReadyToComputeShape()) { ShapeInfo shapeInfo; @@ -212,13 +236,12 @@ void PhysicalEntitySimulation::handleOutgoingChanges(const VectorOfMotionStates& ObjectMotionState* state = &(*stateItr); if (state && state->getType() == MOTIONSTATE_TYPE_ENTITY) { EntityMotionState* entityState = static_cast(state); - EntityItemPointer entity = entityState->getEntity(); - if (entity) { - if (entityState->isCandidateForOwnership(sessionID)) { - _outgoingChanges.insert(entityState); - } - _entitiesToSort.insert(entityState->getEntity()); + EntityItem* entity = entityState->getEntity(); + assert(entity); + if (entityState->isCandidateForOwnership(sessionID)) { + _outgoingChanges.insert(entityState); } + _entitiesToSort.insert(EntityItemPointer(entity)); } } diff --git a/libraries/physics/src/PhysicalEntitySimulation.h b/libraries/physics/src/PhysicalEntitySimulation.h index 6796c2ca90..bc80d50d0a 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.h +++ b/libraries/physics/src/PhysicalEntitySimulation.h @@ -35,6 +35,8 @@ public: virtual void addAction(EntityActionPointer action) override; virtual void applyActionChanges() override; + virtual void takeEntitiesToDelete(VectorOfEntities& entitiesToDelete) override; + protected: // only called by EntitySimulation // overrides for EntitySimulation virtual void updateEntitiesInternal(const quint64& now) override; @@ -46,8 +48,8 @@ protected: // only called by EntitySimulation public: virtual void prepareEntityForDelete(EntityItemPointer entity) override; - void getObjectsToDelete(VectorOfMotionStates& result); - void getObjectsToAdd(VectorOfMotionStates& result); + void getObjectsToRemoveFromPhysics(VectorOfMotionStates& result); + void getObjectsToAddToPhysics(VectorOfMotionStates& result); void setObjectsToChange(const VectorOfMotionStates& objectsToChange); void getObjectsToChange(VectorOfMotionStates& result); @@ -58,7 +60,7 @@ public: private: SetOfEntities _entitiesToRemoveFromPhysics; - SetOfEntities _entitiesToAddToPhysics; // entities to be be added to PhysicsEngine (and a their EntityMotionState created) + SetOfEntities _entitiesToAddToPhysics; SetOfEntityMotionStates _pendingChanges; // EntityMotionStates already in PhysicsEngine that need their physics changed SetOfEntityMotionStates _outgoingChanges; // EntityMotionStates for which we need to send updates to entity-server diff --git a/libraries/physics/src/PhysicsEngine.cpp b/libraries/physics/src/PhysicsEngine.cpp index 8a84509165..9e295d5cf5 100644 --- a/libraries/physics/src/PhysicsEngine.cpp +++ b/libraries/physics/src/PhysicsEngine.cpp @@ -156,7 +156,7 @@ void PhysicsEngine::removeObjectFromDynamicsWorld(ObjectMotionState* object) { _dynamicsWorld->removeRigidBody(body); } -void PhysicsEngine::deleteObjects(const VectorOfMotionStates& objects) { +void PhysicsEngine::removeObjects(const VectorOfMotionStates& objects) { for (auto object : objects) { removeObjectFromDynamicsWorld(object); @@ -165,14 +165,11 @@ void PhysicsEngine::deleteObjects(const VectorOfMotionStates& objects) { object->setRigidBody(nullptr); body->setMotionState(nullptr); delete body; - // adebug TODO: move this into ObjectMotionState dtor - object->releaseShape(); - delete object; } } // Same as above, but takes a Set instead of a Vector. Should only be called during teardown. -void PhysicsEngine::deleteObjects(const SetOfMotionStates& objects) { +void PhysicsEngine::removeObjects(const SetOfMotionStates& objects) { for (auto object : objects) { btRigidBody* body = object->getRigidBody(); removeObjectFromDynamicsWorld(object); @@ -181,9 +178,6 @@ void PhysicsEngine::deleteObjects(const SetOfMotionStates& objects) { object->setRigidBody(nullptr); body->setMotionState(nullptr); delete body; - // adebug TODO: move this into ObjectMotionState dtor - object->releaseShape(); - delete object; } } diff --git a/libraries/physics/src/PhysicsEngine.h b/libraries/physics/src/PhysicsEngine.h index b5ba36cb7c..1161b89f2c 100644 --- a/libraries/physics/src/PhysicsEngine.h +++ b/libraries/physics/src/PhysicsEngine.h @@ -54,8 +54,8 @@ public: void setSessionUUID(const QUuid& sessionID) { _sessionID = sessionID; } const QUuid& getSessionID() const { return _sessionID; } - void deleteObjects(const VectorOfMotionStates& objects); - void deleteObjects(const SetOfMotionStates& objects); // only called during teardown + void removeObjects(const VectorOfMotionStates& objects); + void removeObjects(const SetOfMotionStates& objects); // only called during teardown void addObjects(const VectorOfMotionStates& objects); VectorOfMotionStates changeObjects(const VectorOfMotionStates& objects); @@ -84,8 +84,6 @@ public: /// \brief call bump on any objects that touch the object corresponding to motionState void bump(ObjectMotionState* motionState); - void removeRigidBody(btRigidBody* body); - void setCharacterController(CharacterController* character); void dumpNextStats() { _dumpNextStats = true; } diff --git a/libraries/shared/src/SpatiallyNestable.h b/libraries/shared/src/SpatiallyNestable.h index cc67aedcd3..915d247b92 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() { } + virtual ~SpatiallyNestable() { assert(_isDead); } virtual const QUuid& getID() const { return _id; } virtual void setID(const QUuid& id) { _id = id; } @@ -115,6 +115,9 @@ public: void forEachChild(std::function actor); void forEachDescendant(std::function actor); + void die() { _isDead = true; } + bool isDead() const { return _isDead; } + protected: const NestableType _nestableType; // EntityItem or an AvatarData QUuid _id; @@ -141,7 +144,8 @@ protected: private: mutable ReadWriteLockable _transformLock; Transform _transform; // this is to be combined with parent's world-transform to produce this' world-transform. - mutable bool _parentKnowsMe = false; + mutable bool _parentKnowsMe { false }; + bool _isDead { false }; };