From 61e558e5687c0d1bb6b8eab727a0733b141222c4 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Wed, 8 Feb 2017 09:53:25 -0700 Subject: [PATCH 01/19] Entity Edit Filters for zones Part 1 - just put them in the zones entities. Next I'll hook them up and actually filter... --- .../entities/src/EntityItemProperties.cpp | 14 +++++++++++-- libraries/entities/src/EntityItemProperties.h | 2 ++ libraries/entities/src/EntityPropertyFlags.h | 2 ++ libraries/entities/src/ZoneEntityItem.cpp | 20 ++++++++++++++++++- libraries/entities/src/ZoneEntityItem.h | 5 +++++ scripts/system/html/entityProperties.html | 5 +++++ scripts/system/html/js/entityProperties.js | 5 ++++- 7 files changed, 49 insertions(+), 4 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 21018d8afa..edfabf84e9 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -332,6 +332,7 @@ EntityPropertyFlags EntityItemProperties::getChangedProperties() const { CHECK_PROPERTY_CHANGE(PROP_FLYING_ALLOWED, flyingAllowed); CHECK_PROPERTY_CHANGE(PROP_GHOSTING_ALLOWED, ghostingAllowed); + CHECK_PROPERTY_CHANGE(PROP_FILTER_URL, filterURL); CHECK_PROPERTY_CHANGE(PROP_CLIENT_ONLY, clientOnly); CHECK_PROPERTY_CHANGE(PROP_OWNING_AVATAR_ID, owningAvatarID); @@ -509,6 +510,7 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool COPY_PROPERTY_TO_QSCRIPTVALUE(PROP_FLYING_ALLOWED, flyingAllowed); COPY_PROPERTY_TO_QSCRIPTVALUE(PROP_GHOSTING_ALLOWED, ghostingAllowed); + COPY_PROPERTY_TO_QSCRIPTVALUE(PROP_FILTER_URL, filterURL); } // Web only @@ -751,6 +753,7 @@ void EntityItemProperties::copyFromScriptValue(const QScriptValue& object, bool COPY_PROPERTY_FROM_QSCRIPTVALUE(flyingAllowed, bool, setFlyingAllowed); COPY_PROPERTY_FROM_QSCRIPTVALUE(ghostingAllowed, bool, setGhostingAllowed); + COPY_PROPERTY_FROM_QSCRIPTVALUE(filterURL, QString, setFilterURL); COPY_PROPERTY_FROM_QSCRIPTVALUE(clientOnly, bool, setClientOnly); COPY_PROPERTY_FROM_QSCRIPTVALUE(owningAvatarID, QUuid, setOwningAvatarID); @@ -879,6 +882,7 @@ void EntityItemProperties::merge(const EntityItemProperties& other) { COPY_PROPERTY_IF_CHANGED(flyingAllowed); COPY_PROPERTY_IF_CHANGED(ghostingAllowed); + COPY_PROPERTY_IF_CHANGED(filterURL); COPY_PROPERTY_IF_CHANGED(clientOnly); COPY_PROPERTY_IF_CHANGED(owningAvatarID); @@ -1063,6 +1067,7 @@ void EntityItemProperties::entityPropertyFlagsFromScriptValue(const QScriptValue ADD_PROPERTY_TO_MAP(PROP_FLYING_ALLOWED, FlyingAllowed, flyingAllowed, bool); ADD_PROPERTY_TO_MAP(PROP_GHOSTING_ALLOWED, GhostingAllowed, ghostingAllowed, bool); + ADD_PROPERTY_TO_MAP(PROP_FILTER_URL, FilterURL, filterURL, QString); ADD_PROPERTY_TO_MAP(PROP_DPI, DPI, dpi, uint16_t); @@ -1311,6 +1316,7 @@ bool EntityItemProperties::encodeEntityEditPacket(PacketType command, EntityItem APPEND_ENTITY_PROPERTY(PROP_FLYING_ALLOWED, properties.getFlyingAllowed()); APPEND_ENTITY_PROPERTY(PROP_GHOSTING_ALLOWED, properties.getGhostingAllowed()); + APPEND_ENTITY_PROPERTY(PROP_FILTER_URL, properties.getFilterURL()); } if (properties.getType() == EntityTypes::PolyVox) { @@ -1605,6 +1611,7 @@ bool EntityItemProperties::decodeEntityEditPacket(const unsigned char* data, int READ_ENTITY_PROPERTY_TO_PROPERTIES(PROP_FLYING_ALLOWED, bool, setFlyingAllowed); READ_ENTITY_PROPERTY_TO_PROPERTIES(PROP_GHOSTING_ALLOWED, bool, setGhostingAllowed); + READ_ENTITY_PROPERTY_TO_PROPERTIES(PROP_FILTER_URL, QString, setFilterURL); } if (properties.getType() == EntityTypes::PolyVox) { @@ -1798,7 +1805,7 @@ void EntityItemProperties::markAllChanged() { _parentIDChanged = true; _parentJointIndexChanged = true; - + _jointRotationsSetChanged = true; _jointRotationsChanged = true; _jointTranslationsSetChanged = true; @@ -1808,6 +1815,7 @@ void EntityItemProperties::markAllChanged() { _flyingAllowedChanged = true; _ghostingAllowedChanged = true; + _filterURLChanged = true; _clientOnlyChanged = true; _owningAvatarIDChanged = true; @@ -2150,7 +2158,9 @@ QList EntityItemProperties::listChangedProperties() { if (ghostingAllowedChanged()) { out += "ghostingAllowed"; } - + if (filterURLChanged()) { + out += "filterURL"; + } if (dpiChanged()) { out += "dpi"; } diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index 1961feaf1d..419740e4ea 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -215,6 +215,7 @@ public: DEFINE_PROPERTY(PROP_FLYING_ALLOWED, FlyingAllowed, flyingAllowed, bool, ZoneEntityItem::DEFAULT_FLYING_ALLOWED); DEFINE_PROPERTY(PROP_GHOSTING_ALLOWED, GhostingAllowed, ghostingAllowed, bool, ZoneEntityItem::DEFAULT_GHOSTING_ALLOWED); + DEFINE_PROPERTY(PROP_FILTER_URL, FilterURL, filterURL, QString, ZoneEntityItem::DEFAULT_FILTER_URL); DEFINE_PROPERTY(PROP_CLIENT_ONLY, ClientOnly, clientOnly, bool, false); DEFINE_PROPERTY_REF(PROP_OWNING_AVATAR_ID, OwningAvatarID, owningAvatarID, QUuid, UNKNOWN_ENTITY_ID); @@ -458,6 +459,7 @@ inline QDebug operator<<(QDebug debug, const EntityItemProperties& properties) { DEBUG_PROPERTY_IF_CHANGED(debug, properties, FlyingAllowed, flyingAllowed, ""); DEBUG_PROPERTY_IF_CHANGED(debug, properties, GhostingAllowed, ghostingAllowed, ""); + DEBUG_PROPERTY_IF_CHANGED(debug, properties, FilterURL, filterURL, ""); DEBUG_PROPERTY_IF_CHANGED(debug, properties, ClientOnly, clientOnly, ""); DEBUG_PROPERTY_IF_CHANGED(debug, properties, OwningAvatarID, owningAvatarID, ""); diff --git a/libraries/entities/src/EntityPropertyFlags.h b/libraries/entities/src/EntityPropertyFlags.h index b77d3cc077..b3cfc143c2 100644 --- a/libraries/entities/src/EntityPropertyFlags.h +++ b/libraries/entities/src/EntityPropertyFlags.h @@ -185,6 +185,8 @@ enum EntityPropertyList { PROP_SERVER_SCRIPTS, + PROP_FILTER_URL, + //////////////////////////////////////////////////////////////////////////////////////////////////// // ATTENTION: add new properties to end of list just ABOVE this line PROP_AFTER_LAST_ITEM, diff --git a/libraries/entities/src/ZoneEntityItem.cpp b/libraries/entities/src/ZoneEntityItem.cpp index 3e21497d63..7de3f6b68f 100644 --- a/libraries/entities/src/ZoneEntityItem.cpp +++ b/libraries/entities/src/ZoneEntityItem.cpp @@ -28,7 +28,7 @@ const ShapeType ZoneEntityItem::DEFAULT_SHAPE_TYPE = SHAPE_TYPE_BOX; const QString ZoneEntityItem::DEFAULT_COMPOUND_SHAPE_URL = ""; const bool ZoneEntityItem::DEFAULT_FLYING_ALLOWED = true; const bool ZoneEntityItem::DEFAULT_GHOSTING_ALLOWED = true; - +const QString ZoneEntityItem::DEFAULT_FILTER_URL = ""; EntityItemPointer ZoneEntityItem::factory(const EntityItemID& entityID, const EntityItemProperties& properties) { EntityItemPointer entity { new ZoneEntityItem(entityID) }; @@ -61,6 +61,7 @@ EntityItemProperties ZoneEntityItem::getProperties(EntityPropertyFlags desiredPr COPY_ENTITY_PROPERTY_TO_PROPERTIES(flyingAllowed, getFlyingAllowed); COPY_ENTITY_PROPERTY_TO_PROPERTIES(ghostingAllowed, getGhostingAllowed); + COPY_ENTITY_PROPERTY_TO_PROPERTIES(filterURL, getFilterURL); return properties; } @@ -79,6 +80,7 @@ bool ZoneEntityItem::setProperties(const EntityItemProperties& properties) { SET_ENTITY_PROPERTY_FROM_PROPERTIES(flyingAllowed, setFlyingAllowed); SET_ENTITY_PROPERTY_FROM_PROPERTIES(ghostingAllowed, setGhostingAllowed); + SET_ENTITY_PROPERTY_FROM_PROPERTIES(filterURL, setFilterURL); bool somethingChangedInSkybox = _skyboxProperties.setProperties(properties); @@ -128,6 +130,7 @@ int ZoneEntityItem::readEntitySubclassDataFromBuffer(const unsigned char* data, READ_ENTITY_PROPERTY(PROP_FLYING_ALLOWED, bool, setFlyingAllowed); READ_ENTITY_PROPERTY(PROP_GHOSTING_ALLOWED, bool, setGhostingAllowed); + READ_ENTITY_PROPERTY(PROP_FILTER_URL, QString, setFilterURL); return bytesRead; } @@ -147,6 +150,7 @@ EntityPropertyFlags ZoneEntityItem::getEntityProperties(EncodeBitstreamParams& p requestedProperties += PROP_FLYING_ALLOWED; requestedProperties += PROP_GHOSTING_ALLOWED; + requestedProperties += PROP_FILTER_URL; return requestedProperties; } @@ -177,6 +181,7 @@ void ZoneEntityItem::appendSubclassData(OctreePacketData* packetData, EncodeBits APPEND_ENTITY_PROPERTY(PROP_FLYING_ALLOWED, getFlyingAllowed()); APPEND_ENTITY_PROPERTY(PROP_GHOSTING_ALLOWED, getGhostingAllowed()); + APPEND_ENTITY_PROPERTY(PROP_FILTER_URL, getFilterURL()); } void ZoneEntityItem::debugDump() const { @@ -215,3 +220,16 @@ bool ZoneEntityItem::findDetailedRayIntersection(const glm::vec3& origin, const return _zonesArePickable; } + +bool ZoneEntityItem::containsPoint(glm::vec3& position) { + // use _shapeType shortly + // for now bounding box just so I can get end-to-end working + bool success; + AABox box = getAABox(success); + if (success) { + return box.contains(position); + } + // just return false if no AABox + return success; +} + diff --git a/libraries/entities/src/ZoneEntityItem.h b/libraries/entities/src/ZoneEntityItem.h index 3084d71f46..07520001f7 100644 --- a/libraries/entities/src/ZoneEntityItem.h +++ b/libraries/entities/src/ZoneEntityItem.h @@ -74,6 +74,9 @@ public: void setFlyingAllowed(bool value) { _flyingAllowed = value; } bool getGhostingAllowed() const { return _ghostingAllowed; } void setGhostingAllowed(bool value) { _ghostingAllowed = value; } + QString getFilterURL() const { return _filterURL; } + void setFilterURL(const QString url) { _filterURL = url; } + bool containsPoint(glm::vec3& position); virtual bool supportsDetailedRayIntersection() const override { return true; } virtual bool findDetailedRayIntersection(const glm::vec3& origin, const glm::vec3& direction, @@ -87,6 +90,7 @@ public: static const QString DEFAULT_COMPOUND_SHAPE_URL; static const bool DEFAULT_FLYING_ALLOWED; static const bool DEFAULT_GHOSTING_ALLOWED; + static const QString DEFAULT_FILTER_URL; protected: KeyLightPropertyGroup _keyLightProperties; @@ -101,6 +105,7 @@ protected: bool _flyingAllowed { DEFAULT_FLYING_ALLOWED }; bool _ghostingAllowed { DEFAULT_GHOSTING_ALLOWED }; + QString _filterURL { DEFAULT_FILTER_URL }; static bool _drawZoneBoundaries; static bool _zonesArePickable; diff --git a/scripts/system/html/entityProperties.html b/scripts/system/html/entityProperties.html index 1fca14c2bc..d28416eb18 100644 --- a/scripts/system/html/entityProperties.html +++ b/scripts/system/html/entityProperties.html @@ -449,6 +449,11 @@ +
+
+ + +
diff --git a/scripts/system/html/js/entityProperties.js b/scripts/system/html/js/entityProperties.js index 6b3bdaa0a4..98784d8e96 100644 --- a/scripts/system/html/js/entityProperties.js +++ b/scripts/system/html/js/entityProperties.js @@ -697,6 +697,7 @@ function loaded() { var elZoneFlyingAllowed = document.getElementById("property-zone-flying-allowed"); var elZoneGhostingAllowed = document.getElementById("property-zone-ghosting-allowed"); + var elZoneFilterURL = document.getElementById("property-zone-filter-url"); var elPolyVoxSections = document.querySelectorAll(".poly-vox-section"); allSections.push(elPolyVoxSections); @@ -1034,6 +1035,7 @@ function loaded() { elZoneFlyingAllowed.checked = properties.flyingAllowed; elZoneGhostingAllowed.checked = properties.ghostingAllowed; + elZoneFilterURL.value = properties.filterURL; showElements(document.getElementsByClassName('skybox-section'), elZoneBackgroundMode.value == 'skybox'); } else if (properties.type == "PolyVox") { @@ -1385,7 +1387,8 @@ function loaded() { elZoneFlyingAllowed.addEventListener('change', createEmitCheckedPropertyUpdateFunction('flyingAllowed')); elZoneGhostingAllowed.addEventListener('change', createEmitCheckedPropertyUpdateFunction('ghostingAllowed')); - + elZoneFilterURL.addEventListener('change', createEmitTextPropertyUpdateFunction('filterURL')); + var voxelVolumeSizeChangeFunction = createEmitVec3PropertyUpdateFunction( 'voxelVolumeSize', elVoxelVolumeSizeX, elVoxelVolumeSizeY, elVoxelVolumeSizeZ); elVoxelVolumeSizeX.addEventListener('change', voxelVolumeSizeChangeFunction); From 4821180dd3a6dc6805eb6aefbb01a323cbdad027 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Wed, 8 Feb 2017 10:48:26 -0700 Subject: [PATCH 02/19] Just add a pointer to the filter class but do nothing with it --- libraries/entities/src/EntityTree.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index 5dad282d3b..7d7948844e 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -24,6 +24,7 @@ typedef std::shared_ptr EntityTreePointer; #include "EntityTreeElement.h" #include "DeleteEntityOperator.h" +#include "EntityEditFilters.h" class Model; using ModelPointer = std::shared_ptr; @@ -370,6 +371,7 @@ protected: std::function _entityEditFilterHadUncaughtExceptions; QStringList _entityScriptSourceWhitelist; + EntityEditFilters* _entityEditFilters; }; #endif // hifi_EntityTree_h From c08adc9faa2c67a4a40ab9a261e2efbfa69b9c29 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Wed, 8 Feb 2017 10:51:28 -0700 Subject: [PATCH 03/19] and the new filter class --- libraries/entities/src/EntityEditFilters.cpp | 138 +++++++++++++++++++ libraries/entities/src/EntityEditFilters.h | 46 +++++++ 2 files changed, 184 insertions(+) create mode 100644 libraries/entities/src/EntityEditFilters.cpp create mode 100644 libraries/entities/src/EntityEditFilters.h diff --git a/libraries/entities/src/EntityEditFilters.cpp b/libraries/entities/src/EntityEditFilters.cpp new file mode 100644 index 0000000000..a74ba091b5 --- /dev/null +++ b/libraries/entities/src/EntityEditFilters.cpp @@ -0,0 +1,138 @@ +// +// EntityEditFilters.cpp +// libraries/entities/src +// +// Created by David Kelly on 2/7/2017. +// Copyright 2017 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + + +#include + +#include + +#include "EntityEditFilters.h" + +QScriptValue EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, bool isAdd) { + qDebug() << "in EntityEditFilters"; + return QScriptValue(); +} + +void EntityEditFilters::removeEntityFilter(EntityItemID& entityID) { + QScriptEngine* engine = _filterScriptEngineMap.value(entityID); + if (engine) { + _filterScriptEngineMap.remove(entityID); + delete engine; + } + FilterFunctionPair* pair = _filterFunctionMap.value(entityID); + if (pair) { + _filterFunctionMap.remove(entityID); + delete pair; + } +} + +void EntityEditFilters::addEntityFilter(EntityItemID& entityID, QString filterURL) { + QUrl scriptURL(filterURL); + + // The following should be abstracted out for use in Agent.cpp (and maybe later AvatarMixer.cpp) + if (scriptURL.scheme().isEmpty() || (scriptURL.scheme() == URL_SCHEME_FILE)) { + qWarning() << "Cannot load script from local filesystem, because assignment may be on a different computer."; + scriptRequestFinished(entityID); + return; + } + // first remove any existing info for this entity + removeEntityFilter(entityID); + + auto scriptRequest = ResourceManager::createResourceRequest(this, scriptURL); + if (!scriptRequest) { + qWarning() << "Could not create ResourceRequest for Entity Edit filter script at" << scriptURL.toString(); + scriptRequestFinished(entityID); + return; + } + // Agent.cpp sets up a timeout here, but that is unnecessary, as ResourceRequest has its own. + connect(scriptRequest, &ResourceRequest::finished, this, [this, &entityID]{ EntityEditFilters::scriptRequestFinished(entityID);} ); + // FIXME: handle atp rquests setup here. See Agent::requestScript() + qInfo() << "Requesting script at URL" << qPrintable(scriptRequest->getUrl().toString()); + scriptRequest->send(); + qDebug() << "script request sent"; + +} + +// Copied from ScriptEngine.cpp. We should make this a class method for reuse. +// Note: I've deliberately stopped short of using ScriptEngine instead of QScriptEngine, as that is out of project scope at this point. +static bool hasCorrectSyntax(const QScriptProgram& program) { + const auto syntaxCheck = QScriptEngine::checkSyntax(program.sourceCode()); + if (syntaxCheck.state() != QScriptSyntaxCheckResult::Valid) { + const auto error = syntaxCheck.errorMessage(); + const auto line = QString::number(syntaxCheck.errorLineNumber()); + const auto column = QString::number(syntaxCheck.errorColumnNumber()); + const auto message = QString("[SyntaxError] %1 in %2:%3(%4)").arg(error, program.fileName(), line, column); + qCritical() << qPrintable(message); + return false; + } + return true; +} +static bool hadUncaughtExceptions(QScriptEngine& engine, const QString& fileName) { + if (engine.hasUncaughtException()) { + const auto backtrace = engine.uncaughtExceptionBacktrace(); + const auto exception = engine.uncaughtException().toString(); + const auto line = QString::number(engine.uncaughtExceptionLineNumber()); + engine.clearExceptions(); + + static const QString SCRIPT_EXCEPTION_FORMAT = "[UncaughtException] %1 in %2:%3"; + auto message = QString(SCRIPT_EXCEPTION_FORMAT).arg(exception, fileName, line); + if (!backtrace.empty()) { + static const auto lineSeparator = "\n "; + message += QString("\n[Backtrace]%1%2").arg(lineSeparator, backtrace.join(lineSeparator)); + } + qCritical() << qPrintable(message); + return true; + } + return false; +} + +void EntityEditFilters::scriptRequestFinished(EntityItemID& entityID) { + qDebug() << "script request completed"; + auto scriptRequest = qobject_cast(sender()); + const QString urlString = scriptRequest->getUrl().toString(); + if (scriptRequest && scriptRequest->getResult() == ResourceRequest::Success) { + auto scriptContents = scriptRequest->getData(); + qInfo() << "Downloaded script:" << scriptContents; + QScriptProgram program(scriptContents, urlString); + if (hasCorrectSyntax(program)) { + // create a QScriptEngine for this script + QScriptEngine* engine = new QScriptEngine(); + engine->evaluate(scriptContents); + if (!hadUncaughtExceptions(*engine, urlString)) { + // put the engine in the engine map (so we don't leak them, etc...) + _filterScriptEngineMap.insert(entityID, engine); + + // define the uncaughtException function + FilterFunctionPair* pair = new FilterFunctionPair(); + QScriptEngine& engineRef = *engine; + pair->second = [this, &engineRef, &urlString]() { return hadUncaughtExceptions(engineRef, urlString); }; + + // now get the filter function + auto global = engine->globalObject(); + pair->first = global.property("filter"); + if (!pair->first.isFunction()) { + qDebug() << "Filter function specified but not found. Ignoring filter"; + return; + } + _filterFunctionMap.insert(entityID, pair); + qDebug() << "script request filter processed"; + return; + } + } + } else if (scriptRequest) { + qCritical() << "Failed to download script at" << urlString; + // See HTTPResourceRequest::onRequestFinished for interpretation of codes. For example, a 404 is code 6 and 403 is 3. A timeout is 2. Go figure. + qCritical() << "ResourceRequest error was" << scriptRequest->getResult(); + } else { + qCritical() << "Failed to create script request."; + } + +} diff --git a/libraries/entities/src/EntityEditFilters.h b/libraries/entities/src/EntityEditFilters.h new file mode 100644 index 0000000000..678b3ad0d1 --- /dev/null +++ b/libraries/entities/src/EntityEditFilters.h @@ -0,0 +1,46 @@ +// +// EntityEditFilters.h +// libraries/entities/src +// +// Created by David Kelly on 2/7/2017. +// Copyright 2017 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// +#ifndef hifi_EntityEditFilters_h +#define hifi_EntityEditFilters_h + +#include +#include +#include +#include +#include + +#include + +#include "EntityItemID.h" +#include "EntityItemProperties.h" + + +typedef QPair> FilterFunctionPair; + +class EntityEditFilters : public QObject { + Q_OBJECT +public: + EntityEditFilters() {}; + + void addEntityFilter(EntityItemID& entityID, QString filterURL); + void removeEntityFilter(EntityItemID& entityID); + + QScriptValue filter(glm::vec3& position, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, bool isAdd); + +private slots: + void scriptRequestFinished(EntityItemID& entityID); + +private: + QMap _filterFunctionMap; + QMap _filterScriptEngineMap; +}; + +#endif //hifi_EntityEditFilters_h From ff7c9d354694e6b74dd9d10d82e7b4b3c85368c1 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Wed, 8 Feb 2017 15:36:16 -0700 Subject: [PATCH 04/19] Working like before Single entity script running properly. Now, need to add the zone filters and execute them. --- .../src/entities/EntityServer.cpp | 105 ++++-------------- assignment-client/src/entities/EntityServer.h | 7 +- libraries/entities/src/EntityEditFilters.cpp | 69 ++++++++++-- libraries/entities/src/EntityEditFilters.h | 16 ++- libraries/entities/src/EntityTree.cpp | 44 +++----- libraries/entities/src/EntityTree.h | 9 +- 6 files changed, 112 insertions(+), 138 deletions(-) diff --git a/assignment-client/src/entities/EntityServer.cpp b/assignment-client/src/entities/EntityServer.cpp index 02dc552dae..9af9618b92 100644 --- a/assignment-client/src/entities/EntityServer.cpp +++ b/assignment-client/src/entities/EntityServer.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include "EntityServer.h" #include "EntityServerConsts.h" @@ -292,97 +293,33 @@ void EntityServer::readAdditionalConfiguration(const QJsonObject& settingsSectio } else { tree->setEntityScriptSourceWhitelist(""); } + + _entityEditFilters = new EntityEditFilters(); - if (readOptionString("entityEditFilter", settingsSectionObject, _entityEditFilter) && !_entityEditFilter.isEmpty()) { - // Tell the tree that we have a filter, so that it doesn't accept edits until we have a filter function set up. - std::static_pointer_cast(_tree)->setHasEntityFilter(true); - // Now fetch script from file asynchronously. - QUrl scriptURL(_entityEditFilter); - - // The following should be abstracted out for use in Agent.cpp (and maybe later AvatarMixer.cpp) - if (scriptURL.scheme().isEmpty() || (scriptURL.scheme() == URL_SCHEME_FILE)) { - qWarning() << "Cannot load script from local filesystem, because assignment may be on a different computer."; - scriptRequestFinished(); - return; - } - auto scriptRequest = ResourceManager::createResourceRequest(this, scriptURL); - if (!scriptRequest) { - qWarning() << "Could not create ResourceRequest for Agent script at" << scriptURL.toString(); - scriptRequestFinished(); - return; - } - // Agent.cpp sets up a timeout here, but that is unnecessary, as ResourceRequest has its own. - connect(scriptRequest, &ResourceRequest::finished, this, &EntityServer::scriptRequestFinished); - // FIXME: handle atp rquests setup here. See Agent::requestScript() - qInfo() << "Requesting script at URL" << qPrintable(scriptRequest->getUrl().toString()); - scriptRequest->send(); - qDebug() << "script request sent"; + std::static_pointer_cast(tree)->setEntityEditFilters(_entityEditFilters); + QString filterURL; + if (readOptionString("entityEditFilter", settingsSectionObject, filterURL) && !filterURL.isEmpty()) { + // connect the filterAdded signal, and block edits until you hear back + connect(_entityEditFilters, &EntityEditFilters::filterAdded, this, &EntityServer::entityFilterAdded); + + _entityEditFilters->rejectAll(true); + _entityEditFilters->addFilter(EntityItemID(), filterURL); } } -// Copied from ScriptEngine.cpp. We should make this a class method for reuse. -// Note: I've deliberately stopped short of using ScriptEngine instead of QScriptEngine, as that is out of project scope at this point. -static bool hasCorrectSyntax(const QScriptProgram& program) { - const auto syntaxCheck = QScriptEngine::checkSyntax(program.sourceCode()); - if (syntaxCheck.state() != QScriptSyntaxCheckResult::Valid) { - const auto error = syntaxCheck.errorMessage(); - const auto line = QString::number(syntaxCheck.errorLineNumber()); - const auto column = QString::number(syntaxCheck.errorColumnNumber()); - const auto message = QString("[SyntaxError] %1 in %2:%3(%4)").arg(error, program.fileName(), line, column); - qCritical() << qPrintable(message); - return false; +void EntityServer::entityFilterAdded(EntityItemID id, bool success) { + if(id.isInvalidID()) { + // this is the domain-wide entity filter, which we want to stop + // the world for + if (success) { + _entityEditFilters->rejectAll(false); + } else { + qDebug() << "entity edit filter unsuccessfully added, stopping..."; + stop(); + } } - return true; } -static bool hadUncaughtExceptions(QScriptEngine& engine, const QString& fileName) { - if (engine.hasUncaughtException()) { - const auto backtrace = engine.uncaughtExceptionBacktrace(); - const auto exception = engine.uncaughtException().toString(); - const auto line = QString::number(engine.uncaughtExceptionLineNumber()); - engine.clearExceptions(); - static const QString SCRIPT_EXCEPTION_FORMAT = "[UncaughtException] %1 in %2:%3"; - auto message = QString(SCRIPT_EXCEPTION_FORMAT).arg(exception, fileName, line); - if (!backtrace.empty()) { - static const auto lineSeparator = "\n "; - message += QString("\n[Backtrace]%1%2").arg(lineSeparator, backtrace.join(lineSeparator)); - } - qCritical() << qPrintable(message); - return true; - } - return false; -} -void EntityServer::scriptRequestFinished() { - qDebug() << "script request completed"; - auto scriptRequest = qobject_cast(sender()); - const QString urlString = scriptRequest->getUrl().toString(); - if (scriptRequest && scriptRequest->getResult() == ResourceRequest::Success) { - auto scriptContents = scriptRequest->getData(); - qInfo() << "Downloaded script:" << scriptContents; - QScriptProgram program(scriptContents, urlString); - if (hasCorrectSyntax(program)) { - _entityEditFilterEngine.evaluate(scriptContents); - if (!hadUncaughtExceptions(_entityEditFilterEngine, urlString)) { - std::static_pointer_cast(_tree)->initEntityEditFilterEngine(&_entityEditFilterEngine, [this]() { - return hadUncaughtExceptions(_entityEditFilterEngine, _entityEditFilter); - }); - scriptRequest->deleteLater(); - qDebug() << "script request filter processed"; - return; - } - } - } else if (scriptRequest) { - qCritical() << "Failed to download script at" << urlString; - // See HTTPResourceRequest::onRequestFinished for interpretation of codes. For example, a 404 is code 6 and 403 is 3. A timeout is 2. Go figure. - qCritical() << "ResourceRequest error was" << scriptRequest->getResult(); - } else { - qCritical() << "Failed to create script request."; - } - // Hard stop of the assignment client on failure. We don't want anyone to think they have a filter in place when they don't. - // Alas, only indications will be the above logging with assignment client restarting repeatedly, and clients will not see any entities. - qDebug() << "script request failure causing stop"; - stop(); -} void EntityServer::nodeAdded(SharedNodePointer node) { EntityTreePointer tree = std::static_pointer_cast(_tree); diff --git a/assignment-client/src/entities/EntityServer.h b/assignment-client/src/entities/EntityServer.h index f142145d5f..0edbc45ce1 100644 --- a/assignment-client/src/entities/EntityServer.h +++ b/assignment-client/src/entities/EntityServer.h @@ -63,13 +63,13 @@ public slots: virtual void nodeAdded(SharedNodePointer node) override; virtual void nodeKilled(SharedNodePointer node) override; void pruneDeletedEntities(); + void entityFilterAdded(EntityItemID id, bool success); protected: virtual OctreePointer createTree() override; private slots: void handleEntityPacket(QSharedPointer message, SharedNodePointer senderNode); - void scriptRequestFinished(); private: SimpleEntitySimulationPointer _entitySimulation; @@ -77,9 +77,8 @@ private: QReadWriteLock _viewerSendingStatsLock; QMap> _viewerSendingStats; - - QString _entityEditFilter{}; - QScriptEngine _entityEditFilterEngine{}; + + EntityEditFilters* _entityEditFilters{}; }; #endif // hifi_EntityServer_h diff --git a/libraries/entities/src/EntityEditFilters.cpp b/libraries/entities/src/EntityEditFilters.cpp index a74ba091b5..2afcd93b63 100644 --- a/libraries/entities/src/EntityEditFilters.cpp +++ b/libraries/entities/src/EntityEditFilters.cpp @@ -16,12 +16,55 @@ #include "EntityEditFilters.h" -QScriptValue EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, bool isAdd) { - qDebug() << "in EntityEditFilters"; - return QScriptValue(); +bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, EntityTree::FilterType filterType) { + qCDebug(entities) << "in EntityEditFilters"; + + // allows us to start entity server and reject all edits until filter is setup + if (_rejectAll) { + return false; + } + + bool accepted = true; + + qCDebug(entities) << "_filterFunctionMap.size:" << _filterFunctionMap.size(); + //first off lets call the filter (if any) that is domain-wide (EntityItemID()) + FilterFunctionPair* pair = _filterFunctionMap.value(EntityItemID()); + QScriptEngine* engine = _filterScriptEngineMap.value(EntityItemID()); + qCDebug(entities) << "pair: " << (qint64) pair << ", engine" << (qint64)engine; + if (pair != nullptr && engine != nullptr) { + + qCDebug(entities) << "attempting to call filter"; + auto oldProperties = propertiesIn.getDesiredProperties(); + auto specifiedProperties = propertiesIn.getChangedProperties(); + propertiesIn.setDesiredProperties(specifiedProperties); + QScriptValue inputValues = propertiesIn.copyToScriptValue(engine, 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. + QScriptValueList args; + args << inputValues; + args << filterType; + + QScriptValue result = pair->first.call(_nullObjectForFilter, args); + if (pair->second()) { + result = QScriptValue(); + } + + accepted = result.isObject(); + + if (accepted) { + qCDebug(entities) << "filter result accepted"; + // call rest of them soon + propertiesOut.copyFromScriptValue(result, false); + // 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()); + wasChanged = in != out; + } + } + return accepted; } -void EntityEditFilters::removeEntityFilter(EntityItemID& entityID) { +void EntityEditFilters::removeFilter(EntityItemID& entityID) { QScriptEngine* engine = _filterScriptEngineMap.value(entityID); if (engine) { _filterScriptEngineMap.remove(entityID); @@ -34,7 +77,7 @@ void EntityEditFilters::removeEntityFilter(EntityItemID& entityID) { } } -void EntityEditFilters::addEntityFilter(EntityItemID& entityID, QString filterURL) { +void EntityEditFilters::addFilter(EntityItemID& entityID, QString filterURL) { QUrl scriptURL(filterURL); // The following should be abstracted out for use in Agent.cpp (and maybe later AvatarMixer.cpp) @@ -44,7 +87,7 @@ void EntityEditFilters::addEntityFilter(EntityItemID& entityID, QString filterUR return; } // first remove any existing info for this entity - removeEntityFilter(entityID); + removeFilter(entityID); auto scriptRequest = ResourceManager::createResourceRequest(this, scriptURL); if (!scriptRequest) { @@ -53,11 +96,11 @@ void EntityEditFilters::addEntityFilter(EntityItemID& entityID, QString filterUR return; } // Agent.cpp sets up a timeout here, but that is unnecessary, as ResourceRequest has its own. - connect(scriptRequest, &ResourceRequest::finished, this, [this, &entityID]{ EntityEditFilters::scriptRequestFinished(entityID);} ); + connect(scriptRequest, &ResourceRequest::finished, this, [this, entityID]{ EntityEditFilters::scriptRequestFinished(entityID);} ); // FIXME: handle atp rquests setup here. See Agent::requestScript() qInfo() << "Requesting script at URL" << qPrintable(scriptRequest->getUrl().toString()); scriptRequest->send(); - qDebug() << "script request sent"; + qDebug() << "script request sent for entity " << entityID; } @@ -94,8 +137,8 @@ static bool hadUncaughtExceptions(QScriptEngine& engine, const QString& fileName return false; } -void EntityEditFilters::scriptRequestFinished(EntityItemID& entityID) { - qDebug() << "script request completed"; +void EntityEditFilters::scriptRequestFinished(EntityItemID entityID) { + qDebug() << "script request completed for entity " << entityID; auto scriptRequest = qobject_cast(sender()); const QString urlString = scriptRequest->getUrl().toString(); if (scriptRequest && scriptRequest->getResult() == ResourceRequest::Success) { @@ -123,7 +166,9 @@ void EntityEditFilters::scriptRequestFinished(EntityItemID& entityID) { return; } _filterFunctionMap.insert(entityID, pair); - qDebug() << "script request filter processed"; + qDebug() << "script request filter processed for entity id " << entityID; + + emit filterAdded(entityID, true); return; } } @@ -134,5 +179,5 @@ void EntityEditFilters::scriptRequestFinished(EntityItemID& entityID) { } else { qCritical() << "Failed to create script request."; } - + emit filterAdded(entityID, false); } diff --git a/libraries/entities/src/EntityEditFilters.h b/libraries/entities/src/EntityEditFilters.h index 678b3ad0d1..ad6722819b 100644 --- a/libraries/entities/src/EntityEditFilters.h +++ b/libraries/entities/src/EntityEditFilters.h @@ -21,7 +21,7 @@ #include "EntityItemID.h" #include "EntityItemProperties.h" - +#include "EntityTree.h" typedef QPair> FilterFunctionPair; @@ -30,15 +30,21 @@ class EntityEditFilters : public QObject { public: EntityEditFilters() {}; - void addEntityFilter(EntityItemID& entityID, QString filterURL); - void removeEntityFilter(EntityItemID& entityID); + void addFilter(EntityItemID& entityID, QString filterURL); + void removeFilter(EntityItemID& entityID); - QScriptValue filter(glm::vec3& position, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, bool isAdd); + bool filter(glm::vec3& position, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, EntityTree::FilterType filterType); + void rejectAll(bool state) {_rejectAll = state; } + +signals: + void filterAdded(EntityItemID id, bool success); private slots: - void scriptRequestFinished(EntityItemID& entityID); + void scriptRequestFinished(EntityItemID entityID); private: + bool _rejectAll {false}; + QScriptValue _nullObjectForFilter{}; QMap _filterFunctionMap; QMap _filterScriptEngineMap; }; diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 4e92b2a572..5e2fc0f1f9 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -24,6 +24,7 @@ #include "EntitiesLogging.h" #include "RecurseOctreeToMapOperator.h" #include "LogHandler.h" +#include "EntityEditFilters.h" static const quint64 DELETED_ENTITIES_EXTRA_USECS_TO_CONSIDER = USECS_PER_MSEC * 50; const float EntityTree::DEFAULT_MAX_TMP_ENTITY_LIFETIME = 60 * 60; // 1 hour @@ -61,6 +62,9 @@ EntityTree::EntityTree(bool shouldReaverage) : EntityTree::~EntityTree() { eraseAllOctreeElements(false); + if (_entityEditFilters) { + delete _entityEditFilters; + } } void EntityTree::setEntityScriptSourceWhitelist(const QString& entityScriptSourceWhitelist) { @@ -940,8 +944,8 @@ void EntityTree::initEntityEditFilterEngine(QScriptEngine* engine, std::function _hasEntityEditFilter = true; } -bool EntityTree::filterProperties(EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, FilterType filterType) { - if (!_entityEditFilterEngine) { +bool EntityTree::filterProperties(EntityItemPointer& existingEntity, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, FilterType filterType) { + if (!_entityEditFilterEngine && !_entityEditFilters) { propertiesOut = propertiesIn; wasChanged = false; // not changed if (_hasEntityEditFilter) { @@ -950,28 +954,9 @@ bool EntityTree::filterProperties(EntityItemProperties& propertiesIn, EntityItem } return true; // allowed } - 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. - QScriptValueList args; - args << inputValues; - args << filterType; - - QScriptValue result = _entityEditFilterFunction.call(_nullObjectForFilter, args); - if (_entityEditFilterHadUncaughtExceptions()) { - result = QScriptValue(); - } - - bool accepted = result.isObject(); // filters should return null or false to completely reject edit or add - if (accepted) { - propertiesOut.copyFromScriptValue(result, false); - // 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()); - wasChanged = in != out; + bool accepted = true; + if (_entityEditFilters) { + accepted = _entityEditFilters->filter(existingEntity->getPosition(), propertiesIn, propertiesOut, wasChanged, filterType); } return accepted; @@ -1076,11 +1061,16 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c // an existing entity... handle appropriately if (validEditPacket) { + // search for the entity by EntityItemID + startLookup = usecTimestampNow(); + EntityItemPointer existingEntity = findEntityByEntityItemID(entityItemID); + endLookup = usecTimestampNow(); + startFilter = usecTimestampNow(); bool wasChanged = false; // Having (un)lock rights bypasses the filter, unless it's a physics result. FilterType filterType = isPhysics ? FilterType::Physics : (isAdd ? FilterType::Add : FilterType::Edit); - bool allowed = (!isPhysics && senderNode->isAllowedEditor()) || filterProperties(properties, properties, wasChanged, filterType); + bool allowed = (!isPhysics && senderNode->isAllowedEditor()) || filterProperties(existingEntity, properties, properties, wasChanged, filterType); if (!allowed) { auto timestamp = properties.getLastEdited(); properties = EntityItemProperties(); @@ -1093,10 +1083,6 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c } endFilter = usecTimestampNow(); - // search for the entity by EntityItemID - startLookup = usecTimestampNow(); - EntityItemPointer existingEntity = findEntityByEntityItemID(entityItemID); - endLookup = usecTimestampNow(); if (existingEntity && !isAdd) { if (suppressDisallowedScript) { diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index 7d7948844e..34a52ce1b2 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -24,8 +24,8 @@ typedef std::shared_ptr EntityTreePointer; #include "EntityTreeElement.h" #include "DeleteEntityOperator.h" -#include "EntityEditFilters.h" +class EntityEditFilters; class Model; using ModelPointer = std::shared_ptr; using ModelWeakPointer = std::weak_ptr; @@ -274,7 +274,8 @@ public: void initEntityEditFilterEngine(QScriptEngine* engine, std::function entityEditFilterHadUncaughtExceptions); void setHasEntityFilter(bool hasFilter) { _hasEntityEditFilter = hasFilter; } - + + void setEntityEditFilters(EntityEditFilters* entityEditFilters) { _entityEditFilters = entityEditFilters; } static const float DEFAULT_MAX_TMP_ENTITY_LIFETIME; public slots: @@ -363,7 +364,7 @@ protected: float _maxTmpEntityLifetime { DEFAULT_MAX_TMP_ENTITY_LIFETIME }; - bool filterProperties(EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, FilterType filterType); + bool filterProperties(EntityItemPointer& existingEntity, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, FilterType filterType); bool _hasEntityEditFilter{ false }; QScriptEngine* _entityEditFilterEngine{}; QScriptValue _entityEditFilterFunction{}; @@ -371,7 +372,7 @@ protected: std::function _entityEditFilterHadUncaughtExceptions; QStringList _entityScriptSourceWhitelist; - EntityEditFilters* _entityEditFilters; + EntityEditFilters* _entityEditFilters{}; }; #endif // hifi_EntityTree_h From 8d666854c79eb15b71e284e52b5abd62b3232d71 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Thu, 9 Feb 2017 15:39:38 -0700 Subject: [PATCH 05/19] working it seems But, AABox for zones isn't very helpful (box is _small). Time to use the shape of the zone. --- .../src/entities/EntityServer.cpp | 15 ++- assignment-client/src/entities/EntityServer.h | 2 - libraries/entities/src/EntityEditFilters.cpp | 122 +++++++++++++----- libraries/entities/src/EntityEditFilters.h | 6 + libraries/entities/src/EntityTree.cpp | 8 +- libraries/entities/src/EntityTree.h | 4 +- libraries/entities/src/ZoneEntityItem.cpp | 15 +++ libraries/entities/src/ZoneEntityItem.h | 2 +- 8 files changed, 125 insertions(+), 49 deletions(-) diff --git a/assignment-client/src/entities/EntityServer.cpp b/assignment-client/src/entities/EntityServer.cpp index 9af9618b92..c3045f599c 100644 --- a/assignment-client/src/entities/EntityServer.cpp +++ b/assignment-client/src/entities/EntityServer.cpp @@ -294,16 +294,15 @@ void EntityServer::readAdditionalConfiguration(const QJsonObject& settingsSectio tree->setEntityScriptSourceWhitelist(""); } - _entityEditFilters = new EntityEditFilters(); - - std::static_pointer_cast(tree)->setEntityEditFilters(_entityEditFilters); + auto entityEditFilters = tree->createEntityEditFilters(); + QString filterURL; if (readOptionString("entityEditFilter", settingsSectionObject, filterURL) && !filterURL.isEmpty()) { // connect the filterAdded signal, and block edits until you hear back - connect(_entityEditFilters, &EntityEditFilters::filterAdded, this, &EntityServer::entityFilterAdded); + connect(entityEditFilters, &EntityEditFilters::filterAdded, this, &EntityServer::entityFilterAdded); - _entityEditFilters->rejectAll(true); - _entityEditFilters->addFilter(EntityItemID(), filterURL); + entityEditFilters->rejectAll(true); + entityEditFilters->addFilter(EntityItemID(), filterURL); } } @@ -311,8 +310,10 @@ void EntityServer::entityFilterAdded(EntityItemID id, bool success) { if(id.isInvalidID()) { // this is the domain-wide entity filter, which we want to stop // the world for + auto entityEditFilters = qobject_cast(sender()); if (success) { - _entityEditFilters->rejectAll(false); + qDebug() << "entity edit filter for " << id << "added successfully"; + entityEditFilters->rejectAll(false); } else { qDebug() << "entity edit filter unsuccessfully added, stopping..."; stop(); diff --git a/assignment-client/src/entities/EntityServer.h b/assignment-client/src/entities/EntityServer.h index 0edbc45ce1..325435fe7e 100644 --- a/assignment-client/src/entities/EntityServer.h +++ b/assignment-client/src/entities/EntityServer.h @@ -77,8 +77,6 @@ private: QReadWriteLock _viewerSendingStatsLock; QMap> _viewerSendingStats; - - EntityEditFilters* _entityEditFilters{}; }; #endif // hifi_EntityServer_h diff --git a/libraries/entities/src/EntityEditFilters.cpp b/libraries/entities/src/EntityEditFilters.cpp index 2afcd93b63..dccb49d4f5 100644 --- a/libraries/entities/src/EntityEditFilters.cpp +++ b/libraries/entities/src/EntityEditFilters.cpp @@ -13,58 +13,99 @@ #include #include - #include "EntityEditFilters.h" +QList EntityEditFilters::getZonesByPosition(glm::vec3& position) { + QList zones; + _lock.lockForRead(); + qCDebug(entities) << "looking at " << _filterFunctionMap.size() << "possible zones"; + for (auto it = _filterFunctionMap.begin(); it != _filterFunctionMap.end(); it++) { + auto id = it.key(); + if (!id.isInvalidID()) { + // for now, look it up in the tree (soon we need to cache or similar?) + EntityItemPointer itemPtr = _tree->findEntityByEntityItemID(id); + auto zone = std::dynamic_pointer_cast(itemPtr); + if (zone && zone->containsPoint(position)) { + zones.append(id); + } + } else { + zones.append(id); + } + } + _lock.unlock(); + return zones; +} + bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, EntityTree::FilterType filterType) { qCDebug(entities) << "in EntityEditFilters"; // allows us to start entity server and reject all edits until filter is setup if (_rejectAll) { + qCDebug(entities) << "rejecting all edits"; return false; } - bool accepted = true; + // get the ids of all the zones (plus the global entity edit filter) that the position + // lies within + auto zoneIDs = getZonesByPosition(position); - qCDebug(entities) << "_filterFunctionMap.size:" << _filterFunctionMap.size(); - //first off lets call the filter (if any) that is domain-wide (EntityItemID()) - FilterFunctionPair* pair = _filterFunctionMap.value(EntityItemID()); - QScriptEngine* engine = _filterScriptEngineMap.value(EntityItemID()); - qCDebug(entities) << "pair: " << (qint64) pair << ", engine" << (qint64)engine; - if (pair != nullptr && engine != nullptr) { + // temp debugging -- remove! + qCDebug(entities) << "zones:"; + for (auto zoneID : zoneIDs) { + qCDebug(entities) << zoneID << ","; + } + + auto oldProperties = propertiesIn.getDesiredProperties(); + auto specifiedProperties = propertiesIn.getChangedProperties(); + propertiesIn.setDesiredProperties(specifiedProperties); + for (auto it = zoneIDs.begin(); it != zoneIDs.end(); it++) { + qCDebug(entities) << "applying filter for zone" << *it; - qCDebug(entities) << "attempting to call filter"; - auto oldProperties = propertiesIn.getDesiredProperties(); - auto specifiedProperties = propertiesIn.getChangedProperties(); - propertiesIn.setDesiredProperties(specifiedProperties); - QScriptValue inputValues = propertiesIn.copyToScriptValue(engine, false, true, true); - propertiesIn.setDesiredProperties(oldProperties); + // get the filter pair, etc... + _lock.lockForRead(); + FilterFunctionPair* pair = _filterFunctionMap.value(*it); + QScriptEngine* engine = _filterScriptEngineMap.value(*it); + _lock.unlock(); + + qCDebug(entities) << "pair: " << (qint64) pair << ", engine" << (qint64)engine; + if (pair != nullptr && engine != nullptr) { - auto in = QJsonValue::fromVariant(inputValues.toVariant()); // grab json copy now, because the inputValues might be side effected by the filter. - QScriptValueList args; - args << inputValues; - args << filterType; + QScriptValue inputValues = propertiesIn.copyToScriptValue(engine, false, true, true); + propertiesIn.setDesiredProperties(oldProperties); - QScriptValue result = pair->first.call(_nullObjectForFilter, args); - if (pair->second()) { - result = QScriptValue(); - } - - accepted = result.isObject(); - - if (accepted) { - qCDebug(entities) << "filter result accepted"; - // call rest of them soon - propertiesOut.copyFromScriptValue(result, false); - // 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()); - wasChanged = in != out; + auto in = QJsonValue::fromVariant(inputValues.toVariant()); // grab json copy now, because the inputValues might be side effected by the filter. + QScriptValueList args; + args << inputValues; + args << filterType; + + QScriptValue result = pair->first.call(_nullObjectForFilter, args); + if (pair->second()) { + result = QScriptValue(); + } + + + if (result.isObject()){ + qCDebug(entities) << "filter result accepted"; + // make propertiesIn reflect the changes, for next filter... + propertiesIn.copyFromScriptValue(result, false); + + // and update propertiesOut too. #TODO: this could be more efficient... + propertiesOut.copyFromScriptValue(result, false); + // 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()); + wasChanged |= (in != out); + } else { + // an edit was rejected, so we stop here and return false + qCDebug(entities) << "Edit rejected by " << *it; + } } } - return accepted; + // if we made it here, + return true; } void EntityEditFilters::removeFilter(EntityItemID& entityID) { + QWriteLocker writeLock(&_lock); QScriptEngine* engine = _filterScriptEngineMap.value(entityID); if (engine) { _filterScriptEngineMap.remove(entityID); @@ -78,6 +119,7 @@ void EntityEditFilters::removeFilter(EntityItemID& entityID) { } void EntityEditFilters::addFilter(EntityItemID& entityID, QString filterURL) { + QUrl scriptURL(filterURL); // The following should be abstracted out for use in Agent.cpp (and maybe later AvatarMixer.cpp) @@ -151,8 +193,10 @@ void EntityEditFilters::scriptRequestFinished(EntityItemID entityID) { engine->evaluate(scriptContents); if (!hadUncaughtExceptions(*engine, urlString)) { // put the engine in the engine map (so we don't leak them, etc...) + _lock.lockForWrite(); _filterScriptEngineMap.insert(entityID, engine); - + _lock.unlock(); + // define the uncaughtException function FilterFunctionPair* pair = new FilterFunctionPair(); QScriptEngine& engineRef = *engine; @@ -160,12 +204,20 @@ void EntityEditFilters::scriptRequestFinished(EntityItemID entityID) { // now get the filter function auto global = engine->globalObject(); + auto entitiesObject = engine->newObject(); + entitiesObject.setProperty("ADD_FILTER_TYPE", EntityTree::FilterType::Add); + entitiesObject.setProperty("EDIT_FILTER_TYPE", EntityTree::FilterType::Edit); + entitiesObject.setProperty("PHYSICS_FILTER_TYPE", EntityTree::FilterType::Physics); + global.setProperty("Entities", entitiesObject); pair->first = global.property("filter"); if (!pair->first.isFunction()) { qDebug() << "Filter function specified but not found. Ignoring filter"; return; } - _filterFunctionMap.insert(entityID, pair); + _lock.lockForWrite(); + _filterFunctionMap.insert(entityID, pair); + _lock.unlock(); + qDebug() << "script request filter processed for entity id " << entityID; emit filterAdded(entityID, true); diff --git a/libraries/entities/src/EntityEditFilters.h b/libraries/entities/src/EntityEditFilters.h index ad6722819b..b4d31a54e8 100644 --- a/libraries/entities/src/EntityEditFilters.h +++ b/libraries/entities/src/EntityEditFilters.h @@ -29,6 +29,7 @@ class EntityEditFilters : public QObject { Q_OBJECT public: EntityEditFilters() {}; + EntityEditFilters(EntityTreePointer tree ): _tree(tree) {}; void addFilter(EntityItemID& entityID, QString filterURL); void removeFilter(EntityItemID& entityID); @@ -43,8 +44,13 @@ private slots: void scriptRequestFinished(EntityItemID entityID); private: + QList getZonesByPosition(glm::vec3& position); + + EntityTreePointer _tree {}; bool _rejectAll {false}; QScriptValue _nullObjectForFilter{}; + + QReadWriteLock _lock; QMap _filterFunctionMap; QMap _filterScriptEngineMap; }; diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 5e2fc0f1f9..6a2a52f958 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -62,9 +62,6 @@ EntityTree::EntityTree(bool shouldReaverage) : EntityTree::~EntityTree() { eraseAllOctreeElements(false); - if (_entityEditFilters) { - delete _entityEditFilters; - } } void EntityTree::setEntityScriptSourceWhitelist(const QString& entityScriptSourceWhitelist) { @@ -1751,3 +1748,8 @@ QStringList EntityTree::getJointNames(const QUuid& entityID) const { } return entity->getJointNames(); } + +EntityEditFilters* EntityTree::createEntityEditFilters() { + _entityEditFilters = new EntityEditFilters(getThisPointer()); + return _entityEditFilters; +} diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index 34a52ce1b2..03e0f30b0b 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -275,7 +275,9 @@ public: void initEntityEditFilterEngine(QScriptEngine* engine, std::function entityEditFilterHadUncaughtExceptions); void setHasEntityFilter(bool hasFilter) { _hasEntityEditFilter = hasFilter; } - void setEntityEditFilters(EntityEditFilters* entityEditFilters) { _entityEditFilters = entityEditFilters; } + EntityEditFilters* createEntityEditFilters(); + EntityEditFilters* getEntityEditFilters() { return _entityEditFilters; } + static const float DEFAULT_MAX_TMP_ENTITY_LIFETIME; public slots: diff --git a/libraries/entities/src/ZoneEntityItem.cpp b/libraries/entities/src/ZoneEntityItem.cpp index 7de3f6b68f..61b3800b50 100644 --- a/libraries/entities/src/ZoneEntityItem.cpp +++ b/libraries/entities/src/ZoneEntityItem.cpp @@ -19,6 +19,9 @@ #include "EntityTree.h" #include "EntityTreeElement.h" #include "ZoneEntityItem.h" +#include "EntityEditFilters.h" + +#include bool ZoneEntityItem::_zonesArePickable = false; bool ZoneEntityItem::_drawZoneBoundaries = false; @@ -221,12 +224,24 @@ bool ZoneEntityItem::findDetailedRayIntersection(const glm::vec3& origin, const return _zonesArePickable; } +void ZoneEntityItem::setFilterURL(QString url) { + _filterURL = url; + if (getTree()) { + auto entityEditFilters = getTree()->getEntityEditFilters(); + if (entityEditFilters) { + qCDebug(entities) << "adding filter " << url << "for zone" << getEntityItemID(); + entityEditFilters->addFilter(getEntityItemID(), url); + } + } +} + bool ZoneEntityItem::containsPoint(glm::vec3& position) { // use _shapeType shortly // for now bounding box just so I can get end-to-end working bool success; AABox box = getAABox(success); if (success) { + qCDebug(entities) << "box: " << box; return box.contains(position); } // just return false if no AABox diff --git a/libraries/entities/src/ZoneEntityItem.h b/libraries/entities/src/ZoneEntityItem.h index 07520001f7..f804ac8dbb 100644 --- a/libraries/entities/src/ZoneEntityItem.h +++ b/libraries/entities/src/ZoneEntityItem.h @@ -75,7 +75,7 @@ public: bool getGhostingAllowed() const { return _ghostingAllowed; } void setGhostingAllowed(bool value) { _ghostingAllowed = value; } QString getFilterURL() const { return _filterURL; } - void setFilterURL(const QString url) { _filterURL = url; } + void setFilterURL(const QString url); bool containsPoint(glm::vec3& position); virtual bool supportsDetailedRayIntersection() const override { return true; } From 415d71956f975b0ccd4eef41283814f46c095460 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Thu, 9 Feb 2017 19:59:55 -0700 Subject: [PATCH 06/19] minor logging crap --- libraries/entities/src/EntityEditFilters.cpp | 2 +- libraries/entities/src/ZoneEntityItem.cpp | 9 ++------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/libraries/entities/src/EntityEditFilters.cpp b/libraries/entities/src/EntityEditFilters.cpp index dccb49d4f5..8c4041fd35 100644 --- a/libraries/entities/src/EntityEditFilters.cpp +++ b/libraries/entities/src/EntityEditFilters.cpp @@ -18,7 +18,7 @@ QList EntityEditFilters::getZonesByPosition(glm::vec3& position) { QList zones; _lock.lockForRead(); - qCDebug(entities) << "looking at " << _filterFunctionMap.size() << "possible zones"; + qCDebug(entities) << "looking at " << _filterFunctionMap.size() << "possible zones, at " << position; for (auto it = _filterFunctionMap.begin(); it != _filterFunctionMap.end(); it++) { auto id = it.key(); if (!id.isInvalidID()) { diff --git a/libraries/entities/src/ZoneEntityItem.cpp b/libraries/entities/src/ZoneEntityItem.cpp index 61b3800b50..58b57f357d 100644 --- a/libraries/entities/src/ZoneEntityItem.cpp +++ b/libraries/entities/src/ZoneEntityItem.cpp @@ -239,12 +239,7 @@ bool ZoneEntityItem::containsPoint(glm::vec3& position) { // use _shapeType shortly // for now bounding box just so I can get end-to-end working bool success; - AABox box = getAABox(success); - if (success) { - qCDebug(entities) << "box: " << box; - return box.contains(position); - } - // just return false if no AABox - return success; + bool result = getAABox(success).contains(position); + return result && success; } From cf780b3b73146a72346254331d1c7809cfbdd7ff Mon Sep 17 00:00:00 2001 From: David Kelly Date: Fri, 10 Feb 2017 11:42:56 -0700 Subject: [PATCH 07/19] fixed persist issue, working much better --- .../src/entities/EntityServer.cpp | 5 +-- libraries/entities/src/EntityEditFilters.cpp | 14 +++++--- libraries/entities/src/EntityEditFilters.h | 2 +- libraries/entities/src/EntityItemID.cpp | 1 + libraries/entities/src/EntityTree.cpp | 34 ++----------------- libraries/entities/src/EntityTree.h | 12 ------- libraries/entities/src/ZoneEntityItem.cpp | 10 +++--- 7 files changed, 22 insertions(+), 56 deletions(-) diff --git a/assignment-client/src/entities/EntityServer.cpp b/assignment-client/src/entities/EntityServer.cpp index c3045f599c..845136b0a2 100644 --- a/assignment-client/src/entities/EntityServer.cpp +++ b/assignment-client/src/entities/EntityServer.cpp @@ -72,6 +72,7 @@ OctreePointer EntityServer::createTree() { DependencyManager::registerInheritance(); DependencyManager::set(tree); + DependencyManager::set(std::static_pointer_cast(tree)); return tree; } @@ -294,12 +295,12 @@ void EntityServer::readAdditionalConfiguration(const QJsonObject& settingsSectio tree->setEntityScriptSourceWhitelist(""); } - auto entityEditFilters = tree->createEntityEditFilters(); + auto entityEditFilters = DependencyManager::get(); QString filterURL; if (readOptionString("entityEditFilter", settingsSectionObject, filterURL) && !filterURL.isEmpty()) { // connect the filterAdded signal, and block edits until you hear back - connect(entityEditFilters, &EntityEditFilters::filterAdded, this, &EntityServer::entityFilterAdded); + connect(entityEditFilters.data(), &EntityEditFilters::filterAdded, this, &EntityServer::entityFilterAdded); entityEditFilters->rejectAll(true); entityEditFilters->addFilter(EntityItemID(), filterURL); diff --git a/libraries/entities/src/EntityEditFilters.cpp b/libraries/entities/src/EntityEditFilters.cpp index 8c4041fd35..385fb91c81 100644 --- a/libraries/entities/src/EntityEditFilters.cpp +++ b/libraries/entities/src/EntityEditFilters.cpp @@ -55,9 +55,6 @@ bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& proper qCDebug(entities) << zoneID << ","; } - auto oldProperties = propertiesIn.getDesiredProperties(); - auto specifiedProperties = propertiesIn.getChangedProperties(); - propertiesIn.setDesiredProperties(specifiedProperties); for (auto it = zoneIDs.begin(); it != zoneIDs.end(); it++) { qCDebug(entities) << "applying filter for zone" << *it; @@ -70,6 +67,9 @@ bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& proper qCDebug(entities) << "pair: " << (qint64) pair << ", engine" << (qint64)engine; if (pair != nullptr && engine != nullptr) { + auto oldProperties = propertiesIn.getDesiredProperties(); + auto specifiedProperties = propertiesIn.getChangedProperties(); + propertiesIn.setDesiredProperties(specifiedProperties); QScriptValue inputValues = propertiesIn.copyToScriptValue(engine, false, true, true); propertiesIn.setDesiredProperties(oldProperties); @@ -122,12 +122,19 @@ void EntityEditFilters::addFilter(EntityItemID& entityID, QString filterURL) { QUrl scriptURL(filterURL); + // setting it to an empty string is same as removing + if (filterURL.size() == 0) { + removeFilter(entityID); + return; + } + // The following should be abstracted out for use in Agent.cpp (and maybe later AvatarMixer.cpp) if (scriptURL.scheme().isEmpty() || (scriptURL.scheme() == URL_SCHEME_FILE)) { qWarning() << "Cannot load script from local filesystem, because assignment may be on a different computer."; scriptRequestFinished(entityID); return; } + // first remove any existing info for this entity removeFilter(entityID); @@ -143,7 +150,6 @@ void EntityEditFilters::addFilter(EntityItemID& entityID, QString filterURL) { qInfo() << "Requesting script at URL" << qPrintable(scriptRequest->getUrl().toString()); scriptRequest->send(); qDebug() << "script request sent for entity " << entityID; - } // Copied from ScriptEngine.cpp. We should make this a class method for reuse. diff --git a/libraries/entities/src/EntityEditFilters.h b/libraries/entities/src/EntityEditFilters.h index b4d31a54e8..36962838d8 100644 --- a/libraries/entities/src/EntityEditFilters.h +++ b/libraries/entities/src/EntityEditFilters.h @@ -25,7 +25,7 @@ typedef QPair> FilterFunctionPair; -class EntityEditFilters : public QObject { +class EntityEditFilters : public QObject, public Dependency { Q_OBJECT public: EntityEditFilters() {}; diff --git a/libraries/entities/src/EntityItemID.cpp b/libraries/entities/src/EntityItemID.cpp index 1462a4ef88..5f07019db4 100644 --- a/libraries/entities/src/EntityItemID.cpp +++ b/libraries/entities/src/EntityItemID.cpp @@ -19,6 +19,7 @@ #include "RegisteredMetaTypes.h" #include "EntityItemID.h" +int entityItemIDTypeID = qRegisterMetaType(); EntityItemID::EntityItemID() : QUuid() { diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 6a2a52f958..4958932fe3 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -924,36 +924,12 @@ void EntityTree::fixupTerseEditLogging(EntityItemProperties& properties, QList entityEditFilterHadUncaughtExceptions) { - _entityEditFilterEngine = engine; - _entityEditFilterHadUncaughtExceptions = entityEditFilterHadUncaughtExceptions; - auto global = _entityEditFilterEngine->globalObject(); - _entityEditFilterFunction = global.property("filter"); - if (!_entityEditFilterFunction.isFunction()) { - qCDebug(entities) << "Filter function specified but not found. Will reject all edits."; - _entityEditFilterEngine = nullptr; // So that we don't try to call it. See filterProperties. - } - auto entitiesObject = _entityEditFilterEngine->newObject(); - entitiesObject.setProperty("ADD_FILTER_TYPE", FilterType::Add); - entitiesObject.setProperty("EDIT_FILTER_TYPE", FilterType::Edit); - entitiesObject.setProperty("PHYSICS_FILTER_TYPE", FilterType::Physics); - global.setProperty("Entities", entitiesObject); - _hasEntityEditFilter = true; -} bool EntityTree::filterProperties(EntityItemPointer& existingEntity, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, FilterType filterType) { - if (!_entityEditFilterEngine && !_entityEditFilters) { - propertiesOut = propertiesIn; - wasChanged = false; // not changed - if (_hasEntityEditFilter) { - qCDebug(entities) << "Rejecting properties because filter has not been set."; - return false; - } - return true; // allowed - } bool accepted = true; - if (_entityEditFilters) { - accepted = _entityEditFilters->filter(existingEntity->getPosition(), propertiesIn, propertiesOut, wasChanged, filterType); + auto entityEditFilters = DependencyManager::get(); + if (entityEditFilters) { + accepted = entityEditFilters->filter(existingEntity->getPosition(), propertiesIn, propertiesOut, wasChanged, filterType); } return accepted; @@ -1749,7 +1725,3 @@ QStringList EntityTree::getJointNames(const QUuid& entityID) const { return entity->getJointNames(); } -EntityEditFilters* EntityTree::createEntityEditFilters() { - _entityEditFilters = new EntityEditFilters(getThisPointer()); - return _entityEditFilters; -} diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index 03e0f30b0b..63f7bbfd66 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -272,12 +272,6 @@ public: void notifyNewCollisionSoundURL(const QString& newCollisionSoundURL, const EntityItemID& entityID); - void initEntityEditFilterEngine(QScriptEngine* engine, std::function entityEditFilterHadUncaughtExceptions); - void setHasEntityFilter(bool hasFilter) { _hasEntityEditFilter = hasFilter; } - - EntityEditFilters* createEntityEditFilters(); - EntityEditFilters* getEntityEditFilters() { return _entityEditFilters; } - static const float DEFAULT_MAX_TMP_ENTITY_LIFETIME; public slots: @@ -368,13 +362,7 @@ protected: bool filterProperties(EntityItemPointer& existingEntity, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, FilterType filterType); bool _hasEntityEditFilter{ false }; - QScriptEngine* _entityEditFilterEngine{}; - QScriptValue _entityEditFilterFunction{}; - QScriptValue _nullObjectForFilter{}; - std::function _entityEditFilterHadUncaughtExceptions; - QStringList _entityScriptSourceWhitelist; - EntityEditFilters* _entityEditFilters{}; }; #endif // hifi_EntityTree_h diff --git a/libraries/entities/src/ZoneEntityItem.cpp b/libraries/entities/src/ZoneEntityItem.cpp index 58b57f357d..9273298d3e 100644 --- a/libraries/entities/src/ZoneEntityItem.cpp +++ b/libraries/entities/src/ZoneEntityItem.cpp @@ -226,12 +226,10 @@ bool ZoneEntityItem::findDetailedRayIntersection(const glm::vec3& origin, const void ZoneEntityItem::setFilterURL(QString url) { _filterURL = url; - if (getTree()) { - auto entityEditFilters = getTree()->getEntityEditFilters(); - if (entityEditFilters) { - qCDebug(entities) << "adding filter " << url << "for zone" << getEntityItemID(); - entityEditFilters->addFilter(getEntityItemID(), url); - } + if (DependencyManager::isSet()) { + auto entityEditFilters = DependencyManager::get(); + qCDebug(entities) << "adding filter " << url << "for zone" << getEntityItemID(); + entityEditFilters->addFilter(getEntityItemID(), url); } } From 7ec27cab13f5b51fae640210c543e7a4db16a55e Mon Sep 17 00:00:00 2001 From: David Kelly Date: Fri, 10 Feb 2017 12:47:48 -0700 Subject: [PATCH 08/19] Use EntityItem.contains Cuz it works - seemed like it was broken but nope. Also, trim off the filters for zones that no longer are in the tree. --- libraries/entities/src/EntityEditFilters.cpp | 17 ++++++++++++++--- libraries/entities/src/ZoneEntityItem.cpp | 8 -------- libraries/entities/src/ZoneEntityItem.h | 1 - 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/libraries/entities/src/EntityEditFilters.cpp b/libraries/entities/src/EntityEditFilters.cpp index 385fb91c81..e914248118 100644 --- a/libraries/entities/src/EntityEditFilters.cpp +++ b/libraries/entities/src/EntityEditFilters.cpp @@ -17,6 +17,7 @@ QList EntityEditFilters::getZonesByPosition(glm::vec3& position) { QList zones; + QList missingZones; _lock.lockForRead(); qCDebug(entities) << "looking at " << _filterFunctionMap.size() << "possible zones, at " << position; for (auto it = _filterFunctionMap.begin(); it != _filterFunctionMap.end(); it++) { @@ -25,14 +26,23 @@ QList EntityEditFilters::getZonesByPosition(glm::vec3& position) { // for now, look it up in the tree (soon we need to cache or similar?) EntityItemPointer itemPtr = _tree->findEntityByEntityItemID(id); auto zone = std::dynamic_pointer_cast(itemPtr); - if (zone && zone->containsPoint(position)) { + if (!zone) { + missingZones.append(id); + } else if (zone->contains(position)) { zones.append(id); } } else { + // the null id is the global filter we put in the domain server's + // advanced entity server settings zones.append(id); } } _lock.unlock(); + // TODO: maybe do this later (not block filter) + // now remove filters for missing zones + for (auto id : missingZones) { + removeFilter(id); + } return zones; } @@ -89,14 +99,15 @@ bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& proper // make propertiesIn reflect the changes, for next filter... propertiesIn.copyFromScriptValue(result, false); - // and update propertiesOut too. #TODO: this could be more efficient... + // and update propertiesOut too. TODO: this could be more efficient... propertiesOut.copyFromScriptValue(result, false); // 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()); wasChanged |= (in != out); } else { // an edit was rejected, so we stop here and return false - qCDebug(entities) << "Edit rejected by " << *it; + qCDebug(entities) << "Edit rejected by filter " << *it ; + return false; } } } diff --git a/libraries/entities/src/ZoneEntityItem.cpp b/libraries/entities/src/ZoneEntityItem.cpp index 9273298d3e..7158bc2588 100644 --- a/libraries/entities/src/ZoneEntityItem.cpp +++ b/libraries/entities/src/ZoneEntityItem.cpp @@ -233,11 +233,3 @@ void ZoneEntityItem::setFilterURL(QString url) { } } -bool ZoneEntityItem::containsPoint(glm::vec3& position) { - // use _shapeType shortly - // for now bounding box just so I can get end-to-end working - bool success; - bool result = getAABox(success).contains(position); - return result && success; -} - diff --git a/libraries/entities/src/ZoneEntityItem.h b/libraries/entities/src/ZoneEntityItem.h index f804ac8dbb..2bef95e452 100644 --- a/libraries/entities/src/ZoneEntityItem.h +++ b/libraries/entities/src/ZoneEntityItem.h @@ -76,7 +76,6 @@ public: void setGhostingAllowed(bool value) { _ghostingAllowed = value; } QString getFilterURL() const { return _filterURL; } void setFilterURL(const QString url); - bool containsPoint(glm::vec3& position); virtual bool supportsDetailedRayIntersection() const override { return true; } virtual bool findDetailedRayIntersection(const glm::vec3& origin, const glm::vec3& direction, From 1c1b0d1332ca1ae2e22757e5a6d220050bd4421b Mon Sep 17 00:00:00 2001 From: David Kelly Date: Fri, 10 Feb 2017 13:16:52 -0700 Subject: [PATCH 09/19] oops, debugging left in --- libraries/entities/src/ZoneEntityItem.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/libraries/entities/src/ZoneEntityItem.cpp b/libraries/entities/src/ZoneEntityItem.cpp index 7158bc2588..37b3be99a3 100644 --- a/libraries/entities/src/ZoneEntityItem.cpp +++ b/libraries/entities/src/ZoneEntityItem.cpp @@ -21,8 +21,6 @@ #include "ZoneEntityItem.h" #include "EntityEditFilters.h" -#include - bool ZoneEntityItem::_zonesArePickable = false; bool ZoneEntityItem::_drawZoneBoundaries = false; From 2996298e7903b3d98da9d9c93282e08240a48262 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Fri, 10 Feb 2017 14:57:09 -0700 Subject: [PATCH 10/19] First bit of cleanup consolidate to one map, some minor other cleaning. More coming. --- .../src/entities/EntityServer.cpp | 1 - libraries/entities/src/EntityEditFilters.cpp | 71 +++++++------------ libraries/entities/src/EntityEditFilters.h | 13 ++-- .../entities/src/EntityItemProperties.cpp | 2 +- 4 files changed, 37 insertions(+), 50 deletions(-) diff --git a/assignment-client/src/entities/EntityServer.cpp b/assignment-client/src/entities/EntityServer.cpp index 845136b0a2..b71242753d 100644 --- a/assignment-client/src/entities/EntityServer.cpp +++ b/assignment-client/src/entities/EntityServer.cpp @@ -322,7 +322,6 @@ void EntityServer::entityFilterAdded(EntityItemID id, bool success) { } } - void EntityServer::nodeAdded(SharedNodePointer node) { EntityTreePointer tree = std::static_pointer_cast(_tree); tree->knowAvatarID(node->getUUID()); diff --git a/libraries/entities/src/EntityEditFilters.cpp b/libraries/entities/src/EntityEditFilters.cpp index e914248118..4a84763a02 100644 --- a/libraries/entities/src/EntityEditFilters.cpp +++ b/libraries/entities/src/EntityEditFilters.cpp @@ -19,15 +19,17 @@ QList EntityEditFilters::getZonesByPosition(glm::vec3& position) { QList zones; QList missingZones; _lock.lockForRead(); - qCDebug(entities) << "looking at " << _filterFunctionMap.size() << "possible zones, at " << position; - for (auto it = _filterFunctionMap.begin(); it != _filterFunctionMap.end(); it++) { - auto id = it.key(); + auto zoneIDs = _filterDataMap.keys(); + _lock.unlock(); + qCDebug(entities) << "looking at " << zoneIDs.size() << "possible zones, at " << position; + for (auto id : zoneIDs) { if (!id.isInvalidID()) { // for now, look it up in the tree (soon we need to cache or similar?) EntityItemPointer itemPtr = _tree->findEntityByEntityItemID(id); auto zone = std::dynamic_pointer_cast(itemPtr); if (!zone) { - missingZones.append(id); + // TODO: maybe remove later? + removeFilter(id); } else if (zone->contains(position)) { zones.append(id); } @@ -37,12 +39,6 @@ QList EntityEditFilters::getZonesByPosition(glm::vec3& position) { zones.append(id); } } - _lock.unlock(); - // TODO: maybe do this later (not block filter) - // now remove filters for missing zones - for (auto id : missingZones) { - removeFilter(id); - } return zones; } @@ -59,28 +55,20 @@ bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& proper // lies within auto zoneIDs = getZonesByPosition(position); - // temp debugging -- remove! - qCDebug(entities) << "zones:"; - for (auto zoneID : zoneIDs) { - qCDebug(entities) << zoneID << ","; - } - - for (auto it = zoneIDs.begin(); it != zoneIDs.end(); it++) { - qCDebug(entities) << "applying filter for zone" << *it; + for (auto id : zoneIDs) { + qCDebug(entities) << "applying filter for zone" << id; // get the filter pair, etc... _lock.lockForRead(); - FilterFunctionPair* pair = _filterFunctionMap.value(*it); - QScriptEngine* engine = _filterScriptEngineMap.value(*it); + FilterData filterData = _filterDataMap.value(id); _lock.unlock(); - qCDebug(entities) << "pair: " << (qint64) pair << ", engine" << (qint64)engine; - if (pair != nullptr && engine != nullptr) { + if (filterData.valid()) { auto oldProperties = propertiesIn.getDesiredProperties(); auto specifiedProperties = propertiesIn.getChangedProperties(); propertiesIn.setDesiredProperties(specifiedProperties); - QScriptValue inputValues = propertiesIn.copyToScriptValue(engine, false, true, true); + QScriptValue inputValues = propertiesIn.copyToScriptValue(filterData.engine, 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. @@ -88,8 +76,8 @@ bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& proper args << inputValues; args << filterType; - QScriptValue result = pair->first.call(_nullObjectForFilter, args); - if (pair->second()) { + QScriptValue result = filterData.filterFn.call(_nullObjectForFilter, args); + if (filterData.uncaughtExceptions()) { result = QScriptValue(); } @@ -106,7 +94,7 @@ bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& proper wasChanged |= (in != out); } else { // an edit was rejected, so we stop here and return false - qCDebug(entities) << "Edit rejected by filter " << *it ; + qCDebug(entities) << "Edit rejected by filter " << id ; return false; } } @@ -117,16 +105,11 @@ bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& proper void EntityEditFilters::removeFilter(EntityItemID& entityID) { QWriteLocker writeLock(&_lock); - QScriptEngine* engine = _filterScriptEngineMap.value(entityID); - if (engine) { - _filterScriptEngineMap.remove(entityID); - delete engine; - } - FilterFunctionPair* pair = _filterFunctionMap.value(entityID); - if (pair) { - _filterFunctionMap.remove(entityID); - delete pair; + FilterData filterData = _filterDataMap.value(entityID); + if (filterData.valid()) { + delete filterData.engine; } + _filterDataMap.remove(entityID); } void EntityEditFilters::addFilter(EntityItemID& entityID, QString filterURL) { @@ -210,14 +193,12 @@ void EntityEditFilters::scriptRequestFinished(EntityItemID entityID) { engine->evaluate(scriptContents); if (!hadUncaughtExceptions(*engine, urlString)) { // put the engine in the engine map (so we don't leak them, etc...) - _lock.lockForWrite(); - _filterScriptEngineMap.insert(entityID, engine); - _lock.unlock(); - + FilterData filterData; + filterData.engine = engine; + // define the uncaughtException function - FilterFunctionPair* pair = new FilterFunctionPair(); QScriptEngine& engineRef = *engine; - pair->second = [this, &engineRef, &urlString]() { return hadUncaughtExceptions(engineRef, urlString); }; + filterData.uncaughtExceptions = [this, &engineRef, &urlString]() { return hadUncaughtExceptions(engineRef, urlString); }; // now get the filter function auto global = engine->globalObject(); @@ -226,13 +207,15 @@ void EntityEditFilters::scriptRequestFinished(EntityItemID entityID) { entitiesObject.setProperty("EDIT_FILTER_TYPE", EntityTree::FilterType::Edit); entitiesObject.setProperty("PHYSICS_FILTER_TYPE", EntityTree::FilterType::Physics); global.setProperty("Entities", entitiesObject); - pair->first = global.property("filter"); - if (!pair->first.isFunction()) { + filterData.filterFn = global.property("filter"); + if (!filterData.filterFn.isFunction()) { qDebug() << "Filter function specified but not found. Ignoring filter"; + delete engine; return; } + _lock.lockForWrite(); - _filterFunctionMap.insert(entityID, pair); + _filterDataMap.insert(entityID, filterData); _lock.unlock(); qDebug() << "script request filter processed for entity id " << entityID; diff --git a/libraries/entities/src/EntityEditFilters.h b/libraries/entities/src/EntityEditFilters.h index 36962838d8..282d00187a 100644 --- a/libraries/entities/src/EntityEditFilters.h +++ b/libraries/entities/src/EntityEditFilters.h @@ -23,11 +23,17 @@ #include "EntityItemProperties.h" #include "EntityTree.h" -typedef QPair> FilterFunctionPair; - class EntityEditFilters : public QObject, public Dependency { Q_OBJECT public: + struct FilterData { + QScriptValue filterFn; + std::function uncaughtExceptions; + QScriptEngine* engine; + + bool valid() { return (engine != nullptr && filterFn.isFunction() && uncaughtExceptions); } + }; + EntityEditFilters() {}; EntityEditFilters(EntityTreePointer tree ): _tree(tree) {}; @@ -51,8 +57,7 @@ private: QScriptValue _nullObjectForFilter{}; QReadWriteLock _lock; - QMap _filterFunctionMap; - QMap _filterScriptEngineMap; + QMap _filterDataMap; }; #endif //hifi_EntityEditFilters_h diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index edfabf84e9..ea81df3801 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -1805,7 +1805,7 @@ void EntityItemProperties::markAllChanged() { _parentIDChanged = true; _parentJointIndexChanged = true; - + _jointRotationsSetChanged = true; _jointRotationsChanged = true; _jointTranslationsSetChanged = true; From 9891d7b7d2528ce45f8bf3c98ad7c5b494c686bd Mon Sep 17 00:00:00 2001 From: David Kelly Date: Fri, 10 Feb 2017 15:24:58 -0700 Subject: [PATCH 11/19] build issue on linux/mac --- libraries/entities/src/EntityEditFilters.cpp | 4 ++-- libraries/entities/src/EntityEditFilters.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/entities/src/EntityEditFilters.cpp b/libraries/entities/src/EntityEditFilters.cpp index 4a84763a02..ddd9ea006a 100644 --- a/libraries/entities/src/EntityEditFilters.cpp +++ b/libraries/entities/src/EntityEditFilters.cpp @@ -103,7 +103,7 @@ bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& proper return true; } -void EntityEditFilters::removeFilter(EntityItemID& entityID) { +void EntityEditFilters::removeFilter(EntityItemID entityID) { QWriteLocker writeLock(&_lock); FilterData filterData = _filterDataMap.value(entityID); if (filterData.valid()) { @@ -112,7 +112,7 @@ void EntityEditFilters::removeFilter(EntityItemID& entityID) { _filterDataMap.remove(entityID); } -void EntityEditFilters::addFilter(EntityItemID& entityID, QString filterURL) { +void EntityEditFilters::addFilter(EntityItemID entityID, QString filterURL) { QUrl scriptURL(filterURL); diff --git a/libraries/entities/src/EntityEditFilters.h b/libraries/entities/src/EntityEditFilters.h index 282d00187a..8ad82621dd 100644 --- a/libraries/entities/src/EntityEditFilters.h +++ b/libraries/entities/src/EntityEditFilters.h @@ -37,8 +37,8 @@ public: EntityEditFilters() {}; EntityEditFilters(EntityTreePointer tree ): _tree(tree) {}; - void addFilter(EntityItemID& entityID, QString filterURL); - void removeFilter(EntityItemID& entityID); + void addFilter(EntityItemID entityID, QString filterURL); + void removeFilter(EntityItemID entityID); bool filter(glm::vec3& position, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, EntityTree::FilterType filterType); void rejectAll(bool state) {_rejectAll = state; } From 8a7a3926c563392adc28f074e5993105d2a726a4 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Fri, 10 Feb 2017 15:57:27 -0700 Subject: [PATCH 12/19] rest of issues w/linux and mac --- libraries/entities/src/EntityTree.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 4958932fe3..e1edc6bb20 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -929,7 +929,8 @@ bool EntityTree::filterProperties(EntityItemPointer& existingEntity, EntityItemP bool accepted = true; auto entityEditFilters = DependencyManager::get(); if (entityEditFilters) { - accepted = entityEditFilters->filter(existingEntity->getPosition(), propertiesIn, propertiesOut, wasChanged, filterType); + auto position = existingEntity->getPosition(); + accepted = entityEditFilters->filter(position, propertiesIn, propertiesOut, wasChanged, filterType); } return accepted; From 590cf6d79870b863c0ff3a41e560c48c0f237695 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Mon, 13 Feb 2017 12:20:47 -0700 Subject: [PATCH 13/19] whitespace --- assignment-client/src/entities/EntityServer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assignment-client/src/entities/EntityServer.cpp b/assignment-client/src/entities/EntityServer.cpp index b71242753d..7fbbefa596 100644 --- a/assignment-client/src/entities/EntityServer.cpp +++ b/assignment-client/src/entities/EntityServer.cpp @@ -308,7 +308,7 @@ void EntityServer::readAdditionalConfiguration(const QJsonObject& settingsSectio } void EntityServer::entityFilterAdded(EntityItemID id, bool success) { - if(id.isInvalidID()) { + if (id.isInvalidID()) { // this is the domain-wide entity filter, which we want to stop // the world for auto entityEditFilters = qobject_cast(sender()); From 156e365eff17d3d28feedc0128387739ccc30858 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Mon, 13 Feb 2017 15:02:31 -0700 Subject: [PATCH 14/19] update version since new property was added, things will appear broken if this version is used with old version (server or client). --- libraries/networking/src/udt/PacketHeaders.cpp | 2 +- libraries/networking/src/udt/PacketHeaders.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index b13b21ba3b..545f394490 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -49,7 +49,7 @@ PacketVersion versionForPacketType(PacketType packetType) { case PacketType::EntityEdit: case PacketType::EntityData: case PacketType::EntityPhysics: - return VERSION_ENTITIES_PHYSICS_PACKET; + return VERSION_ENTITIES_ZONE_FILTERS; case PacketType::EntityQuery: return static_cast(EntityQueryPacketVersion::JsonFilter); case PacketType::AvatarIdentity: diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 0b2c04b031..38af6078a7 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -204,6 +204,7 @@ const PacketVersion VERSION_ENTITIES_ARROW_ACTION = 64; const PacketVersion VERSION_ENTITIES_LAST_EDITED_BY = 65; const PacketVersion VERSION_ENTITIES_SERVER_SCRIPTS = 66; const PacketVersion VERSION_ENTITIES_PHYSICS_PACKET = 67; +const PacketVersion VERSION_ENTITIES_ZONE_FILTERS = 68; enum class EntityQueryPacketVersion: PacketVersion { JsonFilter = 18 From 342584b2a826bfc95d92c1e7b1a4eb81a33a5d35 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Tue, 14 Feb 2017 13:05:12 -0700 Subject: [PATCH 15/19] Filter failure mode updated The decision here is that all failed filters (syntax errors, 404s, bad urls etc...) lock out all edits for those without lock rights. If it is the domain-wide one, then that applies to entire domain. If a zone filter, then that applies to all edits in that zone. Also - zone filters don't apply to the zone itself. Other zone filters whose zones lie within that zone _do_ apply, in addition to the global one. --- .../src/entities/EntityServer.cpp | 8 +---- libraries/entities/src/EntityEditFilters.cpp | 34 ++++++++++++------- libraries/entities/src/EntityEditFilters.h | 10 +++--- libraries/entities/src/EntityTree.cpp | 4 +-- 4 files changed, 31 insertions(+), 25 deletions(-) diff --git a/assignment-client/src/entities/EntityServer.cpp b/assignment-client/src/entities/EntityServer.cpp index 7fbbefa596..425bea2c38 100644 --- a/assignment-client/src/entities/EntityServer.cpp +++ b/assignment-client/src/entities/EntityServer.cpp @@ -302,22 +302,16 @@ void EntityServer::readAdditionalConfiguration(const QJsonObject& settingsSectio // connect the filterAdded signal, and block edits until you hear back connect(entityEditFilters.data(), &EntityEditFilters::filterAdded, this, &EntityServer::entityFilterAdded); - entityEditFilters->rejectAll(true); entityEditFilters->addFilter(EntityItemID(), filterURL); } } void EntityServer::entityFilterAdded(EntityItemID id, bool success) { if (id.isInvalidID()) { - // this is the domain-wide entity filter, which we want to stop - // the world for - auto entityEditFilters = qobject_cast(sender()); if (success) { qDebug() << "entity edit filter for " << id << "added successfully"; - entityEditFilters->rejectAll(false); } else { - qDebug() << "entity edit filter unsuccessfully added, stopping..."; - stop(); + qDebug() << "entity edit filter unsuccessfully added, all edits will be rejected for those without lock rights."; } } } diff --git a/libraries/entities/src/EntityEditFilters.cpp b/libraries/entities/src/EntityEditFilters.cpp index ddd9ea006a..cda7f1bc59 100644 --- a/libraries/entities/src/EntityEditFilters.cpp +++ b/libraries/entities/src/EntityEditFilters.cpp @@ -42,20 +42,18 @@ QList EntityEditFilters::getZonesByPosition(glm::vec3& position) { return zones; } -bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, EntityTree::FilterType filterType) { +bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, + EntityTree::FilterType filterType, EntityItemID& itemID) { qCDebug(entities) << "in EntityEditFilters"; - // allows us to start entity server and reject all edits until filter is setup - if (_rejectAll) { - qCDebug(entities) << "rejecting all edits"; - return false; - } - // get the ids of all the zones (plus the global entity edit filter) that the position // lies within auto zoneIDs = getZonesByPosition(position); - for (auto id : zoneIDs) { + if (id == itemID) { + qCDebug(entities) << "zone filter not applied to its own edit"; + continue; + } qCDebug(entities) << "applying filter for zone" << id; // get the filter pair, etc... @@ -64,7 +62,10 @@ bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& proper _lock.unlock(); if (filterData.valid()) { - + if (filterData.rejectAll) { + qCDebug(entities) << "rejecting all edits"; + return false; + } auto oldProperties = propertiesIn.getDesiredProperties(); auto specifiedProperties = propertiesIn.getChangedProperties(); propertiesIn.setDesiredProperties(specifiedProperties); @@ -81,7 +82,6 @@ bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& proper result = QScriptValue(); } - if (result.isObject()){ qCDebug(entities) << "filter result accepted"; // make propertiesIn reflect the changes, for next filter... @@ -131,7 +131,15 @@ void EntityEditFilters::addFilter(EntityItemID entityID, QString filterURL) { // first remove any existing info for this entity removeFilter(entityID); + + // reject all edits until we load the script + FilterData filterData; + filterData.rejectAll = true; + _lock.lockForWrite(); + _filterDataMap.insert(entityID, filterData); + _lock.unlock(); + auto scriptRequest = ResourceManager::createResourceRequest(this, scriptURL); if (!scriptRequest) { qWarning() << "Could not create ResourceRequest for Entity Edit filter script at" << scriptURL.toString(); @@ -195,6 +203,7 @@ void EntityEditFilters::scriptRequestFinished(EntityItemID entityID) { // put the engine in the engine map (so we don't leak them, etc...) FilterData filterData; filterData.engine = engine; + filterData.rejectAll = false; // define the uncaughtException function QScriptEngine& engineRef = *engine; @@ -209,10 +218,11 @@ void EntityEditFilters::scriptRequestFinished(EntityItemID entityID) { global.setProperty("Entities", entitiesObject); filterData.filterFn = global.property("filter"); if (!filterData.filterFn.isFunction()) { - qDebug() << "Filter function specified but not found. Ignoring filter"; + qDebug() << "Filter function specified but not found. Will reject all edits for those without lock rights."; delete engine; - return; + filterData.rejectAll=true; } + _lock.lockForWrite(); _filterDataMap.insert(entityID, filterData); diff --git a/libraries/entities/src/EntityEditFilters.h b/libraries/entities/src/EntityEditFilters.h index 8ad82621dd..6aeb347603 100644 --- a/libraries/entities/src/EntityEditFilters.h +++ b/libraries/entities/src/EntityEditFilters.h @@ -30,8 +30,10 @@ public: QScriptValue filterFn; std::function uncaughtExceptions; QScriptEngine* engine; - - bool valid() { return (engine != nullptr && filterFn.isFunction() && uncaughtExceptions); } + bool rejectAll; + + FilterData(): engine(nullptr), rejectAll(false) {}; + bool valid() { return (rejectAll || (engine != nullptr && filterFn.isFunction() && uncaughtExceptions)); } }; EntityEditFilters() {}; @@ -40,8 +42,8 @@ public: void addFilter(EntityItemID entityID, QString filterURL); void removeFilter(EntityItemID entityID); - bool filter(glm::vec3& position, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, EntityTree::FilterType filterType); - void rejectAll(bool state) {_rejectAll = state; } + bool filter(glm::vec3& position, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, + EntityTree::FilterType filterType, EntityItemID& entityID); signals: void filterAdded(EntityItemID id, bool success); diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index e1edc6bb20..cd12b703f8 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -928,9 +928,9 @@ void EntityTree::fixupTerseEditLogging(EntityItemProperties& properties, QList(); - if (entityEditFilters) { + if (entityEditFilters && existingEntity) { auto position = existingEntity->getPosition(); - accepted = entityEditFilters->filter(position, propertiesIn, propertiesOut, wasChanged, filterType); + accepted = entityEditFilters->filter(position, propertiesIn, propertiesOut, wasChanged, filterType, existingEntity->getEntityItemID()); } return accepted; From 17acd8fa4fdfc458bc97c5139808cb9f37c07af3 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Tue, 14 Feb 2017 13:30:43 -0700 Subject: [PATCH 16/19] I should just build on linux exclusively --- libraries/entities/src/EntityTree.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index cd12b703f8..04f185ce97 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -930,7 +930,8 @@ bool EntityTree::filterProperties(EntityItemPointer& existingEntity, EntityItemP auto entityEditFilters = DependencyManager::get(); if (entityEditFilters && existingEntity) { auto position = existingEntity->getPosition(); - accepted = entityEditFilters->filter(position, propertiesIn, propertiesOut, wasChanged, filterType, existingEntity->getEntityItemID()); + auto entityID = existingEntity->getEntityItemID(); + accepted = entityEditFilters->filter(position, propertiesIn, propertiesOut, wasChanged, filterType, entityID); } return accepted; From 08d30608c7e237d25b6f95b5864c35a22e2ee529 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Tue, 14 Feb 2017 14:28:20 -0700 Subject: [PATCH 17/19] remove debug logging --- libraries/entities/src/EntityEditFilters.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/libraries/entities/src/EntityEditFilters.cpp b/libraries/entities/src/EntityEditFilters.cpp index cda7f1bc59..d58b996878 100644 --- a/libraries/entities/src/EntityEditFilters.cpp +++ b/libraries/entities/src/EntityEditFilters.cpp @@ -21,7 +21,6 @@ QList EntityEditFilters::getZonesByPosition(glm::vec3& position) { _lock.lockForRead(); auto zoneIDs = _filterDataMap.keys(); _lock.unlock(); - qCDebug(entities) << "looking at " << zoneIDs.size() << "possible zones, at " << position; for (auto id : zoneIDs) { if (!id.isInvalidID()) { // for now, look it up in the tree (soon we need to cache or similar?) @@ -44,17 +43,14 @@ QList EntityEditFilters::getZonesByPosition(glm::vec3& position) { bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, EntityTree::FilterType filterType, EntityItemID& itemID) { - qCDebug(entities) << "in EntityEditFilters"; // get the ids of all the zones (plus the global entity edit filter) that the position // lies within auto zoneIDs = getZonesByPosition(position); for (auto id : zoneIDs) { if (id == itemID) { - qCDebug(entities) << "zone filter not applied to its own edit"; continue; } - qCDebug(entities) << "applying filter for zone" << id; // get the filter pair, etc... _lock.lockForRead(); @@ -63,7 +59,6 @@ bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& proper if (filterData.valid()) { if (filterData.rejectAll) { - qCDebug(entities) << "rejecting all edits"; return false; } auto oldProperties = propertiesIn.getDesiredProperties(); @@ -83,7 +78,6 @@ bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& proper } if (result.isObject()){ - qCDebug(entities) << "filter result accepted"; // make propertiesIn reflect the changes, for next filter... propertiesIn.copyFromScriptValue(result, false); From e6969321caa45c57ea568e39290ceb0a9b55ac6b Mon Sep 17 00:00:00 2001 From: David Kelly Date: Wed, 15 Feb 2017 09:15:35 -0700 Subject: [PATCH 18/19] fix for not properly filtering entity adds --- libraries/entities/src/EntityEditFilters.cpp | 6 ++---- libraries/entities/src/EntityTree.cpp | 6 +++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/libraries/entities/src/EntityEditFilters.cpp b/libraries/entities/src/EntityEditFilters.cpp index d58b996878..3071072efc 100644 --- a/libraries/entities/src/EntityEditFilters.cpp +++ b/libraries/entities/src/EntityEditFilters.cpp @@ -48,7 +48,7 @@ bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& proper // lies within auto zoneIDs = getZonesByPosition(position); for (auto id : zoneIDs) { - if (id == itemID) { + if (!itemID.isInvalidID() && id == itemID) { continue; } @@ -74,7 +74,7 @@ bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& proper QScriptValue result = filterData.filterFn.call(_nullObjectForFilter, args); if (filterData.uncaughtExceptions()) { - result = QScriptValue(); + return false; } if (result.isObject()){ @@ -87,8 +87,6 @@ bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& proper auto out = QJsonValue::fromVariant(result.toVariant()); wasChanged |= (in != out); } else { - // an edit was rejected, so we stop here and return false - qCDebug(entities) << "Edit rejected by filter " << id ; return false; } } diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 04f185ce97..0f65ac55ff 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -928,9 +928,9 @@ void EntityTree::fixupTerseEditLogging(EntityItemProperties& properties, QList(); - if (entityEditFilters && existingEntity) { - auto position = existingEntity->getPosition(); - auto entityID = existingEntity->getEntityItemID(); + if (entityEditFilters) { + auto position = existingEntity ? existingEntity->getPosition() : propertiesIn.getPosition(); + auto entityID = existingEntity ? existingEntity->getEntityItemID() : EntityItemID(); accepted = entityEditFilters->filter(position, propertiesIn, propertiesOut, wasChanged, filterType, entityID); } From f0e87360c15fff5307517c6fb1ee86757f1fa348 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Wed, 15 Feb 2017 12:37:00 -0700 Subject: [PATCH 19/19] don't capture a temporary by reference. Ugh --- libraries/entities/src/EntityEditFilters.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/entities/src/EntityEditFilters.cpp b/libraries/entities/src/EntityEditFilters.cpp index 3071072efc..d62495d95e 100644 --- a/libraries/entities/src/EntityEditFilters.cpp +++ b/libraries/entities/src/EntityEditFilters.cpp @@ -199,7 +199,7 @@ void EntityEditFilters::scriptRequestFinished(EntityItemID entityID) { // define the uncaughtException function QScriptEngine& engineRef = *engine; - filterData.uncaughtExceptions = [this, &engineRef, &urlString]() { return hadUncaughtExceptions(engineRef, urlString); }; + filterData.uncaughtExceptions = [this, &engineRef, urlString]() { return hadUncaughtExceptions(engineRef, urlString); }; // now get the filter function auto global = engine->globalObject();