From f6446c6806e17dfdc76e423b8757143393cf97f2 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Fri, 13 Jan 2017 13:18:24 -0800 Subject: [PATCH 01/12] minimum entity edit filter --- .../src/entities/EntityServer.cpp | 6 +++ assignment-client/src/octree/OctreeServer.cpp | 3 ++ .../resources/describe-settings.json | 8 ++++ libraries/entities/src/EntityTree.cpp | 42 +++++++++++++++++++ libraries/entities/src/EntityTree.h | 12 ++++++ libraries/octree/src/Octree.h | 1 + 6 files changed, 72 insertions(+) diff --git a/assignment-client/src/entities/EntityServer.cpp b/assignment-client/src/entities/EntityServer.cpp index 23eec6197c..e5c22c3d34 100644 --- a/assignment-client/src/entities/EntityServer.cpp +++ b/assignment-client/src/entities/EntityServer.cpp @@ -285,6 +285,12 @@ void EntityServer::readAdditionalConfiguration(const QJsonObject& settingsSectio } else { tree->setEntityScriptSourceWhitelist(""); } + + QString entityEditFilter; + if (readOptionString("entityEditFilter", settingsSectionObject, entityEditFilter)) { + tree->setEntityEditFilter(entityEditFilter); + } + tree->initEntityEditFilterEngine(); // whether supplied or not. } void EntityServer::nodeAdded(SharedNodePointer node) { diff --git a/assignment-client/src/octree/OctreeServer.cpp b/assignment-client/src/octree/OctreeServer.cpp index 3e36250a82..ad40b1c671 100644 --- a/assignment-client/src/octree/OctreeServer.cpp +++ b/assignment-client/src/octree/OctreeServer.cpp @@ -660,6 +660,7 @@ bool OctreeServer::handleHTTPRequest(HTTPConnection* connection, const QUrl& url quint64 averageUpdateTime = _tree->getAverageUpdateTime(); quint64 averageCreateTime = _tree->getAverageCreateTime(); quint64 averageLoggingTime = _tree->getAverageLoggingTime(); + quint64 averageFilterTime = _tree->getAverageFilterTime(); int FLOAT_PRECISION = 3; @@ -699,6 +700,8 @@ bool OctreeServer::handleHTTPRequest(HTTPConnection* connection, const QUrl& url .arg(locale.toString((uint)averageCreateTime).rightJustified(COLUMN_WIDTH, ' ')); statsString += QString(" Average Logging Time: %1 usecs\r\n") .arg(locale.toString((uint)averageLoggingTime).rightJustified(COLUMN_WIDTH, ' ')); + statsString += QString(" Average Filter Time: %1 usecs\r\n") + .arg(locale.toString((uint)averageFilterTime).rightJustified(COLUMN_WIDTH, ' ')); int senderNumber = 0; diff --git a/domain-server/resources/describe-settings.json b/domain-server/resources/describe-settings.json index dd0e4ad4a1..f701f35433 100644 --- a/domain-server/resources/describe-settings.json +++ b/domain-server/resources/describe-settings.json @@ -1290,6 +1290,14 @@ "default": "", "advanced": true }, + { + "name": "entityEditFilter", + "label": "Filter Entity Edits", + "help": "Check all entity edits against this filter function.", + "placeholder": "function filter(properties) { return properties; }", + "default": "", + "advanced": true + }, { "name": "persistFilePath", "label": "Entities File Path", diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 4796dda671..c88ff15032 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -918,6 +918,31 @@ void EntityTree::fixupTerseEditLogging(EntityItemProperties& properties, QList Date: Fri, 13 Jan 2017 16:59:51 -0800 Subject: [PATCH 02/12] getting there, but not yet complete --- libraries/entities/src/EntityItemProperties.cpp | 4 ++-- libraries/entities/src/EntityItemProperties.h | 2 +- libraries/entities/src/EntityTree.cpp | 11 ++++++----- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 927708fc4b..788217b847 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -346,11 +346,11 @@ EntityPropertyFlags EntityItemProperties::getChangedProperties() const { return changedProperties; } -QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool skipDefaults) const { +QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUknownCreateTime) const { QScriptValue properties = engine->newObject(); EntityItemProperties defaultEntityProperties; - if (_created == UNKNOWN_CREATED_TIME) { + if (_created == UNKNOWN_CREATED_TIME && !allowUknownCreateTime) { // No entity properties can have been set so return without setting any default, zero property values. return properties; } diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index fb08182a2e..0fc8db185b 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -73,7 +73,7 @@ public: EntityTypes::EntityType getType() const { return _type; } void setType(EntityTypes::EntityType type) { _type = type; } - virtual QScriptValue copyToScriptValue(QScriptEngine* engine, bool skipDefaults) const; + virtual QScriptValue copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUknownCreateTime = false) const; virtual void copyFromScriptValue(const QScriptValue& object, bool honorReadOnly); static QScriptValue entityPropertyFlagsToScriptValue(QScriptEngine* engine, const EntityPropertyFlags& flags); diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index c88ff15032..47b59a1c19 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -931,7 +931,7 @@ bool EntityTree::filterProperties(const EntityItemProperties& propertiesIn, Enti wasChanged = false; // not changed return true; // allowed } - QScriptValue inputValues = propertiesIn.copyToScriptValue(&_entityEditFilterEngine, false); + QScriptValue inputValues = propertiesIn.copyToScriptValue(&_entityEditFilterEngine, true, true); QScriptValueList args; args << inputValues; @@ -940,6 +940,7 @@ bool EntityTree::filterProperties(const EntityItemProperties& propertiesIn, Enti propertiesOut.copyFromScriptValue(result, false); wasChanged = result.equals(inputValues); // not changed + qDebug() << "result" << propertiesOut << result.toVariant() << wasChanged; return result.isObject(); // filters should return null or false to completely reject edit or add } @@ -1028,17 +1029,17 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c if (validEditPacket) { startFilter = usecTimestampNow(); - QScriptValue originalValues = properties.copyToScriptValue(&_entityEditFilterEngine, true); bool wasChanged = false; bool allowed = filterProperties(properties, properties, wasChanged); - QScriptValue filteredResult = properties.copyToScriptValue(&_entityEditFilterEngine, true); - if (wasChanged) { + if (!allowed) { + properties = EntityItemProperties(); // Maybe other behavior + } else if (wasChanged) { if (properties.getLastEdited() == UNKNOWN_CREATED_TIME) { properties.setLastEdited(usecTimestampNow()); } properties.setLastEdited(properties.getLastEdited() + LAST_EDITED_SERVERSIDE_BUMP); } - qDebug() << "FIXME changed/allowed" << wasChanged << allowed; + qDebug() << "filtered:" << properties; endFilter = usecTimestampNow(); // search for the entity by EntityItemID From 09a57efaf30bc292701e10c3a57cc2e51a60fff3 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Mon, 16 Jan 2017 12:47:26 -0800 Subject: [PATCH 03/12] checkpoint --- libraries/entities/src/EntityTree.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 47b59a1c19..8da21ea582 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -932,6 +932,7 @@ bool EntityTree::filterProperties(const EntityItemProperties& propertiesIn, Enti return true; // allowed } QScriptValue inputValues = propertiesIn.copyToScriptValue(&_entityEditFilterEngine, true, true); + qDebug() << "input" << propertiesIn << inputValues.toVariant(); QScriptValueList args; args << inputValues; @@ -1032,8 +1033,9 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c bool wasChanged = false; bool allowed = filterProperties(properties, properties, wasChanged); if (!allowed) { - properties = EntityItemProperties(); // Maybe other behavior - } else if (wasChanged) { + properties = EntityItemProperties(); + } + if (!allowed || wasChanged) { if (properties.getLastEdited() == UNKNOWN_CREATED_TIME) { properties.setLastEdited(usecTimestampNow()); } From 2e9b26057ea3e61a18bd3cb0094f6fd1c0d1f691 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 18 Jan 2017 10:27:32 -0800 Subject: [PATCH 04/12] refactor bumpTimestamp, and fix implementation of wasChanged --- libraries/entities/src/EntityTree.cpp | 50 +++++++++++++++++---------- libraries/entities/src/EntityTree.h | 1 + 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 8da21ea582..bb6bf204fb 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -932,17 +932,36 @@ bool EntityTree::filterProperties(const EntityItemProperties& propertiesIn, Enti return true; // allowed } QScriptValue inputValues = propertiesIn.copyToScriptValue(&_entityEditFilterEngine, true, true); - qDebug() << "input" << propertiesIn << inputValues.toVariant(); + auto in = QJsonValue::fromVariant(inputValues.toVariant()); // grab json copy now, because the inputValues might be side effected by the filter. + qDebug() << "fixme filter input" << propertiesIn << inputValues.toVariant() << in; QScriptValueList args; args << inputValues; - QScriptValue result = _entityEditFilterEngine.newObject(); - result = _entityEditFilterFunction.call(_nullObjectForFilter, args); + QScriptValue result = _entityEditFilterFunction.call(_nullObjectForFilter, args); propertiesOut.copyFromScriptValue(result, false); - wasChanged = result.equals(inputValues); // not changed - qDebug() << "result" << propertiesOut << result.toVariant() << wasChanged; - return result.isObject(); // filters should return null or false to completely reject edit or add + bool accepted = result.isObject(); // filters should return null or false to completely reject edit or add + if (accepted) { + // Javascript objects are == only if they are the same object. To compare arbitrary values, we need to use JSON. + + auto out = QJsonValue::fromVariant(result.toVariant()); + bool eq = in == out; + qDebug() << "in:" << in << "out:" << out << "eq:" << eq; + wasChanged = !eq; + } + + qDebug() << "fixme filter result" << propertiesOut << result.toVariant() << "accepted:" << accepted << "changed:" << (accepted && wasChanged); + return accepted; +} + +void EntityTree::bumpTimestamp(EntityItemProperties& properties) { //fixme put class/header + const quint64 LAST_EDITED_SERVERSIDE_BUMP = 1; // usec + // also bump up the lastEdited time of the properties so that the interface that created this edit + // will accept our adjustment to lifetime back into its own entity-tree. + if (properties.getLastEdited() == UNKNOWN_CREATED_TIME) { + properties.setLastEdited(usecTimestampNow()); + } + properties.setLastEdited(properties.getLastEdited() + LAST_EDITED_SERVERSIDE_BUMP); } int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned char* editData, int maxLength, @@ -971,7 +990,6 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c quint64 startFilter = 0, endFilter = 0; quint64 startLogging = 0, endLogging = 0; - const quint64 LAST_EDITED_SERVERSIDE_BUMP = 1; // usec bool suppressDisallowedScript = false; _totalEditMessages++; @@ -1016,12 +1034,7 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c if (properties.getLifetime() == ENTITY_ITEM_IMMORTAL_LIFETIME || properties.getLifetime() > _maxTmpEntityLifetime) { properties.setLifetime(_maxTmpEntityLifetime); - // also bump up the lastEdited time of the properties so that the interface that created this edit - // will accept our adjustment to lifetime back into its own entity-tree. - if (properties.getLastEdited() == UNKNOWN_CREATED_TIME) { - properties.setLastEdited(usecTimestampNow()); - } - properties.setLastEdited(properties.getLastEdited() + LAST_EDITED_SERVERSIDE_BUMP); + bumpTimestamp(properties); } } @@ -1036,12 +1049,13 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c properties = EntityItemProperties(); } if (!allowed || wasChanged) { - if (properties.getLastEdited() == UNKNOWN_CREATED_TIME) { - properties.setLastEdited(usecTimestampNow()); + bumpTimestamp(properties); + if (properties.getSimulationOwner().getID() == senderNode->getUUID()) { + qDebug() << "fixme Suppressing ownership from" << senderNode->getUUID(); + properties.setSimulationOwner(QUuid(), 0); } - properties.setLastEdited(properties.getLastEdited() + LAST_EDITED_SERVERSIDE_BUMP); } - qDebug() << "filtered:" << properties; + qDebug() << "fixme final:" << properties; endFilter = usecTimestampNow(); // search for the entity by EntityItemID @@ -1051,7 +1065,7 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c if (existingEntity && message.getType() == PacketType::EntityEdit) { if (suppressDisallowedScript) { - properties.setLastEdited(properties.getLastEdited() + LAST_EDITED_SERVERSIDE_BUMP); + bumpTimestamp(properties); properties.setScript(existingEntity->getScript()); } diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index d471694023..7cdf1b0c77 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -289,6 +289,7 @@ protected: static bool findInBoxOperation(OctreeElementPointer element, void* extraData); static bool findInFrustumOperation(OctreeElementPointer element, void* extraData); static bool sendEntitiesOperation(OctreeElementPointer element, void* extraData); + static void bumpTimestamp(EntityItemProperties& properties); void notifyNewlyCreatedEntity(const EntityItem& newEntity, const SharedNodePointer& senderNode); From 60eb6dca9d0b283161f603e08ca6dd08692c789d Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 18 Jan 2017 10:29:11 -0800 Subject: [PATCH 05/12] remove debug dump that was printing to screeen during << operator --- libraries/entities/src/EntityItemProperties.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index 0fc8db185b..20491bdaf5 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -460,10 +460,6 @@ inline QDebug operator<<(QDebug debug, const EntityItemProperties& properties) { DEBUG_PROPERTY_IF_CHANGED(debug, properties, LastEditedBy, lastEditedBy, ""); - properties.getAnimation().debugDump(); - properties.getSkybox().debugDump(); - properties.getStage().debugDump(); - debug << " last edited:" << properties.getLastEdited() << "\n"; debug << " edited ago:" << properties.getEditedAgo() << "\n"; debug << "]"; From 00e53c8ea6b07a0c290d277802238303d84a789e Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 18 Jan 2017 13:21:28 -0800 Subject: [PATCH 06/12] more tractable copy semantics --- libraries/entities/src/EntityItemProperties.cpp | 16 ++++++++++------ libraries/entities/src/EntityItemProperties.h | 4 +++- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 788217b847..a16a4fbaba 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -346,7 +346,11 @@ EntityPropertyFlags EntityItemProperties::getChangedProperties() const { return changedProperties; } -QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUknownCreateTime) const { +QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUknownCreateTime, bool strictSemantics) const { + // If strictSemantics is true and skipDefaults is false, then all and only those properties are copied in the property flag + // is included in _desiredProperties, or is one of the specially enumerated ALWAYS properties below. + // (There may be exceptions, but if so, they are bugs.) + // In all other cases, you are welcome to inspect the code and try to figure out what was intended. I wish you luck. -HRS 1/18/17 QScriptValue properties = engine->newObject(); EntityItemProperties defaultEntityProperties; @@ -364,7 +368,7 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool created.setTimeSpec(Qt::OffsetFromUTC); COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER_ALWAYS(created, created.toString(Qt::ISODate)); - if (!skipDefaults || _lifetime != defaultEntityProperties._lifetime) { + if ((!skipDefaults || _lifetime != defaultEntityProperties._lifetime) && !strictSemantics) { COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER_NO_SKIP(age, getAge()); // gettable, but not settable COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER_NO_SKIP(ageAsText, formatSecondsElapsed(getAge())); // gettable, but not settable } @@ -539,7 +543,7 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool } // Sitting properties support - if (!skipDefaults) { + if (!skipDefaults && !strictSemantics) { QScriptValue sittingPoints = engine->newObject(); for (int i = 0; i < _sittingPoints.size(); ++i) { QScriptValue sittingPoint = engine->newObject(); @@ -552,7 +556,7 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER_ALWAYS(sittingPoints, sittingPoints); // gettable, but not settable } - if (!skipDefaults) { + if (!skipDefaults && !strictSemantics) { AABox aaBox = getAABox(); QScriptValue boundingBox = engine->newObject(); QScriptValue bottomRightNear = vec3toScriptValue(engine, aaBox.getCorner()); @@ -567,7 +571,7 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool } QString textureNamesStr = QJsonDocument::fromVariant(_textureNames).toJson(); - if (!skipDefaults) { + if (!skipDefaults && !strictSemantics) { COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER_NO_SKIP(originalTextures, textureNamesStr); // gettable, but not settable } @@ -584,7 +588,7 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool COPY_PROPERTY_TO_QSCRIPTVALUE(PROP_OWNING_AVATAR_ID, owningAvatarID); // Rendering info - if (!skipDefaults) { + if (!skipDefaults && !strictSemantics) { QScriptValue renderInfo = engine->newObject(); // currently only supported by models diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index 20491bdaf5..dd81792283 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -73,7 +73,7 @@ public: EntityTypes::EntityType getType() const { return _type; } void setType(EntityTypes::EntityType type) { _type = type; } - virtual QScriptValue copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUknownCreateTime = false) const; + virtual QScriptValue copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUknownCreateTime = false, bool strictSemantics = false) const; virtual void copyFromScriptValue(const QScriptValue& object, bool honorReadOnly); static QScriptValue entityPropertyFlagsToScriptValue(QScriptEngine* engine, const EntityPropertyFlags& flags); @@ -93,6 +93,8 @@ public: void debugDump() const; void setLastEdited(quint64 usecTime); + EntityPropertyFlags getDesiredProperties() { return _desiredProperties; } + void setDesiredProperties(EntityPropertyFlags properties) { _desiredProperties = properties; } // Note: DEFINE_PROPERTY(PROP_FOO, Foo, foo, type, value) creates the following methods and variables: // type getFoo() const; From 957b1d9ae434d514acca36da50c0261204133613 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 18 Jan 2017 13:22:54 -0800 Subject: [PATCH 07/12] pass just what is given to filter, more or less --- libraries/entities/src/EntityTree.cpp | 11 ++++++++--- libraries/entities/src/EntityTree.h | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index bb6bf204fb..8f5df26c17 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -925,13 +925,18 @@ void EntityTree::initEntityEditFilterEngine() { _hasEntityEditFilter = _entityEditFilterFunction.isFunction(); } -bool EntityTree::filterProperties(const EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged) { +bool EntityTree::filterProperties(EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged) { if (!_hasEntityEditFilter) { propertiesOut = propertiesIn; wasChanged = false; // not changed return true; // allowed } - QScriptValue inputValues = propertiesIn.copyToScriptValue(&_entityEditFilterEngine, true, true); + auto oldProperties = propertiesIn.getDesiredProperties(); + auto specifiedProperties = propertiesIn.getChangedProperties(); + propertiesIn.setDesiredProperties(specifiedProperties); + QScriptValue inputValues = propertiesIn.copyToScriptValue(&_entityEditFilterEngine, false, true, true); + propertiesIn.setDesiredProperties(oldProperties); + auto in = QJsonValue::fromVariant(inputValues.toVariant()); // grab json copy now, because the inputValues might be side effected by the filter. qDebug() << "fixme filter input" << propertiesIn << inputValues.toVariant() << in; QScriptValueList args; @@ -950,7 +955,7 @@ bool EntityTree::filterProperties(const EntityItemProperties& propertiesIn, Enti wasChanged = !eq; } - qDebug() << "fixme filter result" << propertiesOut << result.toVariant() << "accepted:" << accepted << "changed:" << (accepted && wasChanged); + qDebug() << "fixme filter accepted:" << accepted << " changed : " << (accepted && wasChanged) << " result:" << propertiesOut << result.toVariant(); return accepted; } diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index 7cdf1b0c77..29c51560fa 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -352,7 +352,7 @@ protected: float _maxTmpEntityLifetime { DEFAULT_MAX_TMP_ENTITY_LIFETIME }; - bool filterProperties(const EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged); + bool filterProperties(EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged); QString _entityEditFilter; bool _hasEntityEditFilter{ false }; QScriptEngine _entityEditFilterEngine; From 90c9c5b137dd1d3495534c26c68ff66252bad8c4 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Wed, 18 Jan 2017 14:32:05 -0800 Subject: [PATCH 08/12] logging change --- libraries/entities/src/EntityTree.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 8f5df26c17..e6dc0de185 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -938,7 +938,6 @@ bool EntityTree::filterProperties(EntityItemProperties& propertiesIn, EntityItem propertiesIn.setDesiredProperties(oldProperties); auto in = QJsonValue::fromVariant(inputValues.toVariant()); // grab json copy now, because the inputValues might be side effected by the filter. - qDebug() << "fixme filter input" << propertiesIn << inputValues.toVariant() << in; QScriptValueList args; args << inputValues; @@ -950,12 +949,17 @@ bool EntityTree::filterProperties(EntityItemProperties& propertiesIn, EntityItem // Javascript objects are == only if they are the same object. To compare arbitrary values, we need to use JSON. auto out = QJsonValue::fromVariant(result.toVariant()); - bool eq = in == out; - qDebug() << "in:" << in << "out:" << out << "eq:" << eq; - wasChanged = !eq; + wasChanged = in != out; + if (wasChanged) { + // Logging will be removed eventually, but for now, the behavior is so fragile that it's worth logging. + qCDebug(entities) << "filter accepted. changed:" << wasChanged; + qCDebug(entities) << " in:" << in; + qCDebug(entities) << " out:" << out; + } + } else { + qCDebug(entities) << "filter rejected. in:" << in; } - qDebug() << "fixme filter accepted:" << accepted << " changed : " << (accepted && wasChanged) << " result:" << propertiesOut << result.toVariant(); return accepted; } @@ -1056,11 +1060,9 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c if (!allowed || wasChanged) { bumpTimestamp(properties); if (properties.getSimulationOwner().getID() == senderNode->getUUID()) { - qDebug() << "fixme Suppressing ownership from" << senderNode->getUUID(); properties.setSimulationOwner(QUuid(), 0); } } - qDebug() << "fixme final:" << properties; endFilter = usecTimestampNow(); // search for the entity by EntityItemID From 7b986dcac64f2b5aafb5a8cb1ea270ce48db64ee Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Fri, 20 Jan 2017 17:03:52 -0800 Subject: [PATCH 09/12] works, but removes an optimization we'd like to keep --- libraries/entities/src/EntityItem.cpp | 4 +++- libraries/entities/src/EntityTree.cpp | 7 +++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 00c9ab2dde..a295319857 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -349,7 +349,6 @@ int EntityItem::expectedBytes() { return MINIMUM_HEADER_BYTES; } - // clients use this method to unpack FULL updates from entity-server int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLeftToRead, ReadBitstreamToTreeParams& args) { if (args.bitstreamVersion < VERSION_ENTITIES_SUPPORT_SPLIT_MTU) { @@ -667,6 +666,9 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef // entity-server is trying to clear our ownership (probably at our own request) // but we actually want to own it, therefore we ignore this clear event // and pretend that we own it (we assume we'll recover it soon) + + // However, for now, when the server uses a newer time than what we sent, listen to what we're told. + if (overwriteLocalData) weOwnSimulation = false; } else if (_simulationOwner.set(newSimOwner)) { _dirtyFlags |= Simulation::DIRTY_SIMULATOR_ID; somethingChanged = true; diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index e6dc0de185..9d5d14402d 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -952,7 +952,7 @@ bool EntityTree::filterProperties(EntityItemProperties& propertiesIn, EntityItem wasChanged = in != out; if (wasChanged) { // Logging will be removed eventually, but for now, the behavior is so fragile that it's worth logging. - qCDebug(entities) << "filter accepted. changed:" << wasChanged; + qCDebug(entities) << "filter accepted. changed: true"; qCDebug(entities) << " in:" << in; qCDebug(entities) << " out:" << out; } @@ -1059,9 +1059,8 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c } if (!allowed || wasChanged) { bumpTimestamp(properties); - if (properties.getSimulationOwner().getID() == senderNode->getUUID()) { - properties.setSimulationOwner(QUuid(), 0); - } + // For now, free ownership on any modification. + properties.clearSimulationOwner(); } endFilter = usecTimestampNow(); From f34f90841d01d5bfd2d7664992f4e9bffe323578 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Mon, 23 Jan 2017 13:08:33 -0800 Subject: [PATCH 10/12] example --- script-archive/entity-server-filter-example.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 script-archive/entity-server-filter-example.js diff --git a/script-archive/entity-server-filter-example.js b/script-archive/entity-server-filter-example.js new file mode 100644 index 0000000000..2ccb550acd --- /dev/null +++ b/script-archive/entity-server-filter-example.js @@ -0,0 +1,15 @@ +function filter(p) { + /* block comments are ok, but not double-slash end-of-line-comments */ + + /* Simple example: if someone specifies name, add an 'x' to it. Note that print is ok to use. */ + if (p.name) {p.name += 'x'; print('fixme name', p. name);} + + /* This example clamps y. A better filter would probably zero y component of velocity and acceleration. */ + if (p.position) {p.position.y = Math.min(1, p.position.y); print('fixme p.y', p.position.y);} + + /* Can also reject altogether */ + if (p.userData) { return false; } + + /* If we make it this far, return the (possibly modified) properties. */ + return p; +} From 0a4634446d217e1ab12ed17f20274e8eb0230078 Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Mon, 23 Jan 2017 13:09:28 -0800 Subject: [PATCH 11/12] typo --- libraries/entities/src/EntityItemProperties.cpp | 4 ++-- libraries/entities/src/EntityItemProperties.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index a16a4fbaba..925eb5d351 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -346,7 +346,7 @@ EntityPropertyFlags EntityItemProperties::getChangedProperties() const { return changedProperties; } -QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUknownCreateTime, bool strictSemantics) const { +QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUnkownCreateTime, bool strictSemantics) const { // If strictSemantics is true and skipDefaults is false, then all and only those properties are copied in the property flag // is included in _desiredProperties, or is one of the specially enumerated ALWAYS properties below. // (There may be exceptions, but if so, they are bugs.) @@ -354,7 +354,7 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool QScriptValue properties = engine->newObject(); EntityItemProperties defaultEntityProperties; - if (_created == UNKNOWN_CREATED_TIME && !allowUknownCreateTime) { + if (_created == UNKNOWN_CREATED_TIME && !allowUnkownCreateTime) { // No entity properties can have been set so return without setting any default, zero property values. return properties; } diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index dd81792283..bc7263bd8e 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -73,7 +73,7 @@ public: EntityTypes::EntityType getType() const { return _type; } void setType(EntityTypes::EntityType type) { _type = type; } - virtual QScriptValue copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUknownCreateTime = false, bool strictSemantics = false) const; + virtual QScriptValue copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUnkownCreateTime = false, bool strictSemantics = false) const; virtual void copyFromScriptValue(const QScriptValue& object, bool honorReadOnly); static QScriptValue entityPropertyFlagsToScriptValue(QScriptEngine* engine, const EntityPropertyFlags& flags); From ada685a376b61c119d969b06d7ee09c20ae618bd Mon Sep 17 00:00:00 2001 From: howard-stearns Date: Mon, 23 Jan 2017 15:31:55 -0800 Subject: [PATCH 12/12] cr changes --- libraries/entities/src/EntityItemProperties.cpp | 6 +++--- libraries/entities/src/EntityItemProperties.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 925eb5d351..77a0209143 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -346,15 +346,15 @@ EntityPropertyFlags EntityItemProperties::getChangedProperties() const { return changedProperties; } -QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUnkownCreateTime, bool strictSemantics) const { - // If strictSemantics is true and skipDefaults is false, then all and only those properties are copied in the property flag +QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUnknownCreateTime, bool strictSemantics) const { + // If strictSemantics is true and skipDefaults is false, then all and only those properties are copied for which the property flag // is included in _desiredProperties, or is one of the specially enumerated ALWAYS properties below. // (There may be exceptions, but if so, they are bugs.) // In all other cases, you are welcome to inspect the code and try to figure out what was intended. I wish you luck. -HRS 1/18/17 QScriptValue properties = engine->newObject(); EntityItemProperties defaultEntityProperties; - if (_created == UNKNOWN_CREATED_TIME && !allowUnkownCreateTime) { + if (_created == UNKNOWN_CREATED_TIME && !allowUnknownCreateTime) { // No entity properties can have been set so return without setting any default, zero property values. return properties; } diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index bc7263bd8e..d8b148bdb6 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -73,7 +73,7 @@ public: EntityTypes::EntityType getType() const { return _type; } void setType(EntityTypes::EntityType type) { _type = type; } - virtual QScriptValue copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUnkownCreateTime = false, bool strictSemantics = false) const; + virtual QScriptValue copyToScriptValue(QScriptEngine* engine, bool skipDefaults, bool allowUnknownCreateTime = false, bool strictSemantics = false) const; virtual void copyFromScriptValue(const QScriptValue& object, bool honorReadOnly); static QScriptValue entityPropertyFlagsToScriptValue(QScriptEngine* engine, const EntityPropertyFlags& flags);