Merge pull request #5274 from AndrewMeadows/argon

cleanup of EntityActionInterface and friends

assignment-client/src/AssignmentAction.cpp

@@ -13,9 +13,8 @@
 
 #include "AssignmentAction.h"
 
-AssignmentAction::AssignmentAction(EntityActionType type, QUuid id, EntityItemPointer ownerEntity) :
+AssignmentAction::AssignmentAction(EntityActionType type, const QUuid& id, EntityItemPointer ownerEntity) :
-    _id(id),
+    EntityActionInterface(type, id),
-    _type(type),
     _data(QByteArray()),
     _active(false),
     _ownerEntity(ownerEntity) {
@@ -28,7 +27,7 @@ void AssignmentAction::removeFromSimulation(EntitySimulation* simulation) const
     simulation->removeAction(_id);
 }
 
-QByteArray AssignmentAction::serialize() {
+QByteArray AssignmentAction::serialize() const {
     return _data;
 }
 

assignment-client/src/AssignmentAction.h

@@ -21,23 +21,19 @@
 
 class AssignmentAction : public EntityActionInterface {
 public:
-    AssignmentAction(EntityActionType type, QUuid id, EntityItemPointer ownerEntity);
+    AssignmentAction(EntityActionType type, const QUuid& id, EntityItemPointer ownerEntity);
     virtual ~AssignmentAction();
 
-    const QUuid& getID() const { return _id; }
-    virtual EntityActionType getType() { return _type; }
     virtual void removeFromSimulation(EntitySimulation* simulation) const;
     virtual EntityItemWeakPointer getOwnerEntity() const { return _ownerEntity; }
     virtual void setOwnerEntity(const EntityItemPointer ownerEntity) { _ownerEntity = ownerEntity; }
     virtual bool updateArguments(QVariantMap arguments);
     virtual QVariantMap getArguments();
 
-    virtual QByteArray serialize();
+    virtual QByteArray serialize() const;
     virtual void deserialize(QByteArray serializedArguments);
 
 private:
-    QUuid _id;
-    EntityActionType _type;
     QByteArray _data;
 
 protected:

assignment-client/src/AssignmentActionFactory.cpp

@@ -12,14 +12,14 @@
 #include "AssignmentActionFactory.h"
 
 
-EntityActionPointer assignmentActionFactory(EntityActionType type, QUuid id, EntityItemPointer ownerEntity) {
+EntityActionPointer assignmentActionFactory(EntityActionType type, const QUuid& id, EntityItemPointer ownerEntity) {
     return (EntityActionPointer) new AssignmentAction(type, id, ownerEntity);
 }
 
 
 EntityActionPointer AssignmentActionFactory::factory(EntitySimulation* simulation,
                                                      EntityActionType type,
-                                                     QUuid id,
+                                                     const QUuid& id,
                                                      EntityItemPointer ownerEntity,
                                                      QVariantMap arguments) {
     EntityActionPointer action = assignmentActionFactory(type, id, ownerEntity);

assignment-client/src/AssignmentActionFactory.h

@@ -21,7 +21,7 @@ public:
     virtual ~AssignmentActionFactory() { }
     virtual EntityActionPointer factory(EntitySimulation* simulation,
                                         EntityActionType type,
-                                        QUuid id,
+                                        const QUuid& id,
                                         EntityItemPointer ownerEntity,
                                         QVariantMap arguments);
     virtual EntityActionPointer factoryBA(EntitySimulation* simulation,

interface/src/InterfaceActionFactory.cpp

@@ -18,16 +18,16 @@
 #include "InterfaceActionFactory.h"
 
 
-EntityActionPointer interfaceActionFactory(EntityActionType type, QUuid id, EntityItemPointer ownerEntity) {
+EntityActionPointer interfaceActionFactory(EntityActionType type, const QUuid& id, EntityItemPointer ownerEntity) {
     switch (type) {
         case ACTION_TYPE_NONE:
             return nullptr;
         case ACTION_TYPE_OFFSET:
-            return (EntityActionPointer) new ObjectActionOffset(type, id, ownerEntity);
+            return (EntityActionPointer) new ObjectActionOffset(id, ownerEntity);
         case ACTION_TYPE_SPRING:
-            return (EntityActionPointer) new ObjectActionSpring(type, id, ownerEntity);
+            return (EntityActionPointer) new ObjectActionSpring(id, ownerEntity);
         case ACTION_TYPE_HOLD:
-            return (EntityActionPointer) new AvatarActionHold(type, id, ownerEntity);
+            return (EntityActionPointer) new AvatarActionHold(id, ownerEntity);
     }
 
     assert(false);
@@ -37,7 +37,7 @@ EntityActionPointer interfaceActionFactory(EntityActionType type, QUuid id, Enti
 
 EntityActionPointer InterfaceActionFactory::factory(EntitySimulation* simulation,
                                                     EntityActionType type,
-                                                    QUuid id,
+                                                    const QUuid& id,
                                                     EntityItemPointer ownerEntity,
                                                     QVariantMap arguments) {
     EntityActionPointer action = interfaceActionFactory(type, id, ownerEntity);

interface/src/InterfaceActionFactory.h

@@ -20,7 +20,7 @@ public:
     virtual ~InterfaceActionFactory() { }
     virtual EntityActionPointer factory(EntitySimulation* simulation,
                                         EntityActionType type,
-                                        QUuid id,
+                                        const QUuid& id,
                                         EntityItemPointer ownerEntity,
                                         QVariantMap arguments);
     virtual EntityActionPointer factoryBA(EntitySimulation* simulation,

interface/src/avatar/AvatarActionHold.cpp

@@ -17,13 +17,14 @@
 
 const uint16_t AvatarActionHold::holdVersion = 1;
 
-AvatarActionHold::AvatarActionHold(EntityActionType type, QUuid id, EntityItemPointer ownerEntity) :
+AvatarActionHold::AvatarActionHold(const QUuid& id, EntityItemPointer ownerEntity) :
-    ObjectActionSpring(type, id, ownerEntity),
+    ObjectActionSpring(id, ownerEntity),
     _relativePosition(glm::vec3(0.0f)),
     _relativeRotation(glm::quat()),
     _hand("right"),
     _mine(false)
 {
+    _type = ACTION_TYPE_HOLD;
     #if WANT_DEBUG
     qDebug() << "AvatarActionHold::AvatarActionHold";
     #endif
@@ -166,8 +167,7 @@ QVariantMap AvatarActionHold::getArguments() {
 
 
 void AvatarActionHold::deserialize(QByteArray serializedArguments) {
-    if (_mine) {
+    if (!_mine) {
-        return;
+        ObjectActionSpring::deserialize(serializedArguments);
     }
-    ObjectActionSpring::deserialize(serializedArguments);
 }

interface/src/avatar/AvatarActionHold.h

@@ -19,11 +19,9 @@
 
 class AvatarActionHold : public ObjectActionSpring {
 public:
-    AvatarActionHold(EntityActionType type, QUuid id, EntityItemPointer ownerEntity);
+    AvatarActionHold(const QUuid& id, EntityItemPointer ownerEntity);
     virtual ~AvatarActionHold();
 
-    virtual EntityActionType getType() { return ACTION_TYPE_HOLD; }
-
     virtual bool updateArguments(QVariantMap arguments);
     virtual QVariantMap getArguments();
 

libraries/entities/src/EntityActionFactoryInterface.h

@@ -25,7 +25,7 @@ class EntityActionFactoryInterface : public QObject, public Dependency {
     virtual ~EntityActionFactoryInterface() { }
     virtual EntityActionPointer factory(EntitySimulation* simulation,
                                         EntityActionType type,
-                                        QUuid id,
+                                        const QUuid& id,
                                         EntityItemPointer ownerEntity,
                                         QVariantMap arguments) { assert(false); return nullptr; }
     virtual EntityActionPointer factoryBA(EntitySimulation* simulation,

libraries/entities/src/EntityActionInterface.h

@@ -29,10 +29,10 @@ enum EntityActionType {
 
 class EntityActionInterface {
 public:
-    EntityActionInterface() { }
+    EntityActionInterface(EntityActionType type, const QUuid& id) : _id(id), _type(type) { }
     virtual ~EntityActionInterface() { }
-    virtual const QUuid& getID() const = 0;
+    const QUuid& getID() const { return _id; }
-    virtual EntityActionType getType() { assert(false); return ACTION_TYPE_NONE; }
+    EntityActionType getType() const { return _type; }
 
     virtual void removeFromSimulation(EntitySimulation* simulation) const = 0;
     virtual EntityItemWeakPointer getOwnerEntity() const = 0;
@@ -40,7 +40,7 @@ public:
     virtual bool updateArguments(QVariantMap arguments) = 0;
     virtual QVariantMap getArguments() = 0;
 
-    virtual QByteArray serialize() = 0;
+    virtual QByteArray serialize() const = 0;
     virtual void deserialize(QByteArray serializedArguments) = 0;
 
     static EntityActionType actionTypeFromString(QString actionTypeString);
@@ -68,6 +68,8 @@ protected:
     static QString extractStringArgument(QString objectName, QVariantMap arguments,
                                          QString argumentName, bool& ok, bool required = true);
 
+    QUuid _id;
+    EntityActionType _type;
 };
 
 

libraries/physics/src/ObjectAction.cpp

@@ -13,9 +13,9 @@
 
 #include "ObjectAction.h"
 
-ObjectAction::ObjectAction(EntityActionType type, QUuid id, EntityItemPointer ownerEntity) :
+ObjectAction::ObjectAction(EntityActionType type, const QUuid& id, EntityItemPointer ownerEntity) :
     btActionInterface(),
-    _id(id),
+    EntityActionInterface(type, id),
     _active(false),
     _ownerEntity(ownerEntity) {
 }
@@ -24,15 +24,15 @@ ObjectAction::~ObjectAction() {
 }
 
 void ObjectAction::updateAction(btCollisionWorld* collisionWorld, btScalar deltaTimeStep) {
-    if (!_active) {
-        return;
-    }
     if (_ownerEntity.expired()) {
         qDebug() << "warning -- action with no entity removing self from btCollisionWorld.";
         btDynamicsWorld* dynamicsWorld = static_cast<btDynamicsWorld*>(collisionWorld);
         dynamicsWorld->removeAction(this);
         return;
     }
+    if (!_active) {
+        return;
+    }
 
     updateActionWorker(deltaTimeStep);
 }
@@ -129,11 +129,3 @@ void ObjectAction::setAngularVelocity(glm::vec3 angularVelocity) {
     rigidBody->activate();
 }
 
-QByteArray ObjectAction::serialize() {
-    assert(false);
-    return QByteArray();
-}
-
-void ObjectAction::deserialize(QByteArray serializedArguments) {
-    assert(false);
-}

libraries/physics/src/ObjectAction.h

@@ -17,8 +17,6 @@
 
 #include <btBulletDynamicsCommon.h>
 
-#include <EntityItem.h>
-
 #include "ObjectMotionState.h"
 #include "BulletUtil.h"
 #include "EntityActionInterface.h"
@@ -26,31 +24,25 @@
 
 class ObjectAction : public btActionInterface, public EntityActionInterface {
 public:
-    ObjectAction(EntityActionType type, QUuid id, EntityItemPointer ownerEntity);
+    ObjectAction(EntityActionType type, const QUuid& id, EntityItemPointer ownerEntity);
     virtual ~ObjectAction();
 
-    const QUuid& getID() const { return _id; }
-    virtual EntityActionType getType() { assert(false); return ACTION_TYPE_NONE; }
     virtual void removeFromSimulation(EntitySimulation* simulation) const;
     virtual EntityItemWeakPointer getOwnerEntity() const { return _ownerEntity; }
     virtual void setOwnerEntity(const EntityItemPointer ownerEntity) { _ownerEntity = ownerEntity; }
 
-    virtual bool updateArguments(QVariantMap arguments) { return false; }
+    virtual bool updateArguments(QVariantMap arguments) = 0;
-    virtual QVariantMap getArguments() { return QVariantMap(); }
+    virtual QVariantMap getArguments() = 0;
 
     // this is called from updateAction and should be overridden by subclasses
-    virtual void updateActionWorker(float deltaTimeStep) {}
+    virtual void updateActionWorker(float deltaTimeStep) = 0;
 
     // these are from btActionInterface
     virtual void updateAction(btCollisionWorld* collisionWorld, btScalar deltaTimeStep);
     virtual void debugDraw(btIDebugDraw* debugDrawer);
 
-    virtual QByteArray serialize();
+    virtual QByteArray serialize() const = 0;
-    virtual void deserialize(QByteArray serializedArguments);
+    virtual void deserialize(QByteArray serializedArguments) = 0;
-
-private:
-    QUuid _id;
-    QReadWriteLock _lock;
 
 protected:
 
@@ -70,6 +62,10 @@ protected:
     bool tryLockForWrite() { return _lock.tryLockForWrite(); }
     void unlock() { _lock.unlock(); }
 
+private:
+    QReadWriteLock _lock;
+
+protected:
     bool _active;
     EntityItemWeakPointer _ownerEntity;
 };

libraries/physics/src/ObjectActionOffset.cpp

@@ -15,8 +15,8 @@
 
 const uint16_t ObjectActionOffset::offsetVersion = 1;
 
-ObjectActionOffset::ObjectActionOffset(EntityActionType type, QUuid id, EntityItemPointer ownerEntity) :
+ObjectActionOffset::ObjectActionOffset(const QUuid& id, EntityItemPointer ownerEntity) :
-    ObjectAction(type, id, ownerEntity) {
+    ObjectAction(ACTION_TYPE_OFFSET, id, ownerEntity) {
     #if WANT_DEBUG
     qDebug() << "ObjectActionOffset::ObjectActionOffset";
     #endif
@@ -127,7 +127,7 @@ QVariantMap ObjectActionOffset::getArguments() {
     return arguments;
 }
 
-QByteArray ObjectActionOffset::serialize() {
+QByteArray ObjectActionOffset::serialize() const {
     QByteArray ba;
     QDataStream dataStream(&ba, QIODevice::WriteOnly);
     dataStream << getType();
@@ -146,13 +146,14 @@ void ObjectActionOffset::deserialize(QByteArray serializedArguments) {
     QDataStream dataStream(serializedArguments);
 
     EntityActionType type;
-    QUuid id;
-    uint16_t serializationVersion;
-
     dataStream >> type;
     assert(type == getType());
+
+    QUuid id;
     dataStream >> id;
     assert(id == getID());
+
+    uint16_t serializationVersion;
     dataStream >> serializationVersion;
     if (serializationVersion != ObjectActionOffset::offsetVersion) {
         return;

libraries/physics/src/ObjectActionOffset.h

@@ -19,17 +19,15 @@
 
 class ObjectActionOffset : public ObjectAction {
 public:
-    ObjectActionOffset(EntityActionType type, QUuid id, EntityItemPointer ownerEntity);
+    ObjectActionOffset(const QUuid& id, EntityItemPointer ownerEntity);
     virtual ~ObjectActionOffset();
 
-    virtual EntityActionType getType() { return ACTION_TYPE_OFFSET; }
-
     virtual bool updateArguments(QVariantMap arguments);
     virtual QVariantMap getArguments();
 
     virtual void updateActionWorker(float deltaTimeStep);
 
-    virtual QByteArray serialize();
+    virtual QByteArray serialize() const;
     virtual void deserialize(QByteArray serializedArguments);
 
  private:

libraries/physics/src/ObjectActionSpring.cpp

@@ -17,8 +17,8 @@ const float SPRING_MAX_SPEED = 10.0f;
 
 const uint16_t ObjectActionSpring::springVersion = 1;
 
-ObjectActionSpring::ObjectActionSpring(EntityActionType type, QUuid id, EntityItemPointer ownerEntity) :
+ObjectActionSpring::ObjectActionSpring(const QUuid& id, EntityItemPointer ownerEntity) :
-    ObjectAction(type, id, ownerEntity),
+    ObjectAction(ACTION_TYPE_SPRING, id, ownerEntity),
     _positionalTarget(glm::vec3(0.0f)),
     _linearTimeScale(0.2f),
     _positionalTargetSet(false),
@@ -206,7 +206,7 @@ QVariantMap ObjectActionSpring::getArguments() {
     return arguments;
 }
 
-QByteArray ObjectActionSpring::serialize() {
+QByteArray ObjectActionSpring::serialize() const {
     QByteArray serializedActionArguments;
     QDataStream dataStream(&serializedActionArguments, QIODevice::WriteOnly);
 
@@ -229,13 +229,14 @@ void ObjectActionSpring::deserialize(QByteArray serializedArguments) {
     QDataStream dataStream(serializedArguments);
 
     EntityActionType type;
-    QUuid id;
-    uint16_t serializationVersion;
-
     dataStream >> type;
     assert(type == getType());
+
+    QUuid id;
     dataStream >> id;
     assert(id == getID());
+
+    uint16_t serializationVersion;
     dataStream >> serializationVersion;
     if (serializationVersion != ObjectActionSpring::springVersion) {
         return;

libraries/physics/src/ObjectActionSpring.h

@@ -12,24 +12,19 @@
 #ifndef hifi_ObjectActionSpring_h
 #define hifi_ObjectActionSpring_h
 
-#include <QUuid>
-
-#include <EntityItem.h>
 #include "ObjectAction.h"
 
 class ObjectActionSpring : public ObjectAction {
 public:
-    ObjectActionSpring(EntityActionType type, QUuid id, EntityItemPointer ownerEntity);
+    ObjectActionSpring(const QUuid& id, EntityItemPointer ownerEntity);
     virtual ~ObjectActionSpring();
 
-    virtual EntityActionType getType() { return ACTION_TYPE_SPRING; }
-
     virtual bool updateArguments(QVariantMap arguments);
     virtual QVariantMap getArguments();
 
     virtual void updateActionWorker(float deltaTimeStep);
 
-    virtual QByteArray serialize();
+    virtual QByteArray serialize() const;
     virtual void deserialize(QByteArray serializedArguments);
 
 protected:

libraries/shared/src/QVariantGLM.cpp

@@ -20,7 +20,7 @@ QVariantList glmToQList(const glm::quat& g) {
     return QVariantList() << g.x << g.y << g.z << g.w;
 }
 
-QVariantList rgbColorToQList(rgbColor& v) {
+QVariantList rgbColorToQList(const rgbColor& v) {
     return QVariantList() << (int)(v[0]) << (int)(v[1]) << (int)(v[2]);
 }
 
@@ -42,12 +42,12 @@ QVariantMap glmToQMap(const glm::quat& glmQuat) {
 }
 
 
-glm::vec3 qListToGlmVec3(const QVariant q) {
+glm::vec3 qListToGlmVec3(const QVariant& q) {
     QVariantList qList = q.toList();
     return glm::vec3(qList[RED_INDEX].toFloat(), qList[GREEN_INDEX].toFloat(), qList[BLUE_INDEX].toFloat());
 }
 
-glm::quat qListToGlmQuat(const QVariant q) {
+glm::quat qListToGlmQuat(const QVariant& q) {
     QVariantList qList = q.toList();
     float x = qList[0].toFloat();
     float y = qList[1].toFloat();
@@ -56,7 +56,7 @@ glm::quat qListToGlmQuat(const QVariant q) {
     return glm::quat(w, x, y, z);
 }
 
-void qListtoRgbColor(const QVariant q, rgbColor returnValue) {
+void qListtoRgbColor(const QVariant& q, rgbColor& returnValue) {
     QVariantList qList = q.toList();
     returnValue[RED_INDEX] = qList[RED_INDEX].toInt();
     returnValue[GREEN_INDEX] = qList[GREEN_INDEX].toInt();

libraries/shared/src/QVariantGLM.h

@@ -19,11 +19,11 @@
 
 QVariantList glmToQList(const glm::vec3& g);
 QVariantList glmToQList(const glm::quat& g);
-QVariantList rgbColorToQList(rgbColor& v);
+QVariantList rgbColorToQList(const rgbColor& v);
 
 QVariantMap glmToQMap(const glm::vec3& glmVector);
 QVariantMap glmToQMap(const glm::quat& glmQuat);
 
-glm::vec3 qListToGlmVec3(const QVariant q);
+glm::vec3 qListToGlmVec3(const QVariant& q);
-glm::quat qListToGlmQuat(const QVariant q);
+glm::quat qListToGlmQuat(const QVariant& q);
-void qListtoRgbColor(const QVariant q, rgbColor returnValue);
+void qListtoRgbColor(const QVariant& q, rgbColor& returnValue);