clean up comments remove dead code

interface/src/Application.cpp

@@ -3541,6 +3541,9 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEngine* scri
     scriptEngine->registerGlobalObject("MIDI", &MIDIManager::getInstance());
 #endif
 
+    // TODO: Consider moving some of this functionality into the ScriptEngine class instead. It seems wrong that this
+    // work is being done in the Application class when really these dependencies are more related to the ScriptEngine's
+    // implementation
     QThread* workerThread = new QThread(this);
     QString scriptEngineName = QString("Script Thread:") + scriptEngine->getFilename();
     workerThread->setObjectName(scriptEngineName);
@@ -3552,9 +3555,6 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEngine* scri
     connect(scriptEngine, SIGNAL(doneRunning()), scriptEngine, SLOT(deleteLater()));
     connect(workerThread, SIGNAL(finished()), workerThread, SLOT(deleteLater()));
 
-    // when the application is about to quit, stop our script engine so it unwinds properly
-    //connect(this, SIGNAL(aboutToQuit()), scriptEngine, SLOT(stop()));
-
     auto nodeList = DependencyManager::get<NodeList>();
     connect(nodeList.data(), &NodeList::nodeKilled, scriptEngine, &ScriptEngine::nodeKilled);
 

libraries/entities-renderer/src/EntityTreeRenderer.cpp

@@ -59,14 +59,14 @@ EntityTreeRenderer::EntityTreeRenderer(bool wantScripts, AbstractViewStateInterf
 }
 
 EntityTreeRenderer::~EntityTreeRenderer() {
-    // NOTE: we don't need to delete _entitiesScriptEngine because it's owned by the application and gets cleaned up 
+    // NOTE: we don't need to delete _entitiesScriptEngine because it is registered with the application and has a 
-    // automatically but we do need to delete our sandbox script engine.
+    // signal tied to call it's deleteLater on doneRunning
     if (_sandboxScriptEngine) {
-        // NOTE: is it possible this is a problem? I think that we hook the script engine object up to a deleteLater()
+        // TODO: consider reworking how _sandboxScriptEngine is managed. It's treated differently than _entitiesScriptEngine
-        // call inside of registerScriptEngineWithApplicationServices() but do we not call that for _sandboxScriptEngine???
+        // because we don't call registerScriptEngineWithApplicationServices() for it. This implementation is confusing and 
-        // this _sandboxScriptEngine implementation is confusing and potentially error prone because it's not a full fledged
+        // potentially error prone because it's not a full fledged ScriptEngine that has been fully connected to the 
-        // ScriptEngine that has been fully connected. We did this so that scripts that were ill-formed could be evaluated
+        // application. We did this so that scripts that were ill-formed could be evaluated but not execute against the 
-        // but not execute against the application.
+        // application services. But this means it's shutdown behavior is different from other ScriptEngines
         delete _sandboxScriptEngine;
         _sandboxScriptEngine = NULL;
     }

libraries/script-engine/src/ScriptEngine.cpp

@@ -135,16 +135,31 @@ void ScriptEngine::stopAllScripts(QObject* application) {
             // complete. After that we can handle the stop process appropriately
             if (scriptEngine->evaluatePending()) {
                 while (scriptEngine->evaluatePending()) {
+                
+                    // This event loop allows any started, but not yet finished evaluate() calls to complete
+                    // we need to let these complete so that we can be guaranteed that the script engine isn't
+                    // in a partially setup state, which can confuse our shutdown unwinding.
                     QEventLoop loop;
                     QObject::connect(scriptEngine, &ScriptEngine::evaluationFinished, &loop, &QEventLoop::quit);
                     loop.exec();
                 }
             }
 
+            // We disconnect any script engine signals from the application because we don't want to do any 
+            // extra stopScript/loadScript processing that the Application normally does when scripts start
+            // and stop. We can safely short circuit this because we know we're in the "quitting" process
             scriptEngine->disconnect(application);
+            
+            // Calling stop on the script engine will set it's internal _isFinished state to true, and result
+            // in the ScriptEngine gracefully ending it's run() method.
             scriptEngine->stop();
 
+            // We need to wait for the engine to be done running before we proceed, because we don't
+            // want any of the scripts final "scriptEnding()" or pending "update()" methods from accessing
+            // any application state after we leave this stopAllScripts() method
             scriptEngine->waitTillDoneRunning();
+            
+            // If the script is stopped, we can remove it from our set
             i.remove();
         }
     }
@@ -156,9 +171,10 @@ void ScriptEngine::stopAllScripts(QObject* application) {
 void ScriptEngine::waitTillDoneRunning() {
     QString scriptName = getFilename();
 
+    // If the script never started running or finished running before we got here, we don't need to wait for it
     if (_isRunning) {
-        _doneRunningThisScript = false;
+
-        _isWaitingForDoneRunning = true;
+        _doneRunningThisScript = false; // NOTE: this is static, we serialize our waiting for scripts to finish
         
         // What can we do here???
         // we will be calling this on the main Application thread, inside of stopAllScripts()
@@ -178,8 +194,6 @@ void ScriptEngine::waitTillDoneRunning() {
                 break;
             }
         }
-
-        _isWaitingForDoneRunning = false;
     }
 }
 

libraries/script-engine/src/ScriptEngine.h

@@ -136,7 +136,6 @@ protected:
     bool _isFinished;
     bool _isRunning;
     int _evaluatesPending = 0;
-    bool _isWaitingForDoneRunning = false;
     bool _isInitialized;
     bool _isAvatar;
     QTimer* _avatarIdentityTimer;