Revert "UniqueID and Age are now persisted through game save/load"

Causeless · Causeless · commit 01f1df5373e6 · 2024-08-19T21:04:12.000+01:00
This reverts commit b0b026f.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -44,8 +44,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 - The Signal Hunt activity no longer has a preview image, as it was not formatted correctly and spoiled the interior structure of the cave.
 
-- `MovableObject`'s `UniqueID` and `Age` fields are now persisted through game save/load.
-
 </details>
 
 <details><summary><b>Fixed</b></summary>
diff --git a/Source/Entities/MovableObject.cpp b/Source/Entities/MovableObject.cpp
@@ -23,6 +23,7 @@ using namespace RTE;
 
 AbstractClassInfo(MovableObject, SceneObject);
 
+std::atomic<long> MovableObject::m_UniqueIDCounter = 1;
 std::string MovableObject::ms_EmptyString = "";
 
 MovableObject::MovableObject() {
@@ -140,17 +141,14 @@ int MovableObject::Create() {
 	if (SceneObject::Create() < 0)
 		return -1;
 
+	m_AgeTimer.Reset();
 	m_RestTimer.Reset();
 
 	// If the stop time hasn't been assigned, just make the same as the life time.
 	if (m_EffectStopTime <= 0)
 		m_EffectStopTime = m_Lifetime;
 
-	if (m_UniqueID == 0)
-	{
-		m_UniqueID = g_MovableMan.GetNextUniqueID();
-		m_AgeTimer.Reset();
-	}
+	m_UniqueID = MovableObject::GetNextUniqueID();
 
 	m_MOIDHit = g_NoMOID;
 	m_TerrainMatHit = g_MaterialAir;
@@ -179,7 +177,7 @@ int MovableObject::Create(const float mass,
 	m_HitsMOs = hitMOs;
 	m_GetsHitByMOs = getHitByMOs;
 
-	m_UniqueID = g_MovableMan.GetNextUniqueID();
+	m_UniqueID = MovableObject::GetNextUniqueID();
 
 	m_MOIDHit = g_NoMOID;
 	m_TerrainMatHit = g_MaterialAir;
@@ -205,6 +203,9 @@ int MovableObject::Create(const MovableObject& reference) {
 	m_RestThreshold = reference.m_RestThreshold;
 	//    m_Force = reference.m_Force;
 	//    m_ImpulseForce = reference.m_ImpulseForce;
+	// Should reset age instead??
+	//    m_AgeTimer = reference.m_AgeTimer;
+	m_AgeTimer.Reset();
 	m_RestTimer.Reset();
 	m_Lifetime = reference.m_Lifetime;
 	m_Sharpness = reference.m_Sharpness;
@@ -265,17 +266,7 @@ int MovableObject::Create(const MovableObject& reference) {
 	m_NumberValueMap = reference.m_NumberValueMap;
 	m_ObjectValueMap = reference.m_ObjectValueMap;
 
-	// If we have a -1 unique ID, then we're currently performing a game save or load
-	// In this case, we actually want to persist our existing IDs
-	if (g_MovableMan.ShouldPersistUniqueIDs()) {
-		m_UniqueID = reference.m_UniqueID;
-		m_AgeTimer = reference.m_AgeTimer;
-	} else {
-		// Otherwise we're copying from a preset normally and ought to create a new object
-		m_UniqueID = g_MovableMan.GetNextUniqueID();
-		m_AgeTimer.Reset();
-	}
-
+	m_UniqueID = MovableObject::GetNextUniqueID();
 	g_MovableMan.RegisterObject(this);
 
 	return 0;
@@ -383,11 +374,6 @@ int MovableObject::ReadProperty(const std::string_view& propName, Reader& reader
 	MatchProperty("SimUpdatesBetweenScriptedUpdates", { reader >> m_SimUpdatesBetweenScriptedUpdates; });
 	MatchProperty("AddCustomValue", { ReadCustomValueProperty(reader); });
 	MatchProperty("ForceIntoMasterLuaState", { reader >> m_ForceIntoMasterLuaState; });
-	MatchProperty("SpecialBehaviour_SetUniqueID", {
-		long oldID = m_UniqueID;
-		reader >> m_UniqueID;
-		g_MovableMan.ReregisterObjectIfApplicable(this, oldID);
-	});
 
 	EndPropertyList;
 }
diff --git a/Source/Entities/MovableObject.h b/Source/Entities/MovableObject.h
@@ -979,6 +979,10 @@ namespace RTE {
 		/// @param newRestThreshold New rest threshold value
 		void SetRestThreshold(int newRestThreshold) { m_RestThreshold = newRestThreshold; }
 
+		/// Returns the next unique id for MO's and increments unique ID counter
+		/// @return Returns the next unique id.
+		static long GetNextUniqueID() { return ++m_UniqueIDCounter; }
+
 		/// Returns this MO's unique persistent ID
 		/// @return Returns this MO's unique persistent ID
 		long GetUniqueID() const { return m_UniqueID; }
@@ -1135,7 +1139,8 @@ namespace RTE {
 		/// @param A MovableObject object which is passed in by reference.
 		// Member variables
 		static Entity::ClassInfo m_sClass;
-
+		// Global counter with unique ID's
+		static std::atomic<long> m_UniqueIDCounter;
 		// The type of MO this is, either Actor, Item, or Particle
 		int m_MOType;
 		float m_Mass; // In metric kilograms (kg).
diff --git a/Source/Entities/Scene.cpp b/Source/Entities/Scene.cpp
@@ -1172,7 +1172,6 @@ void Scene::SaveSceneObject(Writer& writer, const SceneObject* sceneObjectToSave
 		writer.NewPropertyWithValue("LifeTime", movableObjectToSave->GetLifetime());
 		writer.NewPropertyWithValue("Age", movableObjectToSave->GetAge());
 		writer.NewPropertyWithValue("PinStrength", movableObjectToSave->GetPinStrength());
-		writer.NewPropertyWithValue("SpecialBehaviour_SetUniqueID", movableObjectToSave->GetUniqueID());
 	}
 
 	if (const MOSprite* moSpriteToSave = dynamic_cast<const MOSprite*>(sceneObjectToSave)) {
diff --git a/Source/GUI/GUIReader.cpp b/Source/GUI/GUIReader.cpp
@@ -359,12 +359,6 @@ GUIReader& GUIReader::operator>>(unsigned long& var) {
 	return *this;
 }
 
-GUIReader& GUIReader::operator>>(long long& var) {
-	DiscardEmptySpace();
-	*m_Stream >> var;
-	return *this;
-}
-
 // Yeah, this is dumb - read as double and cast.
 // This is because, for whatever fucking reason, iostream can save out floats at a precision that it's then unable to read...
 GUIReader& GUIReader::operator>>(float& var) {
diff --git a/Source/GUI/GUIReader.h b/Source/GUI/GUIReader.h
@@ -92,7 +92,6 @@ namespace RTE {
 		GUIReader& operator>>(unsigned int& var);
 		GUIReader& operator>>(long& var);
 		GUIReader& operator>>(unsigned long& var);
-		GUIReader& operator>>(long long& var);
 		GUIReader& operator>>(float& var);
 		GUIReader& operator>>(double& var);
 		GUIReader& operator>>(std::string& var);
diff --git a/Source/Main.cpp b/Source/Main.cpp
@@ -245,7 +245,6 @@ void PollSDLEvents() {
 void RunMenuLoop() {
 	g_UInputMan.DisableKeys(false);
 	g_UInputMan.TrapMousePos(false);
-	g_TimerMan.PauseSim(true);
 
 	while (!System::IsSetToQuit()) {
 		g_WindowMan.ClearRenderer();
diff --git a/Source/Managers/ActivityMan.cpp b/Source/Managers/ActivityMan.cpp
@@ -93,9 +93,6 @@ bool ActivityMan::SaveCurrentGame(const std::string& fileName) {
 		return false;
 	}
 
-	long currentMaxID = g_MovableMan.GetMaxUniqueID();
-	g_MovableMan.SetShouldPersistUniqueIDs(true);
-
 	// We need a copy of our scene, because we have to do some fixup to remove PLACEONLOAD items and only keep the current MovableMan state.
 	std::unique_ptr<Scene> modifiableScene(dynamic_cast<Scene*>(scene->Clone()));
 
@@ -124,8 +121,6 @@ bool ActivityMan::SaveCurrentGame(const std::string& fileName) {
 		}
 	}
 
-	writer->NewPropertyWithValue("MaxUniqueID", currentMaxID);
-	writer->NewPropertyWithValue("CurrentSimTicks", g_TimerMan.GetSimTickCount());
 	writer->NewPropertyWithValue("OriginalScenePresetName", scene->GetPresetName());
 	writer->NewPropertyWithValue("PlaceObjectsIfSceneIsRestarted", g_SceneMan.GetPlaceObjectsOnLoad());
 	writer->NewPropertyWithValue("PlaceUnitsIfSceneIsRestarted", g_SceneMan.GetPlaceUnitsOnLoad());
@@ -142,8 +137,6 @@ bool ActivityMan::SaveCurrentGame(const std::string& fileName) {
 	// We didn't transfer ownership, so we must be very careful that sceneAltered's deletion doesn't touch the stuff we got from MovableMan.
 	modifiableScene->ClearPlacedObjectSet(Scene::PlacedObjectSets::PLACEONLOAD, false);
 
-	g_MovableMan.SetShouldPersistUniqueIDs(false);
-
 	g_ConsoleMan.PrintString("SYSTEM: Game saved to \"" + fileName + "\"!");
 	return true;
 }
@@ -163,20 +156,13 @@ bool ActivityMan::LoadAndLaunchGame(const std::string& fileName) {
 	std::unique_ptr<Scene> scene(std::make_unique<Scene>());
 	std::unique_ptr<GAScripted> activity(std::make_unique<GAScripted>());
 
-	long maxUniqueID = 0;
-	long long simTimeTicks = 0;
 	std::string originalScenePresetName = fileName;
 	bool placeObjectsIfSceneIsRestarted = true;
 	bool placeUnitsIfSceneIsRestarted = true;
 	while (reader.NextProperty()) {
 		std::string propName = reader.ReadPropName();
 		if (propName == "Activity") {
 			reader >> activity.get();
-		} else if (propName == "MaxUniqueID") {
-			reader >> maxUniqueID;
-			g_MovableMan.SetShouldPersistUniqueIDs(true);
-		} else if (propName == "CurrentSimTicks") {
-			reader >> simTimeTicks;
 		} else if (propName == "OriginalScenePresetName") {
 			reader >> originalScenePresetName;
 		} else if (propName == "PlaceObjectsIfSceneIsRestarted") {
@@ -195,15 +181,6 @@ bool ActivityMan::LoadAndLaunchGame(const std::string& fileName) {
 	scene->SetPresetName(originalScenePresetName);
 	// For starting Activity, we need to directly clone the Activity we want to start.
 	StartActivity(dynamic_cast<GAScripted*>(activity->Clone()));
-
-	// Set the max unique ID to our loaded maximum so we don't stomp over any existing ones
-	if (maxUniqueID != 0) {
-		g_MovableMan.SetMaxUniqueID(maxUniqueID);
-	}
-
-	g_MovableMan.SetShouldPersistUniqueIDs(false);
-	g_TimerMan.SetSimTickCount(simTimeTicks);
-
 	// When this method exits, our Scene object will be destroyed, which will cause problems if you try to restart it. To avoid this, set the Scene to load to the preset object with the same name.
 	g_SceneMan.SetSceneToLoad(originalScenePresetName, placeObjectsIfSceneIsRestarted, placeUnitsIfSceneIsRestarted);
 
diff --git a/Source/Managers/MovableMan.cpp b/Source/Managers/MovableMan.cpp
@@ -43,7 +43,6 @@ struct MOXPosComparison {
 
 MovableMan::MovableMan() {
 	Clear();
-	m_UniqueIDCounter = 1;
 }
 
 MovableMan::~MovableMan() {
@@ -76,7 +75,6 @@ void MovableMan::Clear() {
 	m_MaxDroppedItems = 100;
 	m_SettlingEnabled = true;
 	m_MOSubtractionEnabled = true;
-	m_ShouldPersistUniqueIDs = false;
 }
 
 int MovableMan::Initialize() {
@@ -183,17 +181,6 @@ void MovableMan::UnregisterObject(MovableObject* mo) {
 	m_KnownObjects.erase(mo->GetUniqueID());
 }
 
-void MovableMan::ReregisterObjectIfApplicable(MovableObject* mo, long oldUniqueId) {
-	if (mo == nullptr || m_KnownObjects.find(oldUniqueId) == m_KnownObjects.end()) {
-		// Old ID was never registered, don't need to do anything
-		return;
-	}
-
-	std::lock_guard<std::mutex> guard(m_ObjectRegisteredMutex);
-	m_KnownObjects[oldUniqueId] = nullptr;
-	m_KnownObjects[mo->GetUniqueID()] = mo;
-}
-
 const std::vector<MovableObject*>* MovableMan::GetMOsInBox(const Box& box, int ignoreTeam, bool getsHitByMOsOnly) const {
 	std::vector<MovableObject*>* vectorForLua = new std::vector<MovableObject*>();
 	*vectorForLua = std::move(g_SceneMan.GetMOIDGrid().GetMOsInBox(box, ignoreTeam, getsHitByMOsOnly));
diff --git a/Source/Managers/MovableMan.h b/Source/Managers/MovableMan.h
@@ -482,40 +482,14 @@ namespace RTE {
 		/// @param mo MO to remove.
 		void UnregisterObject(MovableObject* mo);
 
-		/// Reregisters an object in a global Map collection to handle if it's unique ID changes
-		/// @param mo MO to reregister if necessary.
-		/// @param oldUniqueID The MO's old UniqueID.
-		void ReregisterObjectIfApplicable(MovableObject* mo, long oldUniqueId);
-
-		/// Returns the next unique id for MO's and increments unique ID counter
-		/// @return Returns the next unique id.
-		long GetNextUniqueID() { if (m_ShouldPersistUniqueIDs) { return 0; } return ++m_UniqueIDCounter; }
-
-		/// Returns the max unique id for MO's
-		/// @return Returns the current max unique id.
-		long GetMaxUniqueID() { return m_UniqueIDCounter; }
-
-		/// Set the max unique id for MO's. This is used so we can consistently save/load unique IDs so they're consistent within a game session
-		/// @param id Unique Id to set.
-		void SetMaxUniqueID(long newMaxID) { m_UniqueIDCounter = newMaxID; }
-
-		/// Returns whether we should be persisting uniqueIDs for save/load
-		/// @return Returns whether we are persisting uniqueIDs.
-		bool ShouldPersistUniqueIDs() { return m_ShouldPersistUniqueIDs; }
-
-		/// Sets whether we should be persisting uniqueIDs for save/load
-		/// @param newValue Whether we should be persisting uniqueIDs.
-		void SetShouldPersistUniqueIDs(bool newValue) { m_ShouldPersistUniqueIDs = newValue; }
-
 		/// Uses a global lookup map to find an object by it's unique id.
 		/// @param id Unique Id to look for.
 		/// @return Object found or 0 if not found any.
-		MovableObject* FindObjectByUniqueID(long id) {
-			if (m_KnownObjects.count(id) > 0) {
+		MovableObject* FindObjectByUniqueID(long int id) {
+			if (m_KnownObjects.count(id) > 0)
 				return m_KnownObjects[id];
-			} else {
+			else
 				return 0;
-			}
 		}
 
 		/// Returns the size of the object registry collection
@@ -577,10 +551,6 @@ namespace RTE {
 		std::deque<Actor*> m_Actors;
 		// A map to give a unique contiguous identifier per-actor. This is re-created per frame.
 		std::unordered_map<const Actor*, int> m_ContiguousActorIDs;
-		// Global counter with unique ID's
-		std::atomic<long> m_UniqueIDCounter;
-		// Whether or not UniqueIDs should be persisted (for save/load)
-		bool m_ShouldPersistUniqueIDs;
 		// List of items that are pickup-able by actors
 		std::deque<MovableObject*> m_Items;
 		// List of free, dead particles flying around
@@ -651,8 +621,8 @@ namespace RTE {
 
 		unsigned int m_SimUpdateFrameNumber;
 
-		// Global map which stores all objects so they could be found by their unique ID
-		std::map<long, MovableObject*> m_KnownObjects;
+		// Global map which stores all objects so they could be foud by their unique ID
+		std::map<long int, MovableObject*> m_KnownObjects;
 
 		/// Private member variable and method declarations
 	private:
diff --git a/Source/Managers/TimerMan.h b/Source/Managers/TimerMan.h
@@ -85,10 +85,6 @@ namespace RTE {
 		/// @return The number of ticks passed since the simulation started.
 		long long GetSimTickCount() const { return m_SimTimeTicks; }
 
-		/// Sets a current global simulation time measured in ticks from the start of the simulation up to the last Update of this TimerMan. Used to give a consistent value for save/load
-		/// @param newValue The number of ticks passed since the simulation started.
-		void SetSimTickCount(long long newValue) { m_SimTimeTicks = newValue; }
-
 		/// Gets a current global simulation time, measured in sim updates, from the start of the simulation up to the last Update of this TimerMan.
 		/// @return The number of simulation updates that have occurred since the simulation started.
 		long long GetSimUpdateCount() const { return m_SimUpdateCount; }
diff --git a/Source/System/Reader.h b/Source/System/Reader.h
@@ -185,11 +185,6 @@ namespace RTE {
 			*m_Stream >> var;
 			return *this;
 		}
-		Reader& operator>>(long long& var) {
-			DiscardEmptySpace();
-			*m_Stream >> var;
-			return *this;
-		}
 		// Yeah, this is dumb - read as double and cast.
 		// This is because, for whatever fucking reason, iostream can save out floats at a precision that it's then unable to read...
 		Reader& operator>>(float& var) {

Original file line number	Diff line number	Diff line change
`@@ -1172,7 +1172,6 @@ void Scene::SaveSceneObject(Writer& writer, const SceneObject* sceneObjectToSave`
`1172`	`1172`	`writer.NewPropertyWithValue("LifeTime", movableObjectToSave->GetLifetime());`
`1173`	`1173`	`writer.NewPropertyWithValue("Age", movableObjectToSave->GetAge());`
`1174`	`1174`	`writer.NewPropertyWithValue("PinStrength", movableObjectToSave->GetPinStrength());`
`1175`		`- writer.NewPropertyWithValue("SpecialBehaviour_SetUniqueID", movableObjectToSave->GetUniqueID());`
`1176`	`1175`	`}`
`1177`	`1176`
`1178`	`1177`	`if (const MOSprite* moSpriteToSave = dynamic_cast<const MOSprite*>(sceneObjectToSave)) {`