From 8a49edbecbce565f8bea2173f68615abfda4bdf0 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 15 Sep 2017 10:08:35 -0700 Subject: [PATCH] code review --- assignment-client/src/Agent.h | 2 +- .../src/scripts/EntityScriptServer.h | 2 +- interface/src/Application.cpp | 6 ++-- interface/src/Application.h | 2 +- interface/src/avatar/MyAvatar.cpp | 2 +- interface/src/avatar/MyAvatar.h | 4 +-- interface/src/ui/JSConsole.cpp | 4 +-- interface/src/ui/JSConsole.h | 6 ++-- interface/src/ui/TestingDialog.h | 2 +- .../src/EntityTreeRenderer.h | 2 +- .../src/AbstractScriptingServicesInterface.h | 4 +-- libraries/script-engine/src/ScriptEngine.cpp | 6 ++-- libraries/script-engine/src/ScriptEngine.h | 8 ++--- libraries/script-engine/src/ScriptEngines.cpp | 34 +++++++++---------- libraries/script-engine/src/ScriptEngines.h | 18 +++++----- libraries/shared/src/BaseScriptEngine.h | 3 ++ 16 files changed, 54 insertions(+), 51 deletions(-) diff --git a/assignment-client/src/Agent.h b/assignment-client/src/Agent.h index cf18c0ed18..168da185b6 100644 --- a/assignment-client/src/Agent.h +++ b/assignment-client/src/Agent.h @@ -88,7 +88,7 @@ private: void encodeFrameOfZeros(QByteArray& encodedZeros); void computeLoudness(const QByteArray* decodedBuffer, QSharedPointer); - QSharedPointer _scriptEngine; + ScriptEnginePointer _scriptEngine; EntityEditPacketSender _entityEditSender; EntityTreeHeadlessViewer _entityViewer; diff --git a/assignment-client/src/scripts/EntityScriptServer.h b/assignment-client/src/scripts/EntityScriptServer.h index 84454375e5..e6bd12460b 100644 --- a/assignment-client/src/scripts/EntityScriptServer.h +++ b/assignment-client/src/scripts/EntityScriptServer.h @@ -72,7 +72,7 @@ private: bool _shuttingDown { false }; static int _entitiesScriptEngineCount; - QSharedPointer _entitiesScriptEngine; + ScriptEnginePointer _entitiesScriptEngine; EntityEditPacketSender _entityEditSender; EntityTreeHeadlessViewer _entityViewer; diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 86ed0a6666..b83e9c67b1 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -963,7 +963,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo DependencyManager::get().data(), &AddressManager::storeCurrentAddress); auto scriptEngines = DependencyManager::get().data(); - scriptEngines->registerScriptInitializer([this](QSharedPointer engine){ + scriptEngines->registerScriptInitializer([this](ScriptEnginePointer engine){ registerScriptEngineWithApplicationServices(engine); }); @@ -5930,7 +5930,7 @@ int Application::processOctreeStats(ReceivedMessage& message, SharedNodePointer void Application::packetSent(quint64 length) { } -void Application::registerScriptEngineWithApplicationServices(QSharedPointer scriptEngine) { +void Application::registerScriptEngineWithApplicationServices(ScriptEnginePointer scriptEngine) { scriptEngine->setEmitScriptUpdatesFunction([this]() { SharedNodePointer entityServerNode = DependencyManager::get()->soloNodeOfType(NodeType::EntityServer); @@ -5944,7 +5944,7 @@ void Application::registerScriptEngineWithApplicationServices(QSharedPointersetEntityTree(getEntities()->getTree()); // give the script engine to the RecordingScriptingInterface for its callbacks - DependencyManager::get()->setScriptEngine(QSharedPointer(scriptEngine)); + DependencyManager::get()->setScriptEngine(scriptEngine); if (property(hifi::properties::TEST).isValid()) { scriptEngine->registerGlobalObject("Test", TestScriptingInterface::getInstance()); diff --git a/interface/src/Application.h b/interface/src/Application.h index a8783e8e2b..86e4f94917 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -218,7 +218,7 @@ public: NodeToOctreeSceneStats* getOcteeSceneStats() { return &_octreeServerSceneStats; } virtual controller::ScriptingInterface* getControllerScriptingInterface() { return _controllerScriptingInterface; } - virtual void registerScriptEngineWithApplicationServices(QSharedPointer scriptEngine) override; + virtual void registerScriptEngineWithApplicationServices(ScriptEnginePointer scriptEngine) override; virtual void copyCurrentViewFrustum(ViewFrustum& viewOut) const override { copyDisplayViewFrustum(viewOut); } virtual QThread* getMainThread() override { return thread(); } diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 5b34ab6ea2..07d2ff007c 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -262,7 +262,7 @@ void MyAvatar::setDominantHand(const QString& hand) { } } -void MyAvatar::registerMetaTypes(QSharedPointer engine) { +void MyAvatar::registerMetaTypes(ScriptEnginePointer engine) { QScriptValue value = engine->newQObject(this, QScriptEngine::QtOwnership, QScriptEngine::ExcludeDeleteLater | QScriptEngine::ExcludeChildObjects); engine->globalObject().setProperty("MyAvatar", value); diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index c3d74a9609..7e9fadcc58 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -19,7 +19,7 @@ #include #include #include -#include +#include #include #include @@ -170,7 +170,7 @@ public: ~MyAvatar(); void instantiableAvatar() override {}; - void registerMetaTypes(QSharedPointer engine); + void registerMetaTypes(ScriptEnginePointer engine); virtual void simulateAttachments(float deltaTime) override; diff --git a/interface/src/ui/JSConsole.cpp b/interface/src/ui/JSConsole.cpp index 6b079cbc93..c5f8b54ebd 100644 --- a/interface/src/ui/JSConsole.cpp +++ b/interface/src/ui/JSConsole.cpp @@ -61,7 +61,7 @@ void _writeLines(const QString& filename, const QList& lines) { QTextStream(&file) << json; } -JSConsole::JSConsole(QWidget* parent, const QSharedPointer& scriptEngine) : +JSConsole::JSConsole(QWidget* parent, const ScriptEnginePointer& scriptEngine) : QWidget(parent), _ui(new Ui::Console), _currentCommandInHistory(NO_CURRENT_HISTORY_COMMAND), @@ -97,7 +97,7 @@ JSConsole::~JSConsole() { delete _ui; } -void JSConsole::setScriptEngine(const QSharedPointer& scriptEngine) { +void JSConsole::setScriptEngine(const ScriptEnginePointer& scriptEngine) { if (_scriptEngine == scriptEngine && scriptEngine != NULL) { return; } diff --git a/interface/src/ui/JSConsole.h b/interface/src/ui/JSConsole.h index 5010b5b9ca..4b6409a76f 100644 --- a/interface/src/ui/JSConsole.h +++ b/interface/src/ui/JSConsole.h @@ -30,10 +30,10 @@ const int CONSOLE_HEIGHT = 200; class JSConsole : public QWidget { Q_OBJECT public: - JSConsole(QWidget* parent, const QSharedPointer& scriptEngine = QSharedPointer()); + JSConsole(QWidget* parent, const ScriptEnginePointer& scriptEngine = ScriptEnginePointer()); ~JSConsole(); - void setScriptEngine(const QSharedPointer& scriptEngine = QSharedPointer()); + void setScriptEngine(const ScriptEnginePointer& scriptEngine = ScriptEnginePointer()); void clear(); public slots: @@ -66,7 +66,7 @@ private: QString _savedHistoryFilename; QList _commandHistory; QString _rootCommand; - QSharedPointer _scriptEngine; + ScriptEnginePointer _scriptEngine; static const QString _consoleFileName; }; diff --git a/interface/src/ui/TestingDialog.h b/interface/src/ui/TestingDialog.h index 055e43eaf7..a7e909ca0e 100644 --- a/interface/src/ui/TestingDialog.h +++ b/interface/src/ui/TestingDialog.h @@ -29,7 +29,7 @@ public: private: std::unique_ptr _console; - QSharedPointer _engine; + ScriptEnginePointer _engine; }; #endif diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.h b/libraries/entities-renderer/src/EntityTreeRenderer.h index 0950974347..05ddaa6b0a 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.h +++ b/libraries/entities-renderer/src/EntityTreeRenderer.h @@ -181,7 +181,7 @@ private: QVector _currentEntitiesInside; bool _wantScripts; - QSharedPointer _entitiesScriptEngine; + ScriptEnginePointer _entitiesScriptEngine; void playEntityCollisionSound(const EntityItemPointer& entity, const Collision& collision); diff --git a/libraries/script-engine/src/AbstractScriptingServicesInterface.h b/libraries/script-engine/src/AbstractScriptingServicesInterface.h index afc3bb1227..e7a8d11562 100644 --- a/libraries/script-engine/src/AbstractScriptingServicesInterface.h +++ b/libraries/script-engine/src/AbstractScriptingServicesInterface.h @@ -12,13 +12,13 @@ #ifndef hifi_AbstractScriptingServicesInterface_h #define hifi_AbstractScriptingServicesInterface_h -class ScriptEngine; +#include /// Interface provided by Application to other objects that need access to scripting services of the application class AbstractScriptingServicesInterface { public: /// Registers application specific services with a script engine. - virtual void registerScriptEngineWithApplicationServices(QSharedPointer scriptEngine) = 0; + virtual void registerScriptEngineWithApplicationServices(ScriptEnginePointer scriptEngine) = 0; }; diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index ba75ed9856..2260cea616 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -153,11 +153,11 @@ QString ScriptEngine::logException(const QScriptValue& exception) { return message; } -QSharedPointer scriptEngineFactory(ScriptEngine::Context context, +ScriptEnginePointer scriptEngineFactory(ScriptEngine::Context context, const QString& scriptContents, const QString& fileNameString) { ScriptEngine* engine = new ScriptEngine(context, scriptContents, fileNameString); - QSharedPointer engineSP = QSharedPointer(engine); + ScriptEnginePointer engineSP = ScriptEnginePointer(engine); DependencyManager::get()->addScriptEngine(qSharedPointerCast(engineSP)); return engineSP; } @@ -537,7 +537,7 @@ static void scriptableResourceFromScriptValue(const QScriptValue& value, Scripta resource = static_cast(value.toQObject()); } -static QScriptValue createScriptableResourcePrototype(QSharedPointer engine) { +static QScriptValue createScriptableResourcePrototype(ScriptEnginePointer engine) { auto prototype = engine->newObject(); // Expose enum State to JS/QML via properties diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index e743753da1..7109e0f582 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -244,7 +244,7 @@ signals: void errorLoadingScript(const QString& scriptFilename); void update(float deltaTime); void scriptEnding(); - void finished(const QString& fileNameString, QSharedPointer); + void finished(const QString& fileNameString, ScriptEnginePointer); void cleanupMenuItem(const QString& menuItemString); void printedMessage(const QString& message, const QString& scriptName); void errorMessage(const QString& message, const QString& scriptName); @@ -334,8 +334,8 @@ protected: QSharedPointer _scriptEngines; }; -QSharedPointer scriptEngineFactory(ScriptEngine::Context context, - const QString& scriptContents, - const QString& fileNameString); +ScriptEnginePointer scriptEngineFactory(ScriptEngine::Context context, + const QString& scriptContents, + const QString& fileNameString); #endif // hifi_ScriptEngine_h diff --git a/libraries/script-engine/src/ScriptEngines.cpp b/libraries/script-engine/src/ScriptEngines.cpp index 686d8cae03..6cfa678652 100644 --- a/libraries/script-engine/src/ScriptEngines.cpp +++ b/libraries/script-engine/src/ScriptEngines.cpp @@ -137,14 +137,14 @@ void ScriptEngines::registerScriptInitializer(ScriptInitializer initializer) { _scriptInitializers.push_back(initializer); } -void ScriptEngines::addScriptEngine(QSharedPointer engine) { +void ScriptEngines::addScriptEngine(ScriptEnginePointer engine) { if (!_isStopped) { QMutexLocker locker(&_allScriptsMutex); _allKnownScriptEngines.insert(engine); } } -void ScriptEngines::removeScriptEngine(QSharedPointer engine) { +void ScriptEngines::removeScriptEngine(ScriptEnginePointer engine) { // If we're not already in the middle of stopping all scripts, then we should remove ourselves // from the list of running scripts. We don't do this if we're in the process of stopping all scripts // because that method removes scripts from its list as it iterates them @@ -159,9 +159,9 @@ void ScriptEngines::shutdownScripting() { QMutexLocker locker(&_allScriptsMutex); qCDebug(scriptengine) << "Stopping all scripts.... currently known scripts:" << _allKnownScriptEngines.size(); - QMutableSetIterator> i(_allKnownScriptEngines); + QMutableSetIterator i(_allKnownScriptEngines); while (i.hasNext()) { - QSharedPointer scriptEngine = i.next(); + ScriptEnginePointer scriptEngine = i.next(); QString scriptName = scriptEngine->getFilename(); // NOTE: typically all script engines are running. But there's at least one known exception to this, the @@ -364,9 +364,9 @@ void ScriptEngines::stopAllScripts(bool restart) { _isReloading = true; } - for (QHash>::const_iterator it = _scriptEnginesHash.constBegin(); + for (QHash::const_iterator it = _scriptEnginesHash.constBegin(); it != _scriptEnginesHash.constEnd(); it++) { - QSharedPointer scriptEngine = it.value(); + ScriptEnginePointer scriptEngine = it.value(); // skip already stopped scripts if (scriptEngine->isFinished() || scriptEngine->isStopping()) { continue; @@ -412,12 +412,12 @@ bool ScriptEngines::stopScript(const QString& rawScriptURL, bool restart) { QReadLocker lock(&_scriptEnginesHashLock); if (_scriptEnginesHash.contains(scriptURL)) { - QSharedPointer scriptEngine = _scriptEnginesHash[scriptURL]; + ScriptEnginePointer scriptEngine = _scriptEnginesHash[scriptURL]; if (restart) { auto scriptCache = DependencyManager::get(); scriptCache->deleteScript(scriptURL); connect(scriptEngine.data(), &ScriptEngine::finished, - this, [this](QString scriptName, QSharedPointer engine) { + this, [this](QString scriptName, ScriptEnginePointer engine) { reloadScript(scriptName); }); } @@ -445,11 +445,11 @@ void ScriptEngines::reloadAllScripts() { stopAllScripts(true); } -QSharedPointer ScriptEngines::loadScript(const QUrl& scriptFilename, bool isUserLoaded, bool loadScriptFromEditor, +ScriptEnginePointer ScriptEngines::loadScript(const QUrl& scriptFilename, bool isUserLoaded, bool loadScriptFromEditor, bool activateMainWindow, bool reload) { if (thread() != QThread::currentThread()) { - QSharedPointer result { nullptr }; - BLOCKING_INVOKE_METHOD(this, "loadScript", Q_RETURN_ARG(QSharedPointer, result), + ScriptEnginePointer result { nullptr }; + BLOCKING_INVOKE_METHOD(this, "loadScript", Q_RETURN_ARG(ScriptEnginePointer, result), Q_ARG(QUrl, scriptFilename), Q_ARG(bool, isUserLoaded), Q_ARG(bool, loadScriptFromEditor), @@ -475,7 +475,7 @@ QSharedPointer ScriptEngines::loadScript(const QUrl& scriptFilenam return scriptEngine; } - scriptEngine = QSharedPointer(new ScriptEngine(_context, NO_SCRIPT, "about:" + scriptFilename.fileName())); + scriptEngine = ScriptEnginePointer(new ScriptEngine(_context, NO_SCRIPT, "about:" + scriptFilename.fileName())); addScriptEngine(scriptEngine); scriptEngine->setUserLoaded(isUserLoaded); @@ -493,8 +493,8 @@ QSharedPointer ScriptEngines::loadScript(const QUrl& scriptFilenam return scriptEngine; } -QSharedPointer ScriptEngines::getScriptEngine(const QUrl& rawScriptURL) { - QSharedPointer result; +ScriptEnginePointer ScriptEngines::getScriptEngine(const QUrl& rawScriptURL) { + ScriptEnginePointer result; { QReadLocker lock(&_scriptEnginesHashLock); const QUrl scriptURL = normalizeScriptURL(rawScriptURL); @@ -510,7 +510,7 @@ QSharedPointer ScriptEngines::getScriptEngine(const QUrl& rawScrip void ScriptEngines::onScriptEngineLoaded(const QString& rawScriptURL) { UserActivityLogger::getInstance().loadedScript(rawScriptURL); QSharedPointer baseScriptEngine = qobject_cast(sender())->sharedFromThis(); - QSharedPointer scriptEngine = qSharedPointerCast(baseScriptEngine); + ScriptEnginePointer scriptEngine = qSharedPointerCast(baseScriptEngine); launchScriptEngine(scriptEngine); @@ -526,7 +526,7 @@ void ScriptEngines::onScriptEngineLoaded(const QString& rawScriptURL) { emit scriptCountChanged(); } -void ScriptEngines::launchScriptEngine(QSharedPointer scriptEngine) { +void ScriptEngines::launchScriptEngine(ScriptEnginePointer scriptEngine) { connect(scriptEngine.data(), &ScriptEngine::finished, this, &ScriptEngines::onScriptFinished, Qt::DirectConnection); connect(scriptEngine.data(), &ScriptEngine::loadScript, [&](const QString& scriptName, bool userLoaded) { loadScript(scriptName, userLoaded); @@ -552,7 +552,7 @@ void ScriptEngines::launchScriptEngine(QSharedPointer scriptEngine } } -void ScriptEngines::onScriptFinished(const QString& rawScriptURL, QSharedPointer engine) { +void ScriptEngines::onScriptFinished(const QString& rawScriptURL, ScriptEnginePointer engine) { bool removed = false; { QWriteLocker lock(&_scriptEnginesHashLock); diff --git a/libraries/script-engine/src/ScriptEngines.h b/libraries/script-engine/src/ScriptEngines.h index 396b5cc67f..f2b0105be1 100644 --- a/libraries/script-engine/src/ScriptEngines.h +++ b/libraries/script-engine/src/ScriptEngines.h @@ -33,7 +33,7 @@ class ScriptEngines : public QObject, public Dependency { Q_PROPERTY(ScriptsModelFilter* scriptsModelFilter READ scriptsModelFilter CONSTANT) public: - using ScriptInitializer = std::function)>; + using ScriptInitializer = std::function; ScriptEngines(ScriptEngine::Context context); void registerScriptInitializer(ScriptInitializer initializer); @@ -45,7 +45,7 @@ public: void loadDefaultScripts(); void setScriptsLocation(const QString& scriptsLocation); QStringList getRunningScripts(); - QSharedPointer getScriptEngine(const QUrl& scriptHash); + ScriptEnginePointer getScriptEngine(const QUrl& scriptHash); ScriptsModel* scriptsModel() { return &_scriptsModel; }; ScriptsModelFilter* scriptsModelFilter() { return &_scriptsModelFilter; }; @@ -53,7 +53,7 @@ public: QString getDefaultScriptsLocation() const; Q_INVOKABLE void loadOneScript(const QString& scriptFilename); - Q_INVOKABLE QSharedPointer loadScript(const QUrl& scriptFilename = QString(), + Q_INVOKABLE ScriptEnginePointer loadScript(const QUrl& scriptFilename = QString(), bool isUserLoaded = true, bool loadScriptFromEditor = false, bool activateMainWindow = false, bool reload = false); Q_INVOKABLE bool stopScript(const QString& scriptHash, bool restart = false); @@ -72,7 +72,7 @@ public: void shutdownScripting(); bool isStopped() const { return _isStopped; } - void addScriptEngine(QSharedPointer); + void addScriptEngine(ScriptEnginePointer); signals: void scriptCountChanged(); @@ -94,21 +94,21 @@ public slots: void onClearDebugWindow(); protected slots: - void onScriptFinished(const QString& fileNameString, QSharedPointer engine); + void onScriptFinished(const QString& fileNameString, ScriptEnginePointer engine); protected: friend class ScriptEngine; void reloadScript(const QString& scriptName) { loadScript(scriptName, true, false, false, true); } - void removeScriptEngine(QSharedPointer); + void removeScriptEngine(ScriptEnginePointer); void onScriptEngineLoaded(const QString& scriptFilename); void onScriptEngineError(const QString& scriptFilename); - void launchScriptEngine(QSharedPointer); + void launchScriptEngine(ScriptEnginePointer); ScriptEngine::Context _context; QReadWriteLock _scriptEnginesHashLock; - QHash> _scriptEnginesHash; - QSet> _allKnownScriptEngines; + QHash _scriptEnginesHash; + QSet _allKnownScriptEngines; QMutex _allScriptsMutex; std::list _scriptInitializers; mutable Setting::Handle _scriptsLocationHandle; diff --git a/libraries/shared/src/BaseScriptEngine.h b/libraries/shared/src/BaseScriptEngine.h index 138e46fafa..8820a386bf 100644 --- a/libraries/shared/src/BaseScriptEngine.h +++ b/libraries/shared/src/BaseScriptEngine.h @@ -16,6 +16,9 @@ #include #include +class ScriptEngine; +using ScriptEnginePointer = QSharedPointer; + // common base class for extending QScriptEngine itself class BaseScriptEngine : public QScriptEngine, public QEnableSharedFromThis { Q_OBJECT