From da89598eed25993e5f1dc7a41534001d22163eea Mon Sep 17 00:00:00 2001
From: Architector #4 <23612841+Architector4@users.noreply.github.com>
Date: Mon, 3 Nov 2025 08:39:29 +0300
Subject: [PATCH] MovableMan - fix usage of unordered_set::erase

`unordered_set::erase` invalidates the pointer that was erased.

https://en.cppreference.com/w/cpp/container/unordered_set/erase.html

> References and iterators to the erased elements are invalidated. Other iterators and references are not invalidated.

I also tested with a fresh C++ code file with `-fsanitize-address` and, indeed, the pattern of code of using an iterator variable to erase and then accessing through it reliably explodes.

---

What prompted this is that I have just had a completely random segfault crash originating from `SDL_Scancode ()` (????????) while playing the game. Here's the gdb backtrace (yes i play this game in debug build in gdb at this point lmao):

```
(gdb) bt
#0 0x0000555556b39558 in typeinfo name for SDL_Scancode ()
#1 0x0000555555ae47aa in RTE::MovableMan::Update (this=0x555557453f50) at ../Source/Managers/MovableMan.cpp:1618
#2 0x000055555567597c in RunGameLoop () at ../SourceMain.cpp:354
#3 0x000055555567658a in main (argc=1 argv=0x7fffffffdc88) at ../Source/Main.cpp:468
```

The last sensible location in that backtrace is right after the erase.

Nothing seems to be broken with this applied.

---

There are comments and code around these spots that this commit touches that are apparently to work around the same issue I experienced. They're probably redundant/unneeded any more lol
---
 Source/Managers/MovableMan.cpp | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/Source/Managers/MovableMan.cpp b/Source/Managers/MovableMan.cpp
index e68d56cf0..564a3646a 100644
--- a/Source/Managers/MovableMan.cpp
+++ b/Source/Managers/MovableMan.cpp
@@ -1484,8 +1484,6 @@ void MovableMan::Update() {
 				if (!(*aIt)->IsSetToDelete())
 					m_Actors.push_back(*aIt);
 				else {
-					m_ValidActors.erase(*aIt);
-
 					// Also remove actor from the roster
 					if ((*aIt)->GetTeam() >= 0) {
 						// m_ActorRoster[(*aIt)->GetTeam()].remove(*aIt);
@@ -1494,6 +1492,8 @@ void MovableMan::Update() {
 
 					(*aIt)->DestroyScriptState();
 					delete (*aIt);
+
+					m_ValidActors.erase(*aIt);
 				}
 			}
 			m_AddedActors.clear();
@@ -1504,9 +1504,9 @@ void MovableMan::Update() {
 				if (!(*iIt)->IsSetToDelete()) {
 					m_Items.push_back(*iIt);
 				} else {
-					m_ValidItems.erase(*iIt);
 					(*iIt)->DestroyScriptState();
 					delete (*iIt);
+					m_ValidItems.erase(*iIt);
 				}
 			}
 			m_AddedItems.clear();
@@ -1517,9 +1517,9 @@ void MovableMan::Update() {
 				if (!(*parIt)->IsSetToDelete()) {
 					m_Particles.push_back(*parIt);
 				} else {
-					m_ValidParticles.erase(*parIt);
 					(*parIt)->DestroyScriptState();
 					delete (*parIt);
+					m_ValidParticles.erase(*parIt);
 				}
 			}
 			m_AddedParticles.clear();
@@ -1571,8 +1571,8 @@ void MovableMan::Update() {
 					if ((*iIt)->GetRestThreshold() < 0) {
 						(*iIt)->SetRestThreshold(500);
 					}
-					m_ValidItems.erase(*iIt);
 					m_Particles.push_back(*iIt);
+					m_ValidItems.erase(*iIt);
 					iIt++;
 				}
 				m_Items.erase(imidIt, m_Items.end());
@@ -1600,9 +1600,9 @@ void MovableMan::Update() {
 					RemoveActorFromTeamRoster(*aIt);
 
 				// Delete
-				m_ValidActors.erase(*aIt);
 				(*aIt)->DestroyScriptState();
 				delete (*aIt);
+				m_ValidActors.erase(*aIt);
 				aIt++;
 			}
 			// Try to set the existing iterator to a safer value, erase can crash in debug mode otherwise?
@@ -1614,9 +1614,9 @@ void MovableMan::Update() {
 			imidIt = iIt;
 
 			while (iIt != m_Items.end()) {
-				m_ValidItems.erase(*iIt);
 				(*iIt)->DestroyScriptState();
 				delete (*iIt);
+				m_ValidItems.erase(*iIt);
 				iIt++;
 			}
 			m_Items.erase(imidIt, m_Items.end());
@@ -1626,9 +1626,9 @@ void MovableMan::Update() {
 			midIt = parIt;
 
 			while (parIt != m_Particles.end()) {
-				m_ValidParticles.erase(*parIt);
 				(*parIt)->DestroyScriptState();
 				delete (*parIt);
+				m_ValidParticles.erase(*parIt);
 				parIt++;
 			}
 			m_Particles.erase(midIt, m_Particles.end());
@@ -1658,9 +1658,9 @@ void MovableMan::Update() {
 				if ((*parIt)->GetDrawPriority() >= terrMat->GetPriority()) {
 					(*parIt)->DrawToTerrain(g_SceneMan.GetTerrain());
 				}
-				m_ValidParticles.erase(*parIt);
 				(*parIt)->DestroyScriptState();
 				delete (*parIt);
+				m_ValidParticles.erase(*parIt);
 				parIt++;
 			}
 			m_Particles.erase(midIt, m_Particles.end());