responses to code review

This commit is contained in:
Seth Alves 2015-07-01 10:42:04 -07:00
parent 5e2f7204b4
commit e8a6acd65b
4 changed files with 35 additions and 31 deletions

View file

@ -27,7 +27,7 @@ public:
const QUuid& getID() const { return _id; } const QUuid& getID() const { return _id; }
virtual EntityActionType getType() { return _type; } virtual EntityActionType getType() { return _type; }
virtual void removeFromSimulation(EntitySimulation* simulation) const; virtual void removeFromSimulation(EntitySimulation* simulation) const;
virtual const EntityItemWeakPointer getOwnerEntity() const { return _ownerEntity; } virtual EntityItemWeakPointer getOwnerEntity() const { return _ownerEntity; }
virtual void setOwnerEntity(const EntityItemPointer ownerEntity) { _ownerEntity = ownerEntity; } virtual void setOwnerEntity(const EntityItemPointer ownerEntity) { _ownerEntity = ownerEntity; }
virtual bool updateArguments(QVariantMap arguments); virtual bool updateArguments(QVariantMap arguments);
virtual QVariantMap getArguments(); virtual QVariantMap getArguments();

View file

@ -35,7 +35,7 @@ public:
virtual EntityActionType getType() { assert(false); return ACTION_TYPE_NONE; } virtual EntityActionType getType() { assert(false); return ACTION_TYPE_NONE; }
virtual void removeFromSimulation(EntitySimulation* simulation) const = 0; virtual void removeFromSimulation(EntitySimulation* simulation) const = 0;
virtual const EntityItemWeakPointer getOwnerEntity() const = 0; virtual EntityItemWeakPointer getOwnerEntity() const = 0;
virtual void setOwnerEntity(const EntityItemPointer ownerEntity) = 0; virtual void setOwnerEntity(const EntityItemPointer ownerEntity) = 0;
virtual bool updateArguments(QVariantMap arguments) = 0; virtual bool updateArguments(QVariantMap arguments) = 0;
virtual QVariantMap getArguments() = 0; virtual QVariantMap getArguments() = 0;

View file

@ -541,7 +541,7 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef
dataAt += propertyFlags.getEncodedLength(); dataAt += propertyFlags.getEncodedLength();
bytesRead += propertyFlags.getEncodedLength(); bytesRead += propertyFlags.getEncodedLength();
if (args.bitstreamVersion >= VERSION_ENTITIES_HAVE_SIMULATION_OWNER) { if (args.bitstreamVersion >= VERSION_ENTITIES_HAVE_SIMULATION_OWNER_AND_ACTIONS_OVER_WIRE) {
// pack SimulationOwner and terse update properties near each other // pack SimulationOwner and terse update properties near each other
// NOTE: the server is authoritative for changes to simOwnerID so we always unpack ownership data // NOTE: the server is authoritative for changes to simOwnerID so we always unpack ownership data
@ -611,7 +611,7 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef
READ_ENTITY_PROPERTY(PROP_LOCKED, bool, setLocked); READ_ENTITY_PROPERTY(PROP_LOCKED, bool, setLocked);
READ_ENTITY_PROPERTY(PROP_USER_DATA, QString, setUserData); READ_ENTITY_PROPERTY(PROP_USER_DATA, QString, setUserData);
if (args.bitstreamVersion < VERSION_ENTITIES_HAVE_SIMULATION_OWNER) { if (args.bitstreamVersion < VERSION_ENTITIES_HAVE_SIMULATION_OWNER_AND_ACTIONS_OVER_WIRE) {
// this code for when there is only simulatorID and no simulation priority // this code for when there is only simulatorID and no simulation priority
// we always accept the server's notion of simulatorID, so we fake overwriteLocalData as true // we always accept the server's notion of simulatorID, so we fake overwriteLocalData as true

View file

@ -142,7 +142,7 @@ void PhysicsEngine::addObject(ObjectMotionState* motionState) {
motionState->getAndClearIncomingDirtyFlags(); motionState->getAndClearIncomingDirtyFlags();
} }
void PhysicsEngine::removeObject(ObjectMotionState* object) { void PhysicsEngine::removeObject(ObjectMotionState* object) {
// wake up anything touching this object // wake up anything touching this object
bump(object); bump(object);
@ -172,7 +172,7 @@ void PhysicsEngine::deleteObjects(SetOfMotionStates& objects) {
for (auto object : objects) { for (auto object : objects) {
btRigidBody* body = object->getRigidBody(); btRigidBody* body = object->getRigidBody();
removeObject(object); removeObject(object);
// NOTE: setRigidBody() modifies body->m_userPointer so we should clear the MotionState's body BEFORE deleting it. // NOTE: setRigidBody() modifies body->m_userPointer so we should clear the MotionState's body BEFORE deleting it.
object->setRigidBody(nullptr); object->setRigidBody(nullptr);
body->setMotionState(nullptr); body->setMotionState(nullptr);
@ -263,22 +263,26 @@ void PhysicsEngine::doOwnershipInfection(const btCollisionObject* objectA, const
const btCollisionObject* characterObject = _characterController ? _characterController->getCollisionObject() : nullptr; const btCollisionObject* characterObject = _characterController ? _characterController->getCollisionObject() : nullptr;
ObjectMotionState* a = static_cast<ObjectMotionState*>(objectA->getUserPointer()); ObjectMotionState* motionStateA = static_cast<ObjectMotionState*>(objectA->getUserPointer());
ObjectMotionState* b = static_cast<ObjectMotionState*>(objectB->getUserPointer()); ObjectMotionState* motionStateB = static_cast<ObjectMotionState*>(objectB->getUserPointer());
if (b && ((a && a->getSimulatorID() == _sessionID && !objectA->isStaticObject()) || (objectA == characterObject))) { if (motionStateB &&
// NOTE: we might own the simulation of a kinematic object (A) ((motionStateA && motionStateA->getSimulatorID() == _sessionID && !objectA->isStaticObject()) ||
(objectA == characterObject))) {
// NOTE: we might own the simulation of a kinematic object (A)
// but we don't claim ownership of kinematic objects (B) based on collisions here. // but we don't claim ownership of kinematic objects (B) based on collisions here.
if (!objectB->isStaticOrKinematicObject() && b->getSimulatorID() != _sessionID) { if (!objectB->isStaticOrKinematicObject() && motionStateB->getSimulatorID() != _sessionID) {
quint8 priority = a ? a->getSimulationPriority() : PERSONAL_SIMULATION_PRIORITY; quint8 priority = motionStateA ? motionStateA->getSimulationPriority() : PERSONAL_SIMULATION_PRIORITY;
b->bump(priority); motionStateB->bump(priority);
} }
} else if (a && ((b && b->getSimulatorID() == _sessionID && !objectB->isStaticObject()) || (objectB == characterObject))) { } else if (motionStateA &&
// SIMILARLY: we might own the simulation of a kinematic object (B) ((motionStateB && motionStateB->getSimulatorID() == _sessionID && !objectB->isStaticObject()) ||
(objectB == characterObject))) {
// SIMILARLY: we might own the simulation of a kinematic object (B)
// but we don't claim ownership of kinematic objects (A) based on collisions here. // but we don't claim ownership of kinematic objects (A) based on collisions here.
if (!objectA->isStaticOrKinematicObject() && a->getSimulatorID() != _sessionID) { if (!objectA->isStaticOrKinematicObject() && motionStateA->getSimulatorID() != _sessionID) {
quint8 priority = b ? b->getSimulationPriority() : PERSONAL_SIMULATION_PRIORITY; quint8 priority = motionStateB ? motionStateB->getSimulationPriority() : PERSONAL_SIMULATION_PRIORITY;
a->bump(priority); motionStateA->bump(priority);
} }
} }
} }
@ -292,13 +296,13 @@ void PhysicsEngine::updateContactMap() {
for (int i = 0; i < numManifolds; ++i) { for (int i = 0; i < numManifolds; ++i) {
btPersistentManifold* contactManifold = _collisionDispatcher->getManifoldByIndexInternal(i); btPersistentManifold* contactManifold = _collisionDispatcher->getManifoldByIndexInternal(i);
if (contactManifold->getNumContacts() > 0) { if (contactManifold->getNumContacts() > 0) {
// TODO: require scripts to register interest in callbacks for specific objects // TODO: require scripts to register interest in callbacks for specific objects
// so we can filter out most collision events right here. // so we can filter out most collision events right here.
const btCollisionObject* objectA = static_cast<const btCollisionObject*>(contactManifold->getBody0()); const btCollisionObject* objectA = static_cast<const btCollisionObject*>(contactManifold->getBody0());
const btCollisionObject* objectB = static_cast<const btCollisionObject*>(contactManifold->getBody1()); const btCollisionObject* objectB = static_cast<const btCollisionObject*>(contactManifold->getBody1());
if (!(objectA->isActive() || objectB->isActive())) { if (!(objectA->isActive() || objectB->isActive())) {
// both objects are inactive so stop tracking this contact, // both objects are inactive so stop tracking this contact,
// which will eventually trigger a CONTACT_EVENT_TYPE_END // which will eventually trigger a CONTACT_EVENT_TYPE_END
continue; continue;
} }
@ -328,24 +332,24 @@ CollisionEvents& PhysicsEngine::getCollisionEvents() {
ContactInfo& contact = contactItr->second; ContactInfo& contact = contactItr->second;
ContactEventType type = contact.computeType(_numContactFrames); ContactEventType type = contact.computeType(_numContactFrames);
if(type != CONTACT_EVENT_TYPE_CONTINUE || _numSubsteps % CONTINUE_EVENT_FILTER_FREQUENCY == 0) { if(type != CONTACT_EVENT_TYPE_CONTINUE || _numSubsteps % CONTINUE_EVENT_FILTER_FREQUENCY == 0) {
ObjectMotionState* A = static_cast<ObjectMotionState*>(contactItr->first._a); ObjectMotionState* motionStateA = static_cast<ObjectMotionState*>(contactItr->first._a);
ObjectMotionState* B = static_cast<ObjectMotionState*>(contactItr->first._b); ObjectMotionState* motionStateB = static_cast<ObjectMotionState*>(contactItr->first._b);
glm::vec3 velocityChange = (A ? A->getObjectLinearVelocityChange() : glm::vec3(0.0f)) + glm::vec3 velocityChange = (motionStateA ? motionStateA->getObjectLinearVelocityChange() : glm::vec3(0.0f)) +
(B ? B->getObjectLinearVelocityChange() : glm::vec3(0.0f)); (motionStateB ? motionStateB->getObjectLinearVelocityChange() : glm::vec3(0.0f));
if (A && A->getType() == MOTIONSTATE_TYPE_ENTITY) { if (motionStateA && motionStateA->getType() == MOTIONSTATE_TYPE_ENTITY) {
QUuid idA = A->getObjectID(); QUuid idA = motionStateA->getObjectID();
QUuid idB; QUuid idB;
if (B && B->getType() == MOTIONSTATE_TYPE_ENTITY) { if (motionStateB && motionStateB->getType() == MOTIONSTATE_TYPE_ENTITY) {
idB = B->getObjectID(); idB = motionStateB->getObjectID();
} }
glm::vec3 position = bulletToGLM(contact.getPositionWorldOnB()) + _originOffset; glm::vec3 position = bulletToGLM(contact.getPositionWorldOnB()) + _originOffset;
glm::vec3 penetration = bulletToGLM(contact.distance * contact.normalWorldOnB); glm::vec3 penetration = bulletToGLM(contact.distance * contact.normalWorldOnB);
_collisionEvents.push_back(Collision(type, idA, idB, position, penetration, velocityChange)); _collisionEvents.push_back(Collision(type, idA, idB, position, penetration, velocityChange));
} else if (B && B->getType() == MOTIONSTATE_TYPE_ENTITY) { } else if (motionStateB && motionStateB->getType() == MOTIONSTATE_TYPE_ENTITY) {
QUuid idB = B->getObjectID(); QUuid idB = motionStateB->getObjectID();
glm::vec3 position = bulletToGLM(contact.getPositionWorldOnA()) + _originOffset; glm::vec3 position = bulletToGLM(contact.getPositionWorldOnA()) + _originOffset;
// NOTE: we're flipping the order of A and B (so that the first objectID is never NULL) // NOTE: we're flipping the order of A and B (so that the first objectID is never NULL)
// hence we must negate the penetration. // hence we must negate the penetration.
glm::vec3 penetration = - bulletToGLM(contact.distance * contact.normalWorldOnB); glm::vec3 penetration = - bulletToGLM(contact.distance * contact.normalWorldOnB);
_collisionEvents.push_back(Collision(type, idB, QUuid(), position, penetration, velocityChange)); _collisionEvents.push_back(Collision(type, idB, QUuid(), position, penetration, velocityChange));