cleanup of EntityItem::_created timestamp logic

EntityItem::_created initializes to 0 in ctor
and must be set either by EntityItemProperties (via server update)
or manually whenever a script creates a fresh entity

libraries/entities/src/EntityItem.cpp

@@ -53,13 +53,13 @@ void EntityItem::initFromEntityItemID(const EntityItemID& entityItemID) {
     _creatorTokenID = entityItemID.creatorTokenID;
 
     // init values with defaults before calling setProperties
+    quint64 now = usecTimestampNow();
+    _lastSimulated = now;
+    _lastUpdated = now;
     _lastEdited = 0;
     _lastEditedFromRemote = 0;
     _lastEditedFromRemoteInRemoteTime = 0;
-    
+    _created = UNKNOWN_CREATED_TIME;
-    _lastSimulated = 0;
-    _lastUpdated = 0;
-    _created = usecTimestampNow();
     _changedOnServer = 0;
 
     _position = glm::vec3(0,0,0);
@@ -85,12 +85,13 @@ void EntityItem::initFromEntityItemID(const EntityItemID& entityItemID) {
 
 EntityItem::EntityItem(const EntityItemID& entityItemID) {
     _type = EntityTypes::Unknown;
+    quint64 now = usecTimestampNow();
+    _lastSimulated = now;
+    _lastUpdated = now;
     _lastEdited = 0;
     _lastEditedFromRemote = 0;
     _lastEditedFromRemoteInRemoteTime = 0;
-    _lastSimulated = 0;
+    _created = UNKNOWN_CREATED_TIME;
-    _lastUpdated = 0;
-    _created = 0;
     _dirtyFlags = 0;
     _changedOnServer = 0;
     initFromEntityItemID(entityItemID);
@@ -99,16 +100,16 @@ EntityItem::EntityItem(const EntityItemID& entityItemID) {
 EntityItem::EntityItem(const EntityItemID& entityItemID, const EntityItemProperties& properties) {
     _type = EntityTypes::Unknown;
     quint64 now = usecTimestampNow();
-    _created = properties.getCreated() < now ? properties.getCreated() : now;
+    _lastSimulated = now;
-    _lastEdited = _lastEditedFromRemote = _lastSimulated = _lastUpdated = _lastEditedFromRemoteInRemoteTime = _created;
+    _lastUpdated = now;
+    _lastEdited = 0;
     _lastEditedFromRemote = 0;
     _lastEditedFromRemoteInRemoteTime = 0;
-    _lastSimulated = 0;
+    _created = UNKNOWN_CREATED_TIME;
-    _lastUpdated = 0;
     _dirtyFlags = 0;
     _changedOnServer = 0;
     initFromEntityItemID(entityItemID);
-    setProperties(properties, true); // force copy
+    setProperties(properties);
 }
 
 EntityPropertyFlags EntityItem::getEntityProperties(EncodeBitstreamParams& params) const {
@@ -365,12 +366,15 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef
         memcpy(&createdFromBuffer, dataAt, sizeof(createdFromBuffer));
         dataAt += sizeof(createdFromBuffer);
         bytesRead += sizeof(createdFromBuffer);
-        createdFromBuffer -= clockSkew;
 
-        if (createdFromBuffer < _created) {
+        quint64 now = usecTimestampNow();
-            // the server claims that this entity has an older creation time so we accept it
+        if (_created == UNKNOWN_CREATED_TIME) {
+            // we don't yet have a _created timestamp, so we accept this one
+            createdFromBuffer -= clockSkew;
+            if (createdFromBuffer > now || createdFromBuffer == UNKNOWN_CREATED_TIME) {
+                createdFromBuffer = now;
+            }
             _created = createdFromBuffer;
-            _lastEdited = _created;
         }
 
         if (wantDebug) {
@@ -385,7 +389,6 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef
             qDebug() << "    ago=" << editedAgo << "seconds - " << agoAsString;
         }
         
-        quint64 now = usecTimestampNow();
         quint64 lastEditedFromBuffer = 0;
         quint64 lastEditedFromBufferAdjusted = 0;
 
@@ -395,6 +398,9 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef
         dataAt += sizeof(lastEditedFromBuffer);
         bytesRead += sizeof(lastEditedFromBuffer);
         lastEditedFromBufferAdjusted = lastEditedFromBuffer - clockSkew;
+        if (lastEditedFromBufferAdjusted > now) {
+            lastEditedFromBufferAdjusted = now;
+        }
         
         bool fromSameServerEdit = (lastEditedFromBuffer == _lastEditedFromRemoteInRemoteTime);
 
@@ -444,10 +450,12 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef
             }
 
             // don't allow _lastEdited to be in the future
-            _lastEdited = lastEditedFromBufferAdjusted < now ? lastEditedFromBufferAdjusted : now;
+            _lastEdited = lastEditedFromBufferAdjusted;
             _lastEditedFromRemote = now;
             _lastEditedFromRemoteInRemoteTime = lastEditedFromBuffer;
             
+            // TODO: only send this notification if something ACTUALLY changed (hint, we haven't yet parsed 
+            // the properties out of the bitstream (see below))
             somethingChangedNotification(); // notify derived classes that something has changed
         }
 
@@ -456,8 +464,7 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef
         ByteCountCoded<quint64> updateDeltaCoder = encodedUpdateDelta;
         quint64 updateDelta = updateDeltaCoder;
         if (overwriteLocalData) {
-            _lastUpdated = lastEditedFromBufferAdjusted + updateDelta; // don't adjust for clock skew since we already did that for _lastEdited
+            _lastUpdated = lastEditedFromBufferAdjusted + updateDelta; // don't adjust for clock skew since we already did that
-            _lastSimulated = now;
             if (wantDebug) {
                 qDebug() << "_lastUpdated =" << _lastUpdated;
                 qDebug() << "_lastEdited=" << _lastEdited;
@@ -529,6 +536,9 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef
         bytesRead += readEntitySubclassDataFromBuffer(dataAt, (bytesLeftToRead - bytesRead), args, propertyFlags, overwriteLocalData);
 
         recalculateCollisionShape();
+        if (overwriteLocalData && (getDirtyFlags() & EntityItem::DIRTY_POSITION)) {
+            _lastSimulated = now;
+        }
     }
     return bytesRead;
 }
@@ -774,21 +784,9 @@ EntityItemProperties EntityItem::getProperties() const {
     return properties;
 }
 
-bool EntityItem::setProperties(const EntityItemProperties& properties, bool forceCopy) {
+bool EntityItem::setProperties(const EntityItemProperties& properties) {
     bool somethingChanged = false;
 
-    // handle the setting of created timestamps for the basic new entity case
-    if (forceCopy) {
-        quint64 now = usecTimestampNow();
-        if (properties.getCreated() == UNKNOWN_CREATED_TIME) {
-            _created = now;
-        } else if (properties.getCreated() != USE_EXISTING_CREATED_TIME) {
-            quint64 created = properties.getCreated();
-            // don't allow _created to be in the future
-            _created = created < now ? created : now;
-        }
-    }
-
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(position, updatePositionInMeters); // this will call recalculate collision shape if needed
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(dimensions, updateDimensionsInMeters); // NOTE: radius is obsolete
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(rotation, updateRotation);
@@ -818,16 +816,43 @@ bool EntityItem::setProperties(const EntityItemProperties& properties, bool forc
             qDebug() << "EntityItem::setProperties() AFTER update... edited AGO=" << elapsed <<
                     "now=" << now << " getLastEdited()=" << getLastEdited();
         }
-        // don't allow _lastEdited to be in the future
+        if (_created != UNKNOWN_CREATED_TIME) {
-        setLastEdited(properties._lastEdited < now ? properties._lastEdited : now);
+            setLastEdited(now);
+        }
         if (getDirtyFlags() & EntityItem::DIRTY_POSITION) {
-            _lastSimulated = usecTimestampNow();
+            _lastSimulated = now;
         }
     }
 
+    // timestamps
+    quint64 timestamp = properties.getCreated();
+    if (_created == UNKNOWN_CREATED_TIME && timestamp != UNKNOWN_CREATED_TIME) {
+        quint64 now = usecTimestampNow();
+        if (timestamp > now) {
+            timestamp = now;
+        }
+        _created = timestamp;
+
+        timestamp = properties.getLastEdited();
+        if (timestamp > now) {
+            timestamp = now;
+        } else if (timestamp < _created) {
+            timestamp = _created;
+        }
+        _lastEdited = timestamp;
+    }
+
     return somethingChanged;
 }
 
+void EntityItem::recordCreationTime() {
+    assert(_created == UNKNOWN_CREATED_TIME);
+    _created = usecTimestampNow();
+    _lastEdited = _created;
+    _lastUpdated = _created;
+    _lastSimulated = _created;
+}
+
 
 // TODO: is this really correct? how do we use size, does it need to handle rotation?
 float EntityItem::getSize() const { 

libraries/entities/src/EntityItem.h

@@ -74,12 +74,13 @@ public:
     virtual EntityItemProperties getProperties() const;
     
     /// returns true if something changed
-    virtual bool setProperties(const EntityItemProperties& properties, bool forceCopy = false);
+    virtual bool setProperties(const EntityItemProperties& properties);
 
     /// Override this in your derived class if you'd like to be informed when something about the state of the entity
     /// has changed. This will be called with properties change or when new data is loaded from a stream
     virtual void somethingChangedNotification() { }
 
+    void recordCreationTime();    // set _created to 'now'
     quint64 getLastSimulated() const { return _lastSimulated; } /// Last simulated time of this entity universal usecs
 
      /// Last edited time of this entity universal usecs

libraries/entities/src/EntityItemProperties.cpp

@@ -156,6 +156,17 @@ void EntityItemProperties::debugDump() const {
     props.debugDumpBits();
 }
 
+void EntityItemProperties::setCreated(quint64 usecTime) { 
+    _created = usecTime; 
+    if (_lastEdited < _created) {
+        _lastEdited = _created;
+    }
+}
+
+void EntityItemProperties::setLastEdited(quint64 usecTime) { 
+    _lastEdited = usecTime > _created ? usecTime : _created; 
+}
+
 EntityPropertyFlags EntityItemProperties::getChangedProperties() const {
     EntityPropertyFlags changedProperties;
     
@@ -652,9 +663,6 @@ bool EntityItemProperties::decodeEntityEditPacket(const unsigned char* data, int
         entityID.creatorTokenID = UNKNOWN_ENTITY_TOKEN;
         entityID.isKnownID = true;
         valid = true;
-
-        // created time is lastEdited time
-        properties.setCreated(USE_EXISTING_CREATED_TIME);
     }
 
     // Entity Type...

libraries/entities/src/EntityItemProperties.h

@@ -96,8 +96,6 @@ enum EntityPropertyList {
 typedef PropertyFlags<EntityPropertyList> EntityPropertyFlags;
 
 const quint64 UNKNOWN_CREATED_TIME = 0;
-const quint64 USE_EXISTING_CREATED_TIME = 1;
-
 
 /// A collection of properties of an entity item used in the scripting API. Translates between the actual properties of an
 /// entity and a JavaScript style hash/QScriptValue storing a set of properties. Used in scripting to set/get the complete
@@ -134,7 +132,7 @@ public:
     AABox getAABoxInMeters() const;
 
     void debugDump() const;
-    void setLastEdited(quint64 usecTime) { _lastEdited = usecTime; }
+    void setLastEdited(quint64 usecTime);
 
     DEFINE_PROPERTY(PROP_VISIBLE, Visible, visible, bool);
     DEFINE_PROPERTY_REF_WITH_SETTER(PROP_POSITION, Position, position, glm::vec3);
@@ -180,7 +178,7 @@ public:
 
     float getAge() const { return (float)(usecTimestampNow() - _created) / (float)USECS_PER_SECOND; }
     quint64 getCreated() const { return _created; }
-    void setCreated(quint64 usecTime) { _created = usecTime; }
+    void setCreated(quint64 usecTime);
     bool hasCreatedTime() const { return (_created != UNKNOWN_CREATED_TIME); }
 
     bool containsBoundsProperties() const { return (_positionChanged || _dimensionsChanged); }

libraries/entities/src/EntityItemPropertiesMacros.h

@@ -137,13 +137,13 @@
         }
 
 #define SET_ENTITY_PROPERTY_FROM_PROPERTIES(P,M)    \
-    if (properties._##P##Changed || forceCopy) {    \
+    if (properties._##P##Changed) {    \
         M(properties._##P);                         \
         somethingChanged = true;                    \
     }
 
 #define SET_ENTITY_PROPERTY_FROM_PROPERTIES_GETTER(C,G,S)    \
-    if (properties.C() || forceCopy) {    \
+    if (properties.C()) {    \
         S(properties.G());                         \
         somethingChanged = true;                    \
     }

libraries/entities/src/EntityTree.cpp

@@ -164,9 +164,17 @@ EntityItem* EntityTree::addEntity(const EntityItemID& entityID, const EntityItem
 
     // NOTE: This method is used in the client and the server tree. In the client, it's possible to create EntityItems 
     // that do not yet have known IDs. In the server tree however we don't want to have entities without known IDs.
-    if (getIsServer() && !entityID.isKnownID) {
+    bool recordCreationTime = false;
-        qDebug() << "UNEXPECTED!!! ----- EntityTree::addEntity()... (getIsSever() && !entityID.isKnownID)";
+    if (!entityID.isKnownID) {
-        return result;
+        if (getIsServer()) {
+            qDebug() << "UNEXPECTED!!! ----- EntityTree::addEntity()... (getIsSever() && !entityID.isKnownID)";
+            return result;
+        }
+        if (properties.getCreated() == UNKNOWN_CREATED_TIME) {
+            // the entity's creation time was not specified in properties, which means this is a NEW entity
+            // and we must record its creation time
+            recordCreationTime = true;
+        }
     }
 
     // You should not call this on existing entities that are already part of the tree! Call updateEntity()
@@ -182,6 +190,9 @@ EntityItem* EntityTree::addEntity(const EntityItemID& entityID, const EntityItem
     result = EntityTypes::constructEntityItem(type, entityID, properties);
 
     if (result) {
+        if (recordCreationTime) {
+            result->recordCreationTime();
+        }
         // Recurse the tree and store the entity in the correct tree element
         AddEntityOperator theOperator(this, result);
         recurseTreeWithOperator(&theOperator);

libraries/entities/src/EntityTypes.cpp

@@ -77,15 +77,7 @@ EntityItem* EntityTypes::constructEntityItem(EntityType entityType, const Entity
         factory = _factories[entityType];
     }
     if (factory) {
-        // NOTE: if someone attempts to create an entity with properties that do not include a proper "created" time
+        newEntityItem = factory(entityID, properties);
-        // then set the created time to now
-        if (!properties.hasCreatedTime()) {
-            EntityItemProperties mutableProperties = properties;
-            mutableProperties.setCreated(usecTimestampNow());
-            newEntityItem = factory(entityID, mutableProperties);
-        } else {
-            newEntityItem = factory(entityID, properties);
-        }
     }
     return newEntityItem;
 }
@@ -128,13 +120,6 @@ EntityItem* EntityTypes::constructEntityItem(const unsigned char* data, int byte
         
         EntityItemID tempEntityID(actualID);
         EntityItemProperties tempProperties;
-
-        // we set the Creation and Edit times to 'now', but if the server submits an earlier Creation time 
-        // then it will be accepted 
-        quint64 now = usecTimestampNow();
-        tempProperties.setCreated(now);
-        tempProperties.setLastEdited(now);
-
         return constructEntityItem(entityType, tempEntityID, tempProperties);
     }