Updated substitution logic to only replace pointers if safe

src/common/objects/dobject.cpp

@@ -419,7 +419,7 @@ size_t DObject::PropagateMark()
 //==========================================================================
 
 template<typename M>
-static void MapPointerSubstitution(M *map, size_t &changed, DObject *old, DObject *notOld)
+static void MapPointerSubstitution(M *map, size_t &changed, DObject *old, DObject *notOld, const bool shouldSwap)
 {
 	TMapIterator<typename M::KeyType, DObject*> it(*map);
 	typename M::Pair * p;
@@ -427,13 +427,21 @@ static void MapPointerSubstitution(M *map, size_t &changed, DObject *old, DObjec
 	{
 		if (p->Value == old)
 		{
-			p->Value = notOld;
-			changed++;
+			if (shouldSwap)
+			{
+				p->Value = notOld;
+				changed++;
+			}
+			else if (p->Value != nullptr)
+			{
+				p->Value = nullptr;
+				changed++;
+			}
 		}
 	}
 }
 
-size_t DObject::PointerSubstitution (DObject *old, DObject *notOld)
+size_t DObject::PointerSubstitution (DObject *old, DObject *notOld, bool nullOnFail)
 {
 	const PClass *info = GetClass();
 	size_t changed = 0;
@@ -446,11 +454,22 @@ size_t DObject::PointerSubstitution (DObject *old, DObject *notOld)
 	for(size_t i = 0; i < info->FlatPointersSize; i++)
 	{
 		size_t offset = info->FlatPointers[i].first;
+		auto& obj = *(DObject**)((uint8_t*)this + offset);
 
-		if (*(DObject **)((uint8_t *)this + offset) == old)
+		if (obj == old)
 		{
-			*(DObject **)((uint8_t *)this + offset) = notOld;
-			changed++;
+			// If a pointer's type is null, that means it's native and anything native is safe to swap
+			// around due to its inherit type expansiveness.
+			if (info->FlatPointers[i].second == nullptr || notOld->IsKindOf(info->FlatPointers[i].second->PointedClass()))
+			{
+				obj = notOld;
+				changed++;
+			}
+			else if (nullOnFail && obj != nullptr)
+			{
+				obj = nullptr;
+				changed++;
+			}
 		}
 	}
 
@@ -462,13 +481,26 @@ size_t DObject::PointerSubstitution (DObject *old, DObject *notOld)
 
 	for(size_t i = 0; i < info->ArrayPointersSize; i++)
 	{
-		auto aray = (TArray<DObject*>*)((uint8_t *)this + info->ArrayPointers[i].first);
+		const bool isType = notOld->IsKindOf(static_cast<PObjectPointer*>(info->ArrayPointers[i].second->ElementType)->PointedClass());
+
+		if (!isType && !nullOnFail)
+			continue;
+
+		auto aray = (TArray<DObject*>*)((uint8_t*)this + info->ArrayPointers[i].first);
 		for (auto &p : *aray)
 		{
 			if (p == old)
 			{
-				p = notOld;
-				changed++;
+				if (isType)
+				{
+					p = notOld;
+					changed++;
+				}
+				else if (p != nullptr)
+				{
+					p = nullptr;
+					changed++;
+				}
 			}
 		}
 	}
@@ -482,13 +514,18 @@ size_t DObject::PointerSubstitution (DObject *old, DObject *notOld)
 	for(size_t i = 0; i < info->MapPointersSize; i++)
 	{
 		PMap * type = static_cast<PMap*>(info->MapPointers[i].second);
+
+		const bool isType = notOld->IsKindOf(static_cast<PObjectPointer*>(type->ValueType)->PointedClass());
+		if (!isType && !nullOnFail)
+			continue;
+
 		if(type->KeyType->RegType == REGT_STRING)
 		{ // FString,DObject*
-			MapPointerSubstitution((ZSMap<FString,DObject*>*)((uint8_t *)this + info->MapPointers[i].first), changed, old, notOld);
+			MapPointerSubstitution((ZSMap<FString,DObject*>*)((uint8_t *)this + info->MapPointers[i].first), changed, old, notOld, isType);
 		}
 		else
 		{ // uint32_t,DObject*
-			MapPointerSubstitution((ZSMap<uint32_t,DObject*>*)((uint8_t *)this + info->MapPointers[i].first), changed, old, notOld);
+			MapPointerSubstitution((ZSMap<uint32_t,DObject*>*)((uint8_t *)this + info->MapPointers[i].first), changed, old, notOld, isType);
 		}
 	}
 

src/common/objects/dobject.h

@@ -246,7 +246,7 @@ public:
 	inline int* IntArray(FName field);
 
 	// This is only needed for swapping out PlayerPawns and absolutely nothing else!
-	virtual size_t PointerSubstitution (DObject *old, DObject *notOld);
+	virtual size_t PointerSubstitution (DObject *old, DObject *notOld, bool nullOnFail);
 
 	PClass *GetClass() const
 	{

src/g_level.cpp

@@ -1711,7 +1711,7 @@ int FLevelLocals::FinishTravel ()
 		pawn->flags2 &= ~MF2_BLASTED;
 		if (oldpawn != nullptr)
 		{
-			PlayerPointerSubstitution (oldpawn, pawn);
+			PlayerPointerSubstitution (oldpawn, pawn, true);
 			oldpawn->Destroy();
 		}
 		if (pawndup != NULL)

src/playsim/actor.h

@@ -1748,7 +1748,7 @@ struct FTranslatedLineTarget
 	bool unlinked;	// found by a trace that went through an unlinked portal.
 };
 
-void PlayerPointerSubstitution(AActor* oldPlayer, AActor* newPlayer);
+void PlayerPointerSubstitution(AActor* oldPlayer, AActor* newPlayer, bool removeOld);
 int MorphPointerSubstitution(AActor* from, AActor* to);
 
 #define S_FREETARGMOBJ	1

src/playsim/fragglescript/t_script.cpp

@@ -549,9 +549,9 @@ size_t DFraggleThinker::PropagateMark()
 //
 //==========================================================================
 
-size_t DFraggleThinker::PointerSubstitution (DObject *old, DObject *notOld)
+size_t DFraggleThinker::PointerSubstitution (DObject *old, DObject *notOld, bool nullOnFail)
 {
-	size_t changed = Super::PointerSubstitution(old, notOld);
+	size_t changed = Super::PointerSubstitution(old, notOld, nullOnFail);
 	for(unsigned i=0;i<SpawnedThings.Size();i++)
 	{
 		if (SpawnedThings[i] == static_cast<AActor*>(old)) 

src/playsim/fragglescript/t_script.h

@@ -701,7 +701,7 @@ public:
 	void Tick();
 	void InitFunctions();
 	size_t PropagateMark();
-	size_t PointerSubstitution (DObject *old, DObject *notOld);
+	size_t PointerSubstitution (DObject *old, DObject *notOld, bool nullOnFail);
 	bool wait_finished(DRunningScript *script);
 	void AddRunningScript(DRunningScript *runscr);
 

src/playsim/p_mobj.cpp

@@ -5248,15 +5248,14 @@ extern bool demonew;
 
 //==========================================================================
 //
-// This function is dangerous and only designed for swapping player pawns
+// This function is only designed for swapping player pawns
 // over to their new ones upon changing levels or respawning. It SHOULD NOT be
 // used for anything else! Do not export this functionality as it's
-// meant strictly for internal usage. Only swap pointers if the thing being swapped
-// to is a type of the thing being swapped from.
+// meant strictly for internal usage.
 //
 //==========================================================================
 
-void PlayerPointerSubstitution(AActor* oldPlayer, AActor* newPlayer)
+void PlayerPointerSubstitution(AActor* oldPlayer, AActor* newPlayer, bool removeOld)
 {
 	if (oldPlayer == nullptr || newPlayer == nullptr || oldPlayer == newPlayer
 		|| !oldPlayer->IsKindOf(NAME_PlayerPawn) || !newPlayer->IsKindOf(NAME_PlayerPawn))
@@ -5293,20 +5292,16 @@ void PlayerPointerSubstitution(AActor* oldPlayer, AActor* newPlayer)
 			sec.SoundTarget = newPlayer;
 	}
 
-	// Update all the remaining object pointers. This is dangerous but needed to ensure
-	// everything functions correctly when respawning or changing levels.
+	// Update all the remaining object pointers.
 	for (DObject* probe = GC::Root; probe != nullptr; probe = probe->ObjNext)
-		probe->PointerSubstitution(oldPlayer, newPlayer);
+		probe->PointerSubstitution(oldPlayer, newPlayer, removeOld);
 }
 
 //==========================================================================
 //
-// This function is much safer than PlayerPointerSubstition as it only truly
-// swaps a few safe pointers. This has some extra barriers to it to allow
+// This has some extra barriers compared to PlayerPointerSubstitution to allow
 // Actors to freely morph into other Actors which is its main usage.
-// Previously this used raw pointer substitutions but that's far too
-// volatile to use with modder-provided information. It also allows morphing
-// to be more extendable from ZScript.
+// It also allows morphing to be more extendable from ZScript.
 //
 //==========================================================================
 
@@ -5351,30 +5346,6 @@ int MorphPointerSubstitution(AActor* from, AActor* to)
 		VMCall(func, params, 2, nullptr, 0);
 	}
 
-	// Only change some gameplay-related pointers that we know we can safely swap to whatever
-	// new Actor class is present.
-	AActor* mo = nullptr;
-	auto it = from->Level->GetThinkerIterator<AActor>();
-	while ((mo = it.Next()) != nullptr)
-	{
-		if (mo->target == from)
-			mo->target = to;
-		if (mo->tracer == from)
-			mo->tracer = to;
-		if (mo->master == from)
-			mo->master = to;
-		if (mo->goal == from)
-			mo->goal = to;
-		if (mo->lastenemy == from)
-			mo->lastenemy = to;
-		if (mo->LastHeard == from)
-			mo->LastHeard = to;
-		if (mo->LastLookActor == from)
-			mo->LastLookActor = to;
-		if (mo->Poisoner == from)
-			mo->Poisoner = to;
-	}
-
 	// Go through player infos.
 	for (int i = 0; i < MAXPLAYERS; ++i)
 	{
@@ -5404,6 +5375,10 @@ int MorphPointerSubstitution(AActor* from, AActor* to)
 			sec.SoundTarget = to;
 	}
 
+	// Replace any object pointers that are safe to swap around.
+	for (DObject* probe = GC::Root; probe != nullptr; probe = probe->ObjNext)
+		probe->PointerSubstitution(from, to, false);
+
 	// Remaining maintenance related to morphing.
 	if (from->player != nullptr)
 	{
@@ -5696,7 +5671,7 @@ AActor *FLevelLocals::SpawnPlayer (FPlayerStart *mthing, int playernum, int flag
 				if (sec.SoundTarget == oldactor) sec.SoundTarget = nullptr;
 			}
 
-			PlayerPointerSubstitution (oldactor, p->mo);
+			PlayerPointerSubstitution (oldactor, p->mo, false);
 
 			localEventManager->PlayerRespawned(PlayerNum(p));
 			Behaviors.StartTypedScripts (SCRIPT_Respawn, p->mo, true);