make use of stl containers and RAII

The QTimer class automatically stops the timer, so no need to do it automatically.

RTUnreal committed 4 months ago

Commit b87aa041 parent 42bbbaf2 files · +12 −45Browse files

Showing the changes to libraries/script-engine/src/ScriptManager.cpp (1 of 2 files). Show all

Unified · Split

libraries/script-engine/src/ScriptManager.cpp+11−44

 #include "ScriptManager.h"
 #include <chrono>
+#include <memory>
 #include <thread>
 #include <QtCore/QCoreApplication>
 // NOTE: This is private because it must be called on the same thread that created the timers, which is why
 // we want to only call it in our own run "shutdown" processing.
 void ScriptManager::stopAllTimers() {
-    for (const auto& pair : _timerFunctionMap) {
-        QTimer* timer = std::get<0>(pair);
-        timer->stop();
-        delete timer;
-    }
     _timerFunctionMap.clear();
     _timerHandleCounter = 1;
 }
 void ScriptManager::stopAllTimersForEntityScript(const EntityItemID& entityID) {
-    std::vector<int> toDelete = {};
+    std::erase_if(_timerFunctionMap, [entityID](auto& i) { return i.second.second.definingEntityIdentifier == entityID; });
-    for (
-        auto i = _timerFunctionMap.constKeyValueBegin();
-        i != _timerFunctionMap.constKeyValueEnd();
-        ++i
-    ) {
-        const auto& handle = i->first;
-        const auto& [timer, callbackData] = i->second;
-        if (callbackData.definingEntityIdentifier != entityID) { continue; }
-        timer->stop();
-        delete timer;
-        toDelete.push_back(handle);
-    }
-    for (auto handle : toDelete) {
-        _timerFunctionMap.remove(handle);
-    }
     // the timer map is potentially shared across multiple entity scripts,
     // so don't clear it or reset the handle counter
 }
         return;
     }
-    auto [timer, data] = _timerFunctionMap.value(handle);
+    auto& functionPair = _timerFunctionMap[handle];
+    auto& timer = functionPair.first;
+    auto data = functionPair.second;
     if (!timer->isActive()) {
         // this timer is done, we can kill it
-        _timerFunctionMap.remove(handle);
+        _timerFunctionMap.erase(handle);
-        delete timer;
     }
     // call the associated JS function, if it exists
     int handle = _timerHandleCounter++;
     // create the timer, add it to the map, and start it
-    QTimer* newTimer = new QTimer(this);
+    CallbackData timerData = { function, currentEntityIdentifier, currentSandboxURL };
+    auto& newTimer =
+        _timerFunctionMap.emplace(handle, std::make_pair(std::make_unique<QTimer>(), timerData)).first->second.first;
     newTimer->setSingleShot(isSingleShot);
     // The default timer type is not very accurate below about 200ms http://doc.qt.io/qt-5/qt.html#TimerType-enum
         newTimer->setTimerType(Qt::PreciseTimer);
     }
-    connect(newTimer, &QTimer::timeout, [this, handle]() { timerFired(handle); });
+    connect(newTimer.get(), &QTimer::timeout, [this, handle]() { timerFired(handle); });
-    // make sure the timer stops when the script does
-    connect(this, &ScriptManager::scriptEnding, newTimer, &QTimer::stop);
-    CallbackData timerData = { function, currentEntityIdentifier, currentSandboxURL };
-    _timerFunctionMap.insert(handle, { newTimer, timerData });
     if (intervalMS < 0) {
         int lineNumber = -1;
 void ScriptManager::stopTimer(int handle) {
     if (_timerFunctionMap.contains(handle)) {
-        auto [timer, callbackData] = _timerFunctionMap.value(handle);
+        _timerFunctionMap.erase(handle);
-        timer->stop();
-        _timerFunctionMap.remove(handle);
-        delete timer;
     } else {
         qCDebug(scriptengine) << "stopTimer -- not in _timerFunctionMap" << handle;
     }