From d5c0c05ab2bfcaa534ef6c22a207e28386041f5d Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 20 Sep 2018 14:27:24 -0700 Subject: [PATCH 1/3] wait for an entity's script to load before adding it to the contain-avatar list. do this so the script doesn't miss the 'enterEntity' entity-method due to not being loaded. --- libraries/entities-renderer/src/EntityTreeRenderer.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index 3d782f69a7..4bbc09ff8a 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -512,7 +512,11 @@ bool EntityTreeRenderer::findBestZoneAndMaybeContainingEntities(QVectorshouldPreloadScript())) { // now check to see if the point contains our entity, this can be expensive if // the entity has a collision hull if (entity->contains(_avatarPosition)) { From f6e57f54b00082d12b9983f4ac914312d6e64d8b Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 21 Sep 2018 10:26:31 -0700 Subject: [PATCH 2/3] don't call enterEntity until script preload has finished --- .../src/EntityTreeRenderer.cpp | 8 ++++++- libraries/entities/src/EntityItem.cpp | 23 +++++++++++++++++++ libraries/entities/src/EntityItem.h | 10 ++++---- libraries/script-engine/src/ScriptEngine.cpp | 2 ++ libraries/script-engine/src/ScriptEngine.h | 7 ++++++ 5 files changed, 45 insertions(+), 5 deletions(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index 4bbc09ff8a..c78036d5ed 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -187,6 +187,11 @@ void EntityTreeRenderer::resetEntitiesScriptEngine() { connect(entityScriptingInterface.data(), &EntityScriptingInterface::hoverLeaveEntity, _entitiesScriptEngine.data(), [&](const EntityItemID& entityID, const PointerEvent& event) { _entitiesScriptEngine->callEntityScriptMethod(entityID, "hoverLeaveEntity", event); }); + + connect(_entitiesScriptEngine.data(), &ScriptEngine::entityScriptPreloadFinished, [&](const EntityItemID& entityID) { + EntityItemPointer entity = getTree()->findEntityByID(entityID); + entity->setScriptHasFinishedPreload(true); + }); } void EntityTreeRenderer::clear() { @@ -516,7 +521,7 @@ bool EntityTreeRenderer::findBestZoneAndMaybeContainingEntities(QVectorshouldPreloadScript())) { + (hasScript && entity->isScriptPreloadFinished())) { // now check to see if the point contains our entity, this can be expensive if // the entity has a collision hull if (entity->contains(_avatarPosition)) { @@ -976,6 +981,7 @@ void EntityTreeRenderer::checkAndCallPreload(const EntityItemID& entityID, bool entity->scriptHasUnloaded(); } if (shouldLoad) { + entity->setScriptHasFinishedPreload(false); _entitiesScriptEngine->loadEntityScript(entityID, resolveScriptURL(scriptUrl), reload); entity->scriptHasPreloaded(); } diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 8e382fabd4..7a0e61b29a 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -3197,3 +3197,26 @@ void EntityItem::setCloneIDs(const QVector& cloneIDs) { _cloneIDs = cloneIDs; }); } + +bool EntityItem::shouldPreloadScript() const { + return !_script.isEmpty() && ((_loadedScript != _script) || (_loadedScriptTimestamp != _scriptTimestamp)); +} + +void EntityItem::scriptHasPreloaded() { + _loadedScript = _script; + _loadedScriptTimestamp = _scriptTimestamp; +} + +void EntityItem::scriptHasUnloaded() { + _loadedScript = ""; + _loadedScriptTimestamp = 0; + _scriptPreloadFinished = false; +} + +void EntityItem::setScriptHasFinishedPreload(bool value) { + _scriptPreloadFinished = value; +} + +bool EntityItem::isScriptPreloadFinished() { + return _scriptPreloadFinished; +} diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index 490f9b9e6b..405b114ab3 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -470,10 +470,11 @@ public: /// We only want to preload if: /// there is some script, and either the script value or the scriptTimestamp /// value have changed since our last preload - bool shouldPreloadScript() const { return !_script.isEmpty() && - ((_loadedScript != _script) || (_loadedScriptTimestamp != _scriptTimestamp)); } - void scriptHasPreloaded() { _loadedScript = _script; _loadedScriptTimestamp = _scriptTimestamp; } - void scriptHasUnloaded() { _loadedScript = ""; _loadedScriptTimestamp = 0; } + bool shouldPreloadScript() const; + void scriptHasPreloaded(); + void scriptHasUnloaded(); + void setScriptHasFinishedPreload(bool value); + bool isScriptPreloadFinished(); bool getClientOnly() const { return _clientOnly; } virtual void setClientOnly(bool clientOnly) { _clientOnly = clientOnly; } @@ -584,6 +585,7 @@ protected: QString _script { ENTITY_ITEM_DEFAULT_SCRIPT }; /// the value of the script property QString _loadedScript; /// the value of _script when the last preload signal was sent quint64 _scriptTimestamp { ENTITY_ITEM_DEFAULT_SCRIPT_TIMESTAMP }; /// the script loaded property used for forced reload + bool _scriptPreloadFinished { false }; QString _serverScripts; /// keep track of time when _serverScripts property was last changed diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index cfd155e14b..4d395070d6 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -2442,6 +2442,8 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co // if we got this far, then call the preload method callEntityScriptMethod(entityID, "preload"); + emit entityScriptPreloadFinished(entityID); + _occupiedScriptURLs.remove(entityScript); processDeferredEntityLoads(entityScript, entityID); } diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 08e2c492e8..17afd3dbbd 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -712,6 +712,13 @@ signals: // script is updated (goes from RUNNING to ERROR_RUNNING_SCRIPT, for example) void entityScriptDetailsUpdated(); + /**jsdoc + * @function Script.entityScriptPreloadFinished + * @returns {Signal} + */ + // Emitted when an entity script has finished running preload + void entityScriptPreloadFinished(const EntityItemID& entityID); + protected: void init(); From 6eb3fa251dbd59420b74a334462023c35961b1d6 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 21 Sep 2018 10:39:41 -0700 Subject: [PATCH 3/3] guard against null deref --- libraries/entities-renderer/src/EntityTreeRenderer.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index c78036d5ed..dbbf8af4b9 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -190,7 +190,9 @@ void EntityTreeRenderer::resetEntitiesScriptEngine() { connect(_entitiesScriptEngine.data(), &ScriptEngine::entityScriptPreloadFinished, [&](const EntityItemID& entityID) { EntityItemPointer entity = getTree()->findEntityByID(entityID); - entity->setScriptHasFinishedPreload(true); + if (entity) { + entity->setScriptHasFinishedPreload(true); + } }); }