From 534bfa5983916519a1f763c26c0f19c02f99265c Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 28 Dec 2015 17:25:07 -0800 Subject: [PATCH 01/13] remove dangling whitespace --- interface/src/avatar/AvatarMotionState.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/interface/src/avatar/AvatarMotionState.cpp b/interface/src/avatar/AvatarMotionState.cpp index acd9a45aab..9acbe6c3d4 100644 --- a/interface/src/avatar/AvatarMotionState.cpp +++ b/interface/src/avatar/AvatarMotionState.cpp @@ -76,7 +76,7 @@ void AvatarMotionState::getWorldTransform(btTransform& worldTrans) const { } } -// virtual +// virtual void AvatarMotionState::setWorldTransform(const btTransform& worldTrans) { if (!_avatar) { return; @@ -154,12 +154,12 @@ QUuid AvatarMotionState::getSimulatorID() const { return _avatar->getSessionUUID(); } -// virtual +// virtual int16_t AvatarMotionState::computeCollisionGroup() { return COLLISION_GROUP_OTHER_AVATAR; } -// virtual +// virtual void AvatarMotionState::clearObjectBackPointer() { ObjectMotionState::clearObjectBackPointer(); _avatar = nullptr; From 4208380d1c8c889e515e875212d6e022195b603b Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 28 Dec 2015 17:25:19 -0800 Subject: [PATCH 02/13] namechange --- libraries/physics/src/ObjectMotionState.cpp | 1 + libraries/physics/src/PhysicsEngine.cpp | 18 +++++++++++------- libraries/physics/src/PhysicsEngine.h | 7 ++++--- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/libraries/physics/src/ObjectMotionState.cpp b/libraries/physics/src/ObjectMotionState.cpp index 7389d18143..4d7df9b43c 100644 --- a/libraries/physics/src/ObjectMotionState.cpp +++ b/libraries/physics/src/ObjectMotionState.cpp @@ -71,6 +71,7 @@ ObjectMotionState::ObjectMotionState(btCollisionShape* shape) : } ObjectMotionState::~ObjectMotionState() { + // adebug TODO: move shape release out of PhysicsEngine and into into the ObjectMotionState dtor assert(!_body); assert(!_shape); } diff --git a/libraries/physics/src/PhysicsEngine.cpp b/libraries/physics/src/PhysicsEngine.cpp index 22695a1b66..8a84509165 100644 --- a/libraries/physics/src/PhysicsEngine.cpp +++ b/libraries/physics/src/PhysicsEngine.cpp @@ -67,7 +67,8 @@ void PhysicsEngine::init() { } } -void PhysicsEngine::addObject(ObjectMotionState* motionState) { +// private +void PhysicsEngine::addObjectToDynamicsWorld(ObjectMotionState* motionState) { assert(motionState); btVector3 inertia(0.0f, 0.0f, 0.0f); @@ -144,7 +145,8 @@ void PhysicsEngine::addObject(ObjectMotionState* motionState) { motionState->clearIncomingDirtyFlags(); } -void PhysicsEngine::removeObject(ObjectMotionState* object) { +// private +void PhysicsEngine::removeObjectFromDynamicsWorld(ObjectMotionState* object) { // wake up anything touching this object bump(object); removeContacts(object); @@ -156,13 +158,14 @@ void PhysicsEngine::removeObject(ObjectMotionState* object) { void PhysicsEngine::deleteObjects(const VectorOfMotionStates& objects) { for (auto object : objects) { - removeObject(object); + removeObjectFromDynamicsWorld(object); // NOTE: setRigidBody() modifies body->m_userPointer so we should clear the MotionState's body BEFORE deleting it. btRigidBody* body = object->getRigidBody(); object->setRigidBody(nullptr); body->setMotionState(nullptr); delete body; + // adebug TODO: move this into ObjectMotionState dtor object->releaseShape(); delete object; } @@ -172,12 +175,13 @@ void PhysicsEngine::deleteObjects(const VectorOfMotionStates& objects) { void PhysicsEngine::deleteObjects(const SetOfMotionStates& objects) { for (auto object : objects) { btRigidBody* body = object->getRigidBody(); - removeObject(object); + removeObjectFromDynamicsWorld(object); // NOTE: setRigidBody() modifies body->m_userPointer so we should clear the MotionState's body BEFORE deleting it. object->setRigidBody(nullptr); body->setMotionState(nullptr); delete body; + // adebug TODO: move this into ObjectMotionState dtor object->releaseShape(); delete object; } @@ -185,7 +189,7 @@ void PhysicsEngine::deleteObjects(const SetOfMotionStates& objects) { void PhysicsEngine::addObjects(const VectorOfMotionStates& objects) { for (auto object : objects) { - addObject(object); + addObjectToDynamicsWorld(object); } } @@ -211,8 +215,8 @@ VectorOfMotionStates PhysicsEngine::changeObjects(const VectorOfMotionStates& ob } void PhysicsEngine::reinsertObject(ObjectMotionState* object) { - removeObject(object); - addObject(object); + removeObjectFromDynamicsWorld(object); + addObjectToDynamicsWorld(object); } void PhysicsEngine::removeContacts(ObjectMotionState* motionState) { diff --git a/libraries/physics/src/PhysicsEngine.h b/libraries/physics/src/PhysicsEngine.h index 05032ccae2..b5ba36cb7c 100644 --- a/libraries/physics/src/PhysicsEngine.h +++ b/libraries/physics/src/PhysicsEngine.h @@ -54,11 +54,9 @@ public: void setSessionUUID(const QUuid& sessionID) { _sessionID = sessionID; } const QUuid& getSessionID() const { return _sessionID; } - void addObject(ObjectMotionState* motionState); - void removeObject(ObjectMotionState* motionState); - void deleteObjects(const VectorOfMotionStates& objects); void deleteObjects(const SetOfMotionStates& objects); // only called during teardown + void addObjects(const VectorOfMotionStates& objects); VectorOfMotionStates changeObjects(const VectorOfMotionStates& objects); void reinsertObject(ObjectMotionState* object); @@ -100,6 +98,9 @@ public: void forEachAction(std::function actor); private: + void addObjectToDynamicsWorld(ObjectMotionState* motionState); + void removeObjectFromDynamicsWorld(ObjectMotionState* motionState); + void removeContacts(ObjectMotionState* motionState); void doOwnershipInfection(const btCollisionObject* objectA, const btCollisionObject* objectB); From b34df211dfb6c1cfbfbad0180c4cb7756e89b2db Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 29 Dec 2015 09:36:10 -0800 Subject: [PATCH 03/13] cleanup API for removal from EntitySimulation --- libraries/entities/src/EntitySimulation.cpp | 67 +++++++------------ libraries/entities/src/EntitySimulation.h | 13 ++-- libraries/entities/src/EntityTree.cpp | 3 +- .../entities/src/SimpleEntitySimulation.cpp | 1 + .../entities/src/SimpleEntitySimulation.h | 10 +-- .../physics/src/PhysicalEntitySimulation.cpp | 1 + 6 files changed, 37 insertions(+), 58 deletions(-) diff --git a/libraries/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp index ad354465d0..324a7a25dd 100644 --- a/libraries/entities/src/EntitySimulation.cpp +++ b/libraries/entities/src/EntitySimulation.cpp @@ -47,6 +47,24 @@ void EntitySimulation::getEntitiesToDelete(VectorOfEntities& entitiesToDelete) { _entitiesToDelete.clear(); } +void EntitySimulation::removeEntityInternal(EntityItemPointer entity) { + _mortalEntities.remove(entity); + _entitiesToUpdate.remove(entity); + _entitiesToSort.remove(entity); + _simpleKinematicEntities.remove(entity); + _allEntities.remove(entity); + entity->_simulated = false; +} + +void EntitySimulation::prepareEntityForDelete(EntityItemPointer entity) { + assert(entity); + if (entity->_simulated) { + entity->clearActions(this); + _entitiesToDelete.insert(entity); + removeEntityInternal(entity); + } +} + void EntitySimulation::addEntityInternal(EntityItemPointer entity) { if (entity->isMoving() && !entity->getPhysicsInfo()) { _simpleKinematicEntities.insert(entity); @@ -71,15 +89,8 @@ void EntitySimulation::expireMortalEntities(const quint64& now) { EntityItemPointer entity = *itemItr; quint64 expiry = entity->getExpiry(); if (expiry < now) { - _entitiesToDelete.insert(entity); itemItr = _mortalEntities.erase(itemItr); - _entitiesToUpdate.remove(entity); - _entitiesToSort.remove(entity); - _simpleKinematicEntities.remove(entity); - removeEntityInternal(entity); - - _allEntities.remove(entity); - entity->_simulated = false; + prepareEntityForDelete(entity); } else { if (expiry < _nextExpiry) { // remeber the smallest _nextExpiry so we know when to start the next search @@ -97,7 +108,7 @@ void EntitySimulation::callUpdateOnEntitiesThatNeedIt(const quint64& now) { SetOfEntities::iterator itemItr = _entitiesToUpdate.begin(); while (itemItr != _entitiesToUpdate.end()) { EntityItemPointer entity = *itemItr; - // TODO: catch transition from needing update to not as a "change" + // TODO: catch transition from needing update to not as a "change" // so we don't have to scan for it here. if (!entity->needsToCallUpdate()) { itemItr = _entitiesToUpdate.erase(itemItr); @@ -123,16 +134,8 @@ void EntitySimulation::sortEntitiesThatMoved() { AACube newCube = entity->getQueryAACube(success); if (success && !domainBounds.touches(newCube)) { qCDebug(entities) << "Entity " << entity->getEntityItemID() << " moved out of domain bounds."; - _entitiesToDelete.insert(entity); - _mortalEntities.remove(entity); - _entitiesToUpdate.remove(entity); - _simpleKinematicEntities.remove(entity); - removeEntityInternal(entity); - - _allEntities.remove(entity); - entity->_simulated = false; - itemItr = _entitiesToSort.erase(itemItr); + prepareEntityForDelete(entity); } else { moveOperator.addEntityToMoveList(entity, newCube); ++itemItr; @@ -165,37 +168,23 @@ void EntitySimulation::addEntity(EntityItemPointer entity) { _allEntities.insert(entity); entity->_simulated = true; - // DirtyFlags are used to signal changes to entities that have already been added, + // DirtyFlags are used to signal changes to entities that have already been added, // so we can clear them for this entity which has just been added. entity->clearDirtyFlags(); } -void EntitySimulation::removeEntity(EntityItemPointer entity) { - QMutexLocker lock(&_mutex); - assert(entity); - _entitiesToUpdate.remove(entity); - _mortalEntities.remove(entity); - _entitiesToSort.remove(entity); - _simpleKinematicEntities.remove(entity); - _entitiesToDelete.remove(entity); - removeEntityInternal(entity); - - _allEntities.remove(entity); - entity->_simulated = false; -} - void EntitySimulation::changeEntity(EntityItemPointer entity) { QMutexLocker lock(&_mutex); assert(entity); if (!entity->_simulated) { // This entity was either never added to the simulation or has been removed - // (probably for pending delete), so we don't want to keep a pointer to it + // (probably for pending delete), so we don't want to keep a pointer to it // on any internal lists. return; } // Although it is not the responsibility of the EntitySimulation to sort the tree for EXTERNAL changes - // it IS responsibile for triggering deletes for entities that leave the bounds of the domain, hence + // it IS responsibile for triggering deletes for entities that leave the bounds of the domain, hence // we must check for that case here, however we rely on the change event to have set DIRTY_POSITION flag. bool wasRemoved = false; uint32_t dirtyFlags = entity->getDirtyFlags(); @@ -205,13 +194,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."; - _entitiesToDelete.insert(entity); - _mortalEntities.remove(entity); - _entitiesToUpdate.remove(entity); - _entitiesToSort.remove(entity); - _simpleKinematicEntities.remove(entity); - removeEntityInternal(entity); - entity->_simulated = false; + prepareEntityForDelete(entity); wasRemoved = true; } } diff --git a/libraries/entities/src/EntitySimulation.h b/libraries/entities/src/EntitySimulation.h index 4519effbd3..7ed9a94d4b 100644 --- a/libraries/entities/src/EntitySimulation.h +++ b/libraries/entities/src/EntitySimulation.h @@ -63,26 +63,21 @@ public: /// \sideeffect sets relevant backpointers in entity, but maybe later when appropriate data structures are locked void addEntity(EntityItemPointer entity); - /// \param entity pointer to EntityItem to be removed - /// \brief the actual removal may happen later when appropriate data structures are locked - /// \sideeffect nulls relevant backpointers in entity - void removeEntity(EntityItemPointer entity); - - /// \param entity pointer to EntityItem to that may have changed in a way that would affect its simulation + /// \param entity pointer to EntityItem that may have changed in a way that would affect its simulation /// call this whenever an entity was changed from some EXTERNAL event (NOT by the EntitySimulation itself) void changeEntity(EntityItemPointer entity); void clearEntities(); void moveSimpleKinematics(const quint64& now); -protected: // these only called by the EntityTree? - -public: EntityTreePointer getEntityTree() { return _entityTree; } void getEntitiesToDelete(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); + signals: void entityCollisionWithEntity(const EntityItemID& idA, const EntityItemID& idB, const Collision& collision); diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index ca7b892523..af6e1f5ef1 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -453,8 +453,7 @@ void EntityTree::processRemovedEntities(const DeleteEntityOperator& theOperator) } if (_simulation) { - theEntity->clearActions(_simulation); - _simulation->removeEntity(theEntity); + _simulation->prepareEntityForDelete(theEntity); } } } diff --git a/libraries/entities/src/SimpleEntitySimulation.cpp b/libraries/entities/src/SimpleEntitySimulation.cpp index 81d9445f92..3b2523bc92 100644 --- a/libraries/entities/src/SimpleEntitySimulation.cpp +++ b/libraries/entities/src/SimpleEntitySimulation.cpp @@ -53,6 +53,7 @@ void SimpleEntitySimulation::addEntityInternal(EntityItemPointer entity) { } void SimpleEntitySimulation::removeEntityInternal(EntityItemPointer entity) { + EntitySimulation::removeEntityInternal(entity); _entitiesWithSimulator.remove(entity); } diff --git a/libraries/entities/src/SimpleEntitySimulation.h b/libraries/entities/src/SimpleEntitySimulation.h index fff0659067..83c51525a8 100644 --- a/libraries/entities/src/SimpleEntitySimulation.h +++ b/libraries/entities/src/SimpleEntitySimulation.h @@ -22,11 +22,11 @@ public: virtual ~SimpleEntitySimulation() { clearEntitiesInternal(); } protected: - virtual void updateEntitiesInternal(const quint64& now); - virtual void addEntityInternal(EntityItemPointer entity); - virtual void removeEntityInternal(EntityItemPointer entity); - virtual void changeEntityInternal(EntityItemPointer entity); - virtual void clearEntitiesInternal(); + virtual void updateEntitiesInternal(const quint64& now) override; + virtual void addEntityInternal(EntityItemPointer entity) override; + virtual void removeEntityInternal(EntityItemPointer entity) override; + virtual void changeEntityInternal(EntityItemPointer entity) override; + virtual void clearEntitiesInternal() override; SetOfEntities _entitiesWithSimulator; }; diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index 5a12627abd..f050894cfb 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -55,6 +55,7 @@ void PhysicalEntitySimulation::addEntityInternal(EntityItemPointer entity) { } void PhysicalEntitySimulation::removeEntityInternal(EntityItemPointer entity) { + EntitySimulation::removeEntityInternal(entity); EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); if (motionState) { motionState->clearObjectBackPointer(); From f5d24a87ca47118beda69356e1f2872c99c9460b Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 29 Dec 2015 11:05:02 -0800 Subject: [PATCH 04/13] remove entities from physics before we delete them --- libraries/entities/src/EntityItem.h | 4 ++ libraries/entities/src/EntitySimulation.cpp | 17 +++--- libraries/entities/src/EntitySimulation.h | 4 +- .../physics/src/PhysicalEntitySimulation.cpp | 61 +++++++++++-------- .../physics/src/PhysicalEntitySimulation.h | 10 +-- 5 files changed, 58 insertions(+), 38 deletions(-) diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index 79cecfc1e5..4d7106b858 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -337,6 +337,8 @@ public: bool isMoving() const; + bool isSimulated() const { return _simulated; } + void* getPhysicsInfo() const { return _physicsInfo; } void setPhysicsInfo(void* data) { _physicsInfo = data; } @@ -391,6 +393,8 @@ public: protected: + void setSimulated(bool simulated) { _simulated = simulated; } + const QByteArray getActionDataInternal() const; void setActionDataInternal(QByteArray actionData); diff --git a/libraries/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp index 324a7a25dd..62912b8954 100644 --- a/libraries/entities/src/EntitySimulation.cpp +++ b/libraries/entities/src/EntitySimulation.cpp @@ -53,16 +53,15 @@ void EntitySimulation::removeEntityInternal(EntityItemPointer entity) { _entitiesToSort.remove(entity); _simpleKinematicEntities.remove(entity); _allEntities.remove(entity); - entity->_simulated = false; + entity->setSimulated(false); } void EntitySimulation::prepareEntityForDelete(EntityItemPointer entity) { assert(entity); - if (entity->_simulated) { - entity->clearActions(this); - _entitiesToDelete.insert(entity); - removeEntityInternal(entity); - } + assert(entity->isSimulated()); + entity->clearActions(this); + removeEntityInternal(entity); + _entitiesToDelete.insert(entity); } void EntitySimulation::addEntityInternal(EntityItemPointer entity) { @@ -166,7 +165,7 @@ void EntitySimulation::addEntity(EntityItemPointer entity) { addEntityInternal(entity); _allEntities.insert(entity); - entity->_simulated = true; + entity->setSimulated(true); // DirtyFlags are used to signal changes to entities that have already been added, // so we can clear them for this entity which has just been added. @@ -176,7 +175,7 @@ void EntitySimulation::addEntity(EntityItemPointer entity) { void EntitySimulation::changeEntity(EntityItemPointer entity) { QMutexLocker lock(&_mutex); assert(entity); - if (!entity->_simulated) { + if (!entity->isSimulated()) { // This entity was either never added to the simulation or has been removed // (probably for pending delete), so we don't want to keep a pointer to it // on any internal lists. @@ -232,7 +231,7 @@ void EntitySimulation::clearEntities() { clearEntitiesInternal(); for (auto entityItr : _allEntities) { - entityItr->_simulated = false; + entityItr->setSimulated(false); } _allEntities.clear(); } diff --git a/libraries/entities/src/EntitySimulation.h b/libraries/entities/src/EntitySimulation.h index 7ed9a94d4b..f1548b50e9 100644 --- a/libraries/entities/src/EntitySimulation.h +++ b/libraries/entities/src/EntitySimulation.h @@ -101,6 +101,9 @@ protected: QList _actionsToAdd; QSet _actionsToRemove; +protected: + SetOfEntities _entitiesToDelete; // entities simulation decided needed to be deleted (EntityTree will actually delete) + private: void moveSimpleKinematics(); @@ -115,7 +118,6 @@ private: SetOfEntities _entitiesToUpdate; // entities that need to call EntityItem::update() - SetOfEntities _entitiesToDelete; // entities simulation decided needed to be deleted (EntityTree will actually delete) }; diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index f050894cfb..010969d3c9 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -47,7 +47,7 @@ void PhysicalEntitySimulation::addEntityInternal(EntityItemPointer entity) { if (entity->shouldBePhysical()) { EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); if (!motionState) { - _pendingAdds.insert(entity); + _entitiesToAddToPhysics.insert(entity); } } else if (entity->isMoving()) { _simpleKinematicEntities.insert(entity); @@ -56,14 +56,17 @@ void PhysicalEntitySimulation::addEntityInternal(EntityItemPointer entity) { void PhysicalEntitySimulation::removeEntityInternal(EntityItemPointer entity) { EntitySimulation::removeEntityInternal(entity); + _entitiesToAddToPhysics.remove(entity); + EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); if (motionState) { motionState->clearObjectBackPointer(); entity->setPhysicsInfo(nullptr); - _pendingRemoves.insert(motionState); _outgoingChanges.remove(motionState); + _entitiesToRemoveFromPhysics.insert(entity); + } else { + _entitiesToDelete.insert(entity); } - _pendingAdds.remove(entity); } void PhysicalEntitySimulation::changeEntityInternal(EntityItemPointer entity) { @@ -75,8 +78,8 @@ void PhysicalEntitySimulation::changeEntityInternal(EntityItemPointer entity) { // the entity should be removed from the physical simulation _pendingChanges.remove(motionState); _physicalObjects.remove(motionState); - _pendingRemoves.insert(motionState); _outgoingChanges.remove(motionState); + _entitiesToRemoveFromPhysics.insert(entity); if (entity->isMoving()) { _simpleKinematicEntities.insert(entity); } @@ -86,7 +89,7 @@ void PhysicalEntitySimulation::changeEntityInternal(EntityItemPointer entity) { } else if (entity->shouldBePhysical()) { // The intent is for this object to be in the PhysicsEngine, but it has no MotionState yet. // Perhaps it's shape has changed and it can now be added? - _pendingAdds.insert(entity); + _entitiesToAddToPhysics.insert(entity); _simpleKinematicEntities.remove(entity); // just in case it's non-physical-kinematic } else if (entity->isMoving()) { _simpleKinematicEntities.insert(entity); @@ -115,43 +118,53 @@ void PhysicalEntitySimulation::clearEntitiesInternal() { // finally clear all lists (which now have only dangling pointers) _physicalObjects.clear(); - _pendingRemoves.clear(); - _pendingAdds.clear(); + _entitiesToRemoveFromPhysics.clear(); + _entitiesToAddToPhysics.clear(); _pendingChanges.clear(); _outgoingChanges.clear(); } -// end EntitySimulation overrides +// virtual +void PhysicalEntitySimulation::prepareEntityForDelete(EntityItemPointer entity) { + assert(entity); + assert(entity->isSimulated()); + 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) { result.clear(); QMutexLocker lock(&_mutex); - for (auto stateItr : _pendingRemoves) { - EntityMotionState* motionState = &(*stateItr); - _pendingChanges.remove(motionState); - _physicalObjects.remove(motionState); - - EntityItemPointer entity = motionState->getEntity(); - if (entity) { - _pendingAdds.remove(entity); - entity->setPhysicsInfo(nullptr); + for (auto entity: _entitiesToRemoveFromPhysics) { + EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); + if (motionState) { + _pendingChanges.remove(motionState); + _physicalObjects.remove(motionState); motionState->clearObjectBackPointer(); + result.push_back(motionState); } - result.push_back(motionState); + _entitiesToAddToPhysics.remove(entity); + entity->setPhysicsInfo(nullptr); + _entitiesToDelete.insert(entity); } - _pendingRemoves.clear(); + _entitiesToRemoveFromPhysics.clear(); } void PhysicalEntitySimulation::getObjectsToAdd(VectorOfMotionStates& result) { result.clear(); QMutexLocker lock(&_mutex); - SetOfEntities::iterator entityItr = _pendingAdds.begin(); - while (entityItr != _pendingAdds.end()) { + SetOfEntities::iterator entityItr = _entitiesToAddToPhysics.begin(); + while (entityItr != _entitiesToAddToPhysics.end()) { EntityItemPointer entity = *entityItr; assert(!entity->getPhysicsInfo()); if (!entity->shouldBePhysical()) { - // this entity should no longer be on the internal _pendingAdds - entityItr = _pendingAdds.erase(entityItr); + // this entity should no longer be on the internal _entitiesToAddToPhysics + entityItr = _entitiesToAddToPhysics.erase(entityItr); if (entity->isMoving()) { _simpleKinematicEntities.insert(entity); } @@ -164,7 +177,7 @@ void PhysicalEntitySimulation::getObjectsToAdd(VectorOfMotionStates& result) { entity->setPhysicsInfo(static_cast(motionState)); _physicalObjects.insert(motionState); result.push_back(motionState); - entityItr = _pendingAdds.erase(entityItr); + entityItr = _entitiesToAddToPhysics.erase(entityItr); } else { //qDebug() << "Warning! Failed to generate new shape for entity." << entity->getName(); ++entityItr; diff --git a/libraries/physics/src/PhysicalEntitySimulation.h b/libraries/physics/src/PhysicalEntitySimulation.h index c4f96e023a..ad921bd9a1 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.h +++ b/libraries/physics/src/PhysicalEntitySimulation.h @@ -44,6 +44,8 @@ protected: // only called by EntitySimulation virtual void clearEntitiesInternal() override; public: + virtual void prepareEntityForDelete(EntityItemPointer entity) override; + void getObjectsToDelete(VectorOfMotionStates& result); void getObjectsToAdd(VectorOfMotionStates& result); void setObjectsToChange(const VectorOfMotionStates& objectsToChange); @@ -55,12 +57,12 @@ public: EntityEditPacketSender* getPacketSender() { return _entityPacketSender; } private: - // incoming changes - SetOfEntityMotionStates _pendingRemoves; // EntityMotionStates to be removed from PhysicsEngine (and deleted) - SetOfEntities _pendingAdds; // entities to be be added to PhysicsEngine (and a their EntityMotionState created) + // incoming changes to physics simulation + SetOfEntities _entitiesToRemoveFromPhysics; + SetOfEntities _entitiesToAddToPhysics; // entities to be be added to PhysicsEngine (and a their EntityMotionState created) SetOfEntityMotionStates _pendingChanges; // EntityMotionStates already in PhysicsEngine that need their physics changed - // outgoing changes + // outgoing changes from physics simulation SetOfEntityMotionStates _outgoingChanges; // EntityMotionStates for which we need to send updates to entity-server SetOfMotionStates _physicalObjects; // MotionStates of entities in PhysicsEngine From 8886f48c78bf5cc05ccf8268a8143d9f79d45645 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 29 Dec 2015 11:09:49 -0800 Subject: [PATCH 05/13] minor cleanup --- libraries/physics/src/PhysicalEntitySimulation.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libraries/physics/src/PhysicalEntitySimulation.h b/libraries/physics/src/PhysicalEntitySimulation.h index ad921bd9a1..6796c2ca90 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.h +++ b/libraries/physics/src/PhysicalEntitySimulation.h @@ -57,12 +57,10 @@ public: EntityEditPacketSender* getPacketSender() { return _entityPacketSender; } private: - // incoming changes to physics simulation SetOfEntities _entitiesToRemoveFromPhysics; SetOfEntities _entitiesToAddToPhysics; // entities to be be added to PhysicsEngine (and a their EntityMotionState created) - SetOfEntityMotionStates _pendingChanges; // EntityMotionStates already in PhysicsEngine that need their physics changed - // outgoing changes from physics simulation + SetOfEntityMotionStates _pendingChanges; // EntityMotionStates already in PhysicsEngine that need their physics changed SetOfEntityMotionStates _outgoingChanges; // EntityMotionStates for which we need to send updates to entity-server SetOfMotionStates _physicalObjects; // MotionStates of entities in PhysicsEngine From 6eb177091bd79aaaccf08cb0d3d95fa8dfd6ac4b Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 30 Dec 2015 15:20:53 -0800 Subject: [PATCH 06/13] overhaul of MotionState cleanup Moved MotionState deletes out of PhysicsEngine. EntityMotionStates are deleted by the PhysicsEntitySimulation. AvatarMotionStates are deleted in the Avatar dtor. --- interface/src/Application.cpp | 18 ++--- interface/src/avatar/Avatar.cpp | 8 +- interface/src/avatar/AvatarManager.cpp | 49 ++++++------ interface/src/avatar/AvatarManager.h | 11 ++- interface/src/avatar/AvatarMotionState.cpp | 31 ++------ interface/src/avatar/AvatarMotionState.h | 3 +- libraries/avatars/src/AvatarData.h | 5 -- .../src/RenderablePolyVoxEntityItem.h | 2 +- libraries/entities/src/BoxEntityItem.h | 2 +- libraries/entities/src/EntityItem.h | 2 +- libraries/entities/src/EntitySimulation.cpp | 16 ++-- libraries/entities/src/EntitySimulation.h | 2 +- libraries/entities/src/EntityTree.cpp | 3 +- libraries/entities/src/ModelEntityItem.cpp | 2 +- libraries/entities/src/SphereEntityItem.h | 2 +- libraries/entities/src/ZoneEntityItem.h | 2 +- libraries/physics/src/EntityMotionState.cpp | 58 ++++---------- libraries/physics/src/EntityMotionState.h | 7 +- libraries/physics/src/ObjectMotionState.cpp | 4 +- libraries/physics/src/ObjectMotionState.h | 3 - .../physics/src/PhysicalEntitySimulation.cpp | 79 ++++++++++++------- .../physics/src/PhysicalEntitySimulation.h | 8 +- libraries/physics/src/PhysicsEngine.cpp | 10 +-- libraries/physics/src/PhysicsEngine.h | 6 +- libraries/shared/src/SpatiallyNestable.h | 8 +- 25 files changed, 159 insertions(+), 182 deletions(-) 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 }; }; From e8e965d54810f6def87f8842d587b2e2fecc328f Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 31 Dec 2015 13:58:21 -0800 Subject: [PATCH 07/13] fix crash mode for empty LineEntityItem --- .../src/RenderablePolyLineEntityItem.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/libraries/entities-renderer/src/RenderablePolyLineEntityItem.cpp b/libraries/entities-renderer/src/RenderablePolyLineEntityItem.cpp index 7b3bbc4c02..fd28bc043d 100644 --- a/libraries/entities-renderer/src/RenderablePolyLineEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderablePolyLineEntityItem.cpp @@ -122,13 +122,19 @@ void RenderablePolyLineEntityItem::updateVertices() { glm::vec3 v1, v2, tangent, binormal, point; int finalIndex = minVectorSize - 1; + + // Guard against an empty polyline + if (finalIndex < 0) { + return; + } + for (int i = 0; i < finalIndex; i++) { float width = _strokeWidths.at(i); point = _points.at(i); tangent = _points.at(i); - tangent = _points.at(i + 1) - point; + tangent = _points.at(i + 1) - point; glm::vec3 normal = _normals.at(i); binormal = glm::normalize(glm::cross(tangent, normal)) * width; @@ -141,11 +147,6 @@ void RenderablePolyLineEntityItem::updateVertices() { _vertices << v1 << v2; } - // Guard against an empty polyline - if (finalIndex < 0) { - return; - } - // For last point we can assume binormals are the same since it represents the last two vertices of quad point = _points.at(finalIndex); v1 = point + binormal; From 6a642bdcf85a6a9c90c096e98def1cc049caff7d Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 31 Dec 2015 13:59:47 -0800 Subject: [PATCH 08/13] fix misusage of smart-pointers --- interface/src/avatar/Avatar.cpp | 3 ++- interface/src/avatar/AvatarManager.cpp | 4 ++++ interface/src/avatar/AvatarManager.h | 2 ++ libraries/entities/src/EntityItem.cpp | 1 + libraries/physics/src/EntityMotionState.cpp | 5 ++-- libraries/physics/src/EntityMotionState.h | 16 +++++++++---- .../physics/src/PhysicalEntitySimulation.cpp | 24 +++++++++---------- libraries/shared/src/SpatiallyNestable.h | 2 +- 8 files changed, 36 insertions(+), 21 deletions(-) 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; } From e0ec9414764816e91f8dcf2c3a4e66f9b4e6bf86 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 31 Dec 2015 15:01:32 -0800 Subject: [PATCH 09/13] fix typo --- interface/src/avatar/Avatar.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index 5d42d8a7bd..abe2f0c670 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -1,4 +1,4 @@ -/3/ +// // Avatar.cpp // interface/src/avatar // From 4f29156da1bd73677ef5523e55513a96cca50630 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 4 Jan 2016 12:02:11 -0800 Subject: [PATCH 10/13] fixing asserts for EntityItem delete pipeline --- libraries/entities/src/EntityItem.cpp | 1 - libraries/entities/src/EntitySimulation.cpp | 9 +++++---- libraries/physics/src/PhysicalEntitySimulation.cpp | 1 - 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index fb773ee89b..97043a635d 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -85,7 +85,6 @@ 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/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp index 35d5b18fda..ab29cfb2a4 100644 --- a/libraries/entities/src/EntitySimulation.cpp +++ b/libraries/entities/src/EntitySimulation.cpp @@ -59,11 +59,12 @@ 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); + if (entity->isSimulated()) { + entity->clearActions(this); + removeEntityInternal(entity); + _entitiesToDelete.insert(entity); + } } void EntitySimulation::addEntityInternal(EntityItemPointer entity) { diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index 4116ddeda0..9ef27aaf53 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -150,7 +150,6 @@ void PhysicalEntitySimulation::clearEntitiesInternal() { // virtual void PhysicalEntitySimulation::prepareEntityForDelete(EntityItemPointer entity) { assert(entity); - assert(entity->isSimulated()); assert(entity->isDead()); entity->clearActions(this); removeEntityInternal(entity); From c1206ca73d3d02a3097c44b42480809e8fe13670 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 4 Jan 2016 14:49:20 -0800 Subject: [PATCH 11/13] fix bad merge --- interface/src/avatar/Avatar.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/interface/src/avatar/Avatar.cpp b/interface/src/avatar/Avatar.cpp index abe2f0c670..c1eb173629 100644 --- a/interface/src/avatar/Avatar.cpp +++ b/interface/src/avatar/Avatar.cpp @@ -109,9 +109,6 @@ Avatar::Avatar(RigPointer rig) : Avatar::~Avatar() { assert(isDead()); // mark dead before calling the dtor - for(auto attachment : _unusedAttachments) { - delete attachment; - } if (_motionState) { delete _motionState; _motionState = nullptr; From 36e19b43c90dd636ec6f394814ab527657804ac8 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 6 Jan 2016 15:04:47 -0800 Subject: [PATCH 12/13] add some comments; specify const and overrides --- interface/src/avatar/AvatarManager.cpp | 2 + interface/src/avatar/AvatarMotionState.cpp | 2 +- interface/src/avatar/AvatarMotionState.h | 55 ++++++++++++--------- libraries/physics/src/EntityMotionState.cpp | 6 +-- libraries/physics/src/EntityMotionState.h | 38 +++++++------- libraries/physics/src/ObjectMotionState.h | 6 +-- 6 files changed, 61 insertions(+), 48 deletions(-) diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 6993fb6ae1..833ed26cc9 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -215,6 +215,8 @@ void AvatarManager::removeAvatar(const QUuid& sessionUUID) { void AvatarManager::handleRemovedAvatar(const AvatarSharedPointer& removedAvatar) { AvatarHashMap::handleRemovedAvatar(removedAvatar); + // removedAvatar is a shared pointer to an AvatarData but we need to get to the derived Avatar + // class in this context so we can call methods that don't exist at the base class. Avatar* avatar = static_cast(removedAvatar.get()); avatar->die(); diff --git a/interface/src/avatar/AvatarMotionState.cpp b/interface/src/avatar/AvatarMotionState.cpp index f61533a5e3..9ce9594d45 100644 --- a/interface/src/avatar/AvatarMotionState.cpp +++ b/interface/src/avatar/AvatarMotionState.cpp @@ -143,7 +143,7 @@ QUuid AvatarMotionState::getSimulatorID() const { } // virtual -int16_t AvatarMotionState::computeCollisionGroup() { +int16_t AvatarMotionState::computeCollisionGroup() const { return COLLISION_GROUP_OTHER_AVATAR; } diff --git a/interface/src/avatar/AvatarMotionState.h b/interface/src/avatar/AvatarMotionState.h index 8f60c8e607..0465ddf50b 100644 --- a/interface/src/avatar/AvatarMotionState.h +++ b/interface/src/avatar/AvatarMotionState.h @@ -21,54 +21,65 @@ class Avatar; class AvatarMotionState : public ObjectMotionState { public: AvatarMotionState(Avatar* avatar, btCollisionShape* shape); - ~AvatarMotionState(); - virtual MotionType getMotionType() const { return _motionType; } + virtual MotionType getMotionType() const override { return _motionType; } - virtual uint32_t getIncomingDirtyFlags(); - virtual void clearIncomingDirtyFlags(); + virtual uint32_t getIncomingDirtyFlags() override; + virtual void clearIncomingDirtyFlags() override; - virtual MotionType computeObjectMotionType() const; + virtual MotionType computeObjectMotionType() const override; - virtual bool isMoving() const; + virtual bool isMoving() const override; // this relays incoming position/rotation to the RigidBody - virtual void getWorldTransform(btTransform& worldTrans) const; + virtual void getWorldTransform(btTransform& worldTrans) const override; // this relays outgoing position/rotation to the EntityItem - virtual void setWorldTransform(const btTransform& worldTrans); + virtual void setWorldTransform(const btTransform& worldTrans) override; // These pure virtual methods must be implemented for each MotionState type // and make it possible to implement more complicated methods in this base class. - virtual float getObjectRestitution() const; - virtual float getObjectFriction() const; - virtual float getObjectLinearDamping() const; - virtual float getObjectAngularDamping() const; + // pure virtual overrides from ObjectMotionState + virtual float getObjectRestitution() const override; + virtual float getObjectFriction() const override; + virtual float getObjectLinearDamping() const override; + virtual float getObjectAngularDamping() const override; - virtual glm::vec3 getObjectPosition() const; - virtual glm::quat getObjectRotation() const; - virtual glm::vec3 getObjectLinearVelocity() const; - virtual glm::vec3 getObjectAngularVelocity() const; - virtual glm::vec3 getObjectGravity() const; + virtual glm::vec3 getObjectPosition() const override; + virtual glm::quat getObjectRotation() const override; + virtual glm::vec3 getObjectLinearVelocity() const override; + virtual glm::vec3 getObjectAngularVelocity() const override; + virtual glm::vec3 getObjectGravity() const override; - virtual const QUuid& getObjectID() const; + virtual const QUuid& getObjectID() const override; - virtual QUuid getSimulatorID() const; + virtual QUuid getSimulatorID() const override; void setBoundingBox(const glm::vec3& corner, const glm::vec3& diagonal); void addDirtyFlags(uint32_t flags) { _dirtyFlags |= flags; } - virtual int16_t computeCollisionGroup(); + virtual int16_t computeCollisionGroup() const override; friend class AvatarManager; + friend class Avatar; protected: - virtual bool isReadyToComputeShape() { return true; } + // the dtor had been made protected to force the compiler to verify that it is only + // ever called by the Avatar class dtor. + ~AvatarMotionState(); + + virtual bool isReadyToComputeShape() const override { return true; } virtual btCollisionShape* computeNewShape(); - Avatar* _avatar; // do NOT use smartpointer here + + // The AvatarMotionState keeps a RAW backpointer to its Avatar because all AvatarMotionState + // instances are "owned" by their corresponding Avatar instance and are deleted in the Avatar dtor. + // In other words, it is impossible for the Avatar to be deleted out from under its MotionState. + // In conclusion: weak pointer shennanigans would be pure overhead. + Avatar* _avatar; // do NOT use smartpointer here, no need for weakpointer + uint32_t _dirtyFlags; }; diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index b543d9d75b..c1338b772c 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -217,7 +217,7 @@ void EntityMotionState::setWorldTransform(const btTransform& worldTrans) { // virtual and protected -bool EntityMotionState::isReadyToComputeShape() { +bool EntityMotionState::isReadyToComputeShape() const { return _entity->isReadyToComputeShape(); } @@ -604,13 +604,13 @@ void EntityMotionState::setMotionType(MotionType motionType) { // virtual -QString EntityMotionState::getName() { +QString EntityMotionState::getName() const { assert(entityTreeIsLocked()); return _entity->getName(); } // virtual -int16_t EntityMotionState::computeCollisionGroup() { +int16_t EntityMotionState::computeCollisionGroup() const { if (_entity->getIgnoreForCollisions()) { return COLLISION_GROUP_COLLISIONLESS; } diff --git a/libraries/physics/src/EntityMotionState.h b/libraries/physics/src/EntityMotionState.h index da78513b58..53e7982ae1 100644 --- a/libraries/physics/src/EntityMotionState.h +++ b/libraries/physics/src/EntityMotionState.h @@ -38,10 +38,10 @@ public: virtual bool isMoving() const; // this relays incoming position/rotation to the RigidBody - virtual void getWorldTransform(btTransform& worldTrans) const; + virtual void getWorldTransform(btTransform& worldTrans) const override; // this relays outgoing position/rotation to the EntityItem - virtual void setWorldTransform(const btTransform& worldTrans); + virtual void setWorldTransform(const btTransform& worldTrans) override; bool isCandidateForOwnership(const QUuid& sessionID) const; bool remoteSimulationOutOfSync(uint32_t simulationStep); @@ -55,32 +55,32 @@ public: void resetAccelerationNearlyGravityCount() { _accelerationNearlyGravityCount = 0; } quint8 getAccelerationNearlyGravityCount() { return _accelerationNearlyGravityCount; } - virtual float getObjectRestitution() const { return _entity->getRestitution(); } - virtual float getObjectFriction() const { return _entity->getFriction(); } - virtual float getObjectLinearDamping() const { return _entity->getDamping(); } - virtual float getObjectAngularDamping() const { return _entity->getAngularDamping(); } + virtual float getObjectRestitution() const override { return _entity->getRestitution(); } + virtual float getObjectFriction() const override { return _entity->getFriction(); } + virtual float getObjectLinearDamping() const override { return _entity->getDamping(); } + virtual float getObjectAngularDamping() const override { return _entity->getAngularDamping(); } - virtual glm::vec3 getObjectPosition() const { return _entity->getPosition() - ObjectMotionState::getWorldOffset(); } - virtual glm::quat getObjectRotation() const { return _entity->getRotation(); } - virtual glm::vec3 getObjectLinearVelocity() const { return _entity->getVelocity(); } - virtual glm::vec3 getObjectAngularVelocity() const { return _entity->getAngularVelocity(); } - virtual glm::vec3 getObjectGravity() const { return _entity->getGravity(); } - virtual glm::vec3 getObjectLinearVelocityChange() const; + virtual glm::vec3 getObjectPosition() const override { return _entity->getPosition() - ObjectMotionState::getWorldOffset(); } + virtual glm::quat getObjectRotation() const override { return _entity->getRotation(); } + virtual glm::vec3 getObjectLinearVelocity() const override { return _entity->getVelocity(); } + virtual glm::vec3 getObjectAngularVelocity() const override { return _entity->getAngularVelocity(); } + virtual glm::vec3 getObjectGravity() const override { return _entity->getGravity(); } + virtual glm::vec3 getObjectLinearVelocityChange() const override; - virtual const QUuid& getObjectID() const { return _entity->getID(); } + virtual const QUuid& getObjectID() const override { return _entity->getID(); } - virtual quint8 getSimulationPriority() const; - virtual QUuid getSimulatorID() const; - virtual void bump(quint8 priority); + virtual quint8 getSimulationPriority() const override; + virtual QUuid getSimulatorID() const override; + virtual void bump(quint8 priority) override; EntityItemPointer getEntity() const { return _entityPtr.lock(); } void resetMeasuredBodyAcceleration(); void measureBodyAcceleration(); - virtual QString getName(); + virtual QString getName() const override; - virtual int16_t computeCollisionGroup(); + virtual int16_t computeCollisionGroup() const override; // eternal logic can suggest a simuator priority bid for the next outgoing update void setOutgoingPriority(quint8 priority); @@ -92,7 +92,7 @@ protected: bool entityTreeIsLocked() const; #endif - virtual bool isReadyToComputeShape(); + virtual bool isReadyToComputeShape() const override; virtual btCollisionShape* computeNewShape(); virtual void setMotionType(MotionType motionType); diff --git a/libraries/physics/src/ObjectMotionState.h b/libraries/physics/src/ObjectMotionState.h index 2e30269efc..e10d58e3ed 100644 --- a/libraries/physics/src/ObjectMotionState.h +++ b/libraries/physics/src/ObjectMotionState.h @@ -134,9 +134,9 @@ public: virtual QUuid getSimulatorID() const = 0; virtual void bump(quint8 priority) {} - virtual QString getName() { return ""; } + virtual QString getName() const { return ""; } - virtual int16_t computeCollisionGroup() = 0; + virtual int16_t computeCollisionGroup() const = 0; bool isActive() const { return _body ? _body->isActive() : false; } @@ -148,7 +148,7 @@ public: friend class PhysicsEngine; protected: - virtual bool isReadyToComputeShape() = 0; + virtual bool isReadyToComputeShape() const = 0; virtual btCollisionShape* computeNewShape() = 0; void setMotionType(MotionType motionType); void updateCCDConfiguration(); From 42711e5bad48e80a56c9716943919cf28eecc886 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 7 Jan 2016 10:20:03 -0800 Subject: [PATCH 13/13] remove unused variable --- libraries/physics/src/PhysicsEngine.h | 1 - 1 file changed, 1 deletion(-) diff --git a/libraries/physics/src/PhysicsEngine.h b/libraries/physics/src/PhysicsEngine.h index 1161b89f2c..0ca9b2aca8 100644 --- a/libraries/physics/src/PhysicsEngine.h +++ b/libraries/physics/src/PhysicsEngine.h @@ -115,7 +115,6 @@ private: ContactMap _contactMap; uint32_t _numContactFrames = 0; - uint32_t _lastNumSubstepsAtUpdateInternal = 0; /// character collisions CharacterController* _myAvatarController;