Fixed a rare crash

Author: Causeless

CHANGELOG.md

@@ -38,7 +38,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 - Fixed an issue that could cause post-effects to appear very blurry.
 
-- Fixed a rare crash that could occur depending on Lua garbage collection and async processing.
+- Fixed a couple of rare crashes that could occur depending on Lua garbage collection and async processing.
 
 - Fixed a crash that could occur when using the Nucleo Swarm weapon.
 

Source/Main.cpp

@@ -323,7 +323,6 @@ void RunGameLoop() {
 			g_LuaMan.Update();
 
 			g_UInputMan.Update();
-			g_ConsoleMan.Update();
 
 			// It is vital that server is updated after input manager but before activity because input manager will clear received pressed and released events on next update.
 			if (g_NetworkServer.IsServerModeEnabled()) {
@@ -333,6 +332,9 @@ void RunGameLoop() {
 
 			g_FrameMan.Update();
 
+			g_MovableMan.CompleteQueuedMOIDDrawings();
+
+			g_ConsoleMan.Update();
 			g_ActivityMan.Update();
 
 			if (g_SceneMan.GetScene()) {

Source/Managers/MovableMan.cpp

@@ -123,7 +123,20 @@ void MovableMan::Destroy() {
 
 MovableObject* MovableMan::GetMOFromID(MOID whichID) {
 	if (whichID != g_NoMOID && whichID != 0 && whichID < m_MOIDIndex.size()) {
-		return m_MOIDIndex[whichID];
+		// This is really, really awful
+		// But, Lua scripts can take ownership of an MO which exists in this list
+		// And then the MO can be deallocated by Lua GC
+		// Meaning that this ptr points to stale memory.
+		// Due to our pooled memory system, we can avoid a crash by just... checking that the MO isn't NoMOID
+		// But this is still atrociously awful and terrible and this can point to a newly-allocated object that just so happens to be allocated the same place.
+		// Anyways, until we can fix the god-awful abomination that is this game's memory ownership semantics, we're stuck with this
+		// Which is also technically undefined behaviour
+		MovableObject* candidate = m_MOIDIndex[whichID];
+		if (candidate->GetID() != whichID) {
+			return nullptr;
+		}
+
+		return candidate;
 	}
 	return nullptr;
 }
@@ -1654,10 +1667,6 @@ void MovableMan::Update() {
 void MovableMan::Travel() {
 	ZoneScoped;
 
-	if (m_DrawMOIDsTask.valid()) {
-		m_DrawMOIDsTask.wait();
-	}
-
 	// Travel Actors
 	{
 		ZoneScopedN("Actors Travel");
@@ -1857,6 +1866,12 @@ void MovableMan::UpdateDrawMOIDs() {
 	}
 }
 
+void MovableMan::CompleteQueuedMOIDDrawings() {
+	if (m_DrawMOIDsTask.valid()) {
+		m_DrawMOIDsTask.wait();
+	}
+}
+
 void MovableMan::Draw(BITMAP* pTargetBitmap, const Vector& targetPos) {
 	ZoneScoped;
 

Source/Managers/MovableMan.h

@@ -456,6 +456,9 @@ namespace RTE {
 		/// Updates the MOIDs of all current MOs.
 		void UpdateDrawMOIDs();
 
+		// Forces MOID drawing to complete (should be done before any physics sim or collision detection etc)
+		void CompleteQueuedMOIDDrawings();
+
 		/// Draws this MovableMan's current graphical representation to a BITMAP of choice.
 		/// @param pTargetBitmap A pointer to a BITMAP to draw on.
 		/// @param targetPos The absolute position of the target bitmap's upper left corner in the scene. (default: Vector())