change timers to not use ptrs (#1399)

add comments as to why logic may seem confusing.

dGame/Entity.cpp

@@ -1222,39 +1222,56 @@ void Entity::UpdateXMLDoc(tinyxml2::XMLDocument* doc) {
 
 void Entity::Update(const float deltaTime) {
 	uint32_t timerPosition;
-	timerPosition = 0;
-	while (timerPosition < m_Timers.size()) {
-		m_Timers[timerPosition]->Update(deltaTime);
-		if (m_Timers[timerPosition]->GetTime() <= 0) {
-			const auto timerName = m_Timers[timerPosition]->GetName();
-
-			delete m_Timers[timerPosition];
+	for (timerPosition = 0; timerPosition < m_Timers.size();) {
+		auto& timer = m_Timers[timerPosition];
+		timer.Update(deltaTime);
+		// If the timer is expired, erase it and dont increment the position because the next timer will be at the same position.
+		// Before: [0, 1, 2, 3, ..., n]
+		// timerPosition  ^
+		// After:  [0, 1, 3, ..., n]
+		// timerPosition  ^
+		if (timer.GetTime() <= 0) {
+			// Remove the timer from the list of timers first so that scripts and events can remove timers without causing iterator invalidation
+			auto timerName = timer.GetName();
 			m_Timers.erase(m_Timers.begin() + timerPosition);
-
 			for (CppScripts::Script* script : CppScripts::GetEntityScripts(this)) {
 				script->OnTimerDone(this, timerName);
 			}
+
 			TriggerEvent(eTriggerEventType::TIMER_DONE, this);
+		} else {
+			// If the timer isnt expired, go to the next timer.
+			timerPosition++;
+		}
+	}
+
+	for (timerPosition = 0; timerPosition < m_CallbackTimers.size(); ) {
+		// If the timer is expired, erase it and dont increment the position because the next timer will be at the same position.
+		// Before: [0, 1, 2, 3, ..., n]
+		// timerPosition  ^
+		// After:  [0, 1, 3, ..., n]
+		// timerPosition  ^
+		auto& callbackTimer = m_CallbackTimers[timerPosition];
+		callbackTimer.Update(deltaTime);
+		if (callbackTimer.GetTime() <= 0) {
+			// Remove the timer from the list of timers first so that callbacks can remove timers without causing iterator invalidation
+			auto callback = callbackTimer.GetCallback();
+			m_CallbackTimers.erase(m_CallbackTimers.begin() + timerPosition);
+			callback();
 		} else {
 			timerPosition++;
 		}
 	}
 
-	for (int i = 0; i < m_CallbackTimers.size(); i++) {
-		m_CallbackTimers[i]->Update(deltaTime);
-		if (m_CallbackTimers[i]->GetTime() <= 0) {
-			m_CallbackTimers[i]->GetCallback()();
-			delete m_CallbackTimers[i];
-			m_CallbackTimers.erase(m_CallbackTimers.begin() + i);
-		}
+	// Add pending timers to the list of timers so they start next tick.
+	if (!m_PendingTimers.empty()) {
+		m_Timers.insert(m_Timers.end(), m_PendingTimers.begin(), m_PendingTimers.end());
+		m_PendingTimers.clear();
 	}
 
-	// Add pending timers to the list of timers so they start next tick.
-	if (m_PendingTimers.size() > 0) {
-		for (auto namedTimer : m_PendingTimers) {
-			m_Timers.push_back(namedTimer);
-		}
-		m_PendingTimers.clear();
+	if (!m_PendingCallbackTimers.empty()) {
+		m_CallbackTimers.insert(m_CallbackTimers.end(), m_PendingCallbackTimers.begin(), m_PendingCallbackTimers.end());
+		m_PendingCallbackTimers.clear();
 	}
 
 	if (IsSleeping()) {
@@ -1692,31 +1709,20 @@ void Entity::RemoveParent() {
 }
 
 void Entity::AddTimer(std::string name, float time) {
-	EntityTimer* timer = new EntityTimer(name, time);
-	m_PendingTimers.push_back(timer);
+	m_PendingTimers.emplace_back(name, time);
 }
 
 void Entity::AddCallbackTimer(float time, std::function<void()> callback) {
-	EntityCallbackTimer* timer = new EntityCallbackTimer(time, callback);
-	m_CallbackTimers.push_back(timer);
+	m_PendingCallbackTimers.emplace_back(time, callback);
 }
 
 bool Entity::HasTimer(const std::string& name) {
-	for (auto* timer : m_Timers) {
-		if (timer->GetName() == name) {
-			return true;
-		}
-	}
-
-	return false;
+	return std::find(m_Timers.begin(), m_Timers.end(), name) != m_Timers.end();
 }
 
 void Entity::CancelCallbackTimers() {
-	for (auto* callback : m_CallbackTimers) {
-		delete callback;
-	}
-
 	m_CallbackTimers.clear();
+	m_PendingCallbackTimers.clear();
 }
 
 void Entity::ScheduleKillAfterUpdate(Entity* murderer) {
@@ -1728,8 +1734,8 @@ void Entity::ScheduleKillAfterUpdate(Entity* murderer) {
 
 void Entity::CancelTimer(const std::string& name) {
 	for (int i = 0; i < m_Timers.size(); i++) {
-		if (m_Timers[i]->GetName() == name) {
-			delete m_Timers[i];
+		auto& timer = m_Timers[i];
+		if (timer == name) {
 			m_Timers.erase(m_Timers.begin() + i);
 			return;
 		}
@@ -1737,21 +1743,10 @@ void Entity::CancelTimer(const std::string& name) {
 }
 
 void Entity::CancelAllTimers() {
-	/*for (auto timer : m_Timers) {
-		if (timer) delete timer;
-	}*/
-
-	for (auto* timer : m_Timers) {
-		delete timer;
-	}
-
 	m_Timers.clear();
-
-	for (auto* callBackTimer : m_CallbackTimers) {
-		delete callBackTimer;
-	}
-
+	m_PendingTimers.clear();
 	m_CallbackTimers.clear();
+	m_PendingCallbackTimers.clear();
 }
 
 bool Entity::IsPlayer() const {

dGame/Entity.h

@@ -160,6 +160,8 @@ public:
 	void AddChild(Entity* child);
 	void RemoveChild(Entity* child);
 	void RemoveParent();
+
+	// Adds a timer to start next frame with the given name and time.
 	void AddTimer(std::string name, float time);
 	void AddCallbackTimer(float time, std::function<void()> callback);
 	bool HasTimer(const std::string& name);
@@ -324,9 +326,10 @@ protected:
 	std::vector<std::function<void(Entity* target)>> m_PhantomCollisionCallbacks;
 
 	std::unordered_map<eReplicaComponentType, Component*> m_Components;
-	std::vector<EntityTimer*> m_Timers;
-	std::vector<EntityTimer*> m_PendingTimers;
-	std::vector<EntityCallbackTimer*> m_CallbackTimers;
+	std::vector<EntityTimer> m_Timers;
+	std::vector<EntityTimer> m_PendingTimers;
+	std::vector<EntityCallbackTimer> m_CallbackTimers;
+	std::vector<EntityCallbackTimer> m_PendingCallbackTimers;
 
 	bool m_ShouldDestroyAfterUpdate = false;
 

dGame/dEntity/EntityCallbackTimer.cpp

@@ -1,22 +1,10 @@
 #include "EntityCallbackTimer.h"
 
-EntityCallbackTimer::EntityCallbackTimer(float time, std::function<void()> callback) {
+EntityCallbackTimer::EntityCallbackTimer(const float time, const std::function<void()> callback) {
 	m_Time = time;
 	m_Callback = callback;
 }
 
-EntityCallbackTimer::~EntityCallbackTimer() {
-
-}
-
-std::function<void()> EntityCallbackTimer::GetCallback() {
-	return m_Callback;
-}
-
-float EntityCallbackTimer::GetTime() {
-	return m_Time;
-}
-
 void EntityCallbackTimer::Update(float deltaTime) {
 	m_Time -= deltaTime;
 }

dGame/dEntity/EntityCallbackTimer.h

@@ -5,11 +5,11 @@
 
 class EntityCallbackTimer {
 public:
-	EntityCallbackTimer(float time, std::function<void()> callback);
-	~EntityCallbackTimer();
+	EntityCallbackTimer(const float time, const std::function<void()> callback);
 	
-	std::function<void()> GetCallback();
-	float GetTime();
+	std::function<void()> GetCallback() const { return m_Callback; };
+
+	float GetTime() const { return m_Time; };
 
 	void Update(float deltaTime);
 

dGame/dEntity/EntityTimer.cpp

@@ -1,14 +1,10 @@
 #include "EntityTimer.h"
 
-EntityTimer::EntityTimer(std::string name, float time) {
+EntityTimer::EntityTimer(const std::string& name, const float time) {
 	m_Name = name;
 	m_Time = time;
 }
 
-EntityTimer::~EntityTimer() {
-
-}
-
 std::string EntityTimer::GetName() {
 	return m_Name;
 }

dGame/dEntity/EntityTimer.h

@@ -4,8 +4,15 @@
 
 class EntityTimer {
 public:
-	EntityTimer(std::string name, float time);
-	~EntityTimer();
+	EntityTimer(const std::string& name, const float time);
+
+	bool operator==(const EntityTimer& other) const {
+		return m_Name == other.m_Name;
+	}
+
+	bool operator==(const std::string& other) const {
+		return m_Name == other;
+	}
 
 	std::string GetName();
 	float GetTime();