- completely eliminated the horrible fudging that went on with players during hub travel. This means that DestroyMostThinkers and the player substitution mechanism in the object deserializer are gone now.

I wish I had realized this the last time it came up - it would have saved me a lot of trouble.
But as it turns out, the more recent travelling code makes all of this completely unnecessary, working perfectly fine with deleting the player pawns along with the rest of the thinkers before loading the stored ones from the savegame (and getting rid of those in G_FinishTravel.)
And with a sane savegame format that does not depend on side effects from how the thinker serializing handled linking into the lists the old code was even harmful, leaving voodoo dolls behind.
I had the exact same effect when I tried to reshuffle some things for reliably restoring portals, but did not make the connection to interference between two mutually incompatible player travelling mechanisms that just worked by sheer happenstance with the original order of things.

src/dthinker.cpp

@@ -332,24 +332,6 @@ void DThinker::DestroyAllThinkers ()
 	GC::FullGC();
 }
 
-// Destroy all thinkers except for player-controlled actors
-// Players are simply removed from the list of thinkers and
-// will be added back after serialization is complete.
-void DThinker::DestroyMostThinkers ()
-{
-	int i;
-
-	for (i = 0; i <= MAX_STATNUM; i++)
-	{
-		if (i != STAT_TRAVELLING)
-		{
-			DestroyMostThinkersInList (Thinkers[i], i);
-			DestroyMostThinkersInList (FreshThinkers[i], i);
-		}
-	}
-	GC::FullGC();
-}
-
 void DThinker::DestroyThinkersInList (FThinkerList &list)
 {
 	if (list.Sentinel != NULL)
@@ -364,39 +346,6 @@ void DThinker::DestroyThinkersInList (FThinkerList &list)
 	}
 }
 
-void DThinker::DestroyMostThinkersInList (FThinkerList &list, int stat)
-{
-	if (stat != STAT_PLAYER)
-	{
-		DestroyThinkersInList (list);
-	}
-	else if (list.Sentinel != NULL)
-	{ // If it's a voodoo doll, destroy it. Otherwise, simply remove
-	  // it from the list. G_FinishTravel() will find it later from
-	  // a players[].mo link and destroy it then, after copying various
-	  // information to a new player.
-		for (DThinker *probe = list.Sentinel->NextThinker; probe != list.Sentinel; probe = list.Sentinel->NextThinker)
-		{
-			if (!probe->IsKindOf(RUNTIME_CLASS(APlayerPawn)) ||		// <- should not happen
-				static_cast<AActor *>(probe)->player == NULL ||
-				static_cast<AActor *>(probe)->player->mo != probe)
-			{
-				probe->Destroy();
-			}
-			else
-			{
-				probe->Remove();
-				// Technically, this doesn't need to be in any list now, since
-				// it's only going to be found later and destroyed before ever
-				// needing to tick again, but by moving it to a separate list,
-				// I can keep my debug assertions that all thinkers are either
-				// euthanizing or in a list.
-				Thinkers[MAX_STATNUM+1].AddTail(probe);
-			}
-		}
-	}
-}
-
 void DThinker::RunThinkers ()
 {
 	int i, count;

src/dthinker.h

@@ -78,7 +78,6 @@ public:
 	static void RunThinkers ();
 	static void RunThinkers (int statnum);
 	static void DestroyAllThinkers ();
-	static void DestroyMostThinkers ();
 	static void DestroyThinkersInList(int statnum)
 	{
 		DestroyThinkersInList(Thinkers[statnum]);
@@ -94,7 +93,6 @@ private:
 	enum no_link_type { NO_LINK };
 	DThinker(no_link_type) throw();
 	static void DestroyThinkersInList (FThinkerList &list);
-	static void DestroyMostThinkersInList (FThinkerList &list, int stat);
 	static int TickThinkers (FThinkerList *list, FThinkerList *dest);	// Returns: # of thinkers ticked
 	static void SaveList(FSerializer &arc, DThinker *node);
 	void Remove();

src/p_saveg.cpp

@@ -63,17 +63,6 @@
 #include "r_renderer.h"
 #include "serializer.h"
 
-//
-// Thinkers
-//
-
-void P_DestroyThinkers(bool hubLoad)
-{
-	if (hubLoad)
-		DThinker::DestroyMostThinkers();
-	else
-		DThinker::DestroyAllThinkers();
-}
 
 //==========================================================================
 //
@@ -901,7 +890,7 @@ void G_SerializeLevel(FSerializer &arc, bool hubload)
 	Renderer->StartSerialize(arc);
 	if (arc.isReading())
 	{
-		P_DestroyThinkers(hubload);
+		DThinker::DestroyAllThinkers();
 		interpolator.ClearInterpolations();
 		arc.ReadObjects(hubload);
 	}

src/serializer.cpp

@@ -824,14 +824,6 @@ void FSerializer::WriteObjects()
 			obj->SerializeUserVars(*this);
 			obj->Serialize(*this);
 			obj->CheckIfSerialized();
-			if (obj->IsKindOf(RUNTIME_CLASS(AActor)) &&
-				(player = static_cast<AActor *>(obj)->player) &&
-				player->mo == obj)
-			{
-				w->Key("playerindex");
-				w->Int(int(player - players));
-			}
-
 			EndObject();
 		}
 		EndArray();
@@ -895,15 +887,6 @@ void FSerializer::ReadObjects(bool hubtravel)
 						if (obj != nullptr)
 						{
 							int pindex = -1;
-							if (hubtravel)
-							{
-								// mark this as a hub travelling player. This needs to be substituted later, but that's better done when all objects have been loaded.
-								Serialize(*this, "playerindex", pindex, nullptr);
-								if (hubtravel && pindex >= 0 && pindex < MAXPLAYERS)
-								{
-									r->mPlayers[pindex] = int(i);
-								}
-							}
 							try
 							{
 								obj->SerializeUserVars(*this);
@@ -922,38 +905,6 @@ void FSerializer::ReadObjects(bool hubtravel)
 			}
 			EndArray();
 
-			// Special treatment for hub travel: We do not want the player pawn which exited the level when this snapshot was made
-			// but the one that got freshly created when the map was loaded in P_SetupLevel.
-			// The loop above marked all objects that represent a player, so now we go through them, and if a corresponding
-			// player had been spawned on map load it gets substituted here.
-
-			// The substitution does not take place inside the above loop to ensure that no changes to the loaded objects
-			// can occur afterward. At this point everything has been loaded and the substitution is a simple matter of
-			// calling DObject::StaticPointerSubstitution.
-
-			// If no corresponding player can be found among the freshly spawned ones, the one from the snapshot is kept.
-			if (hubtravel)
-			{
-				for (int i = 0; i < MAXPLAYERS; i++)
-				{
-					int pindex = r->mPlayers[i];
-					if (pindex != -1)
-					{
-						if (players[i].mo != nullptr)
-						{
-							// Destroy the old pawn before substituting the pointer so that its inventory also gets properly destroyed.
-							r->mDObjects[pindex]->Destroy();
-							DObject::StaticPointerSubstitution(r->mDObjects[pindex], players[i].mo);
-							r->mDObjects[pindex] = players[i].mo;
-						}
-						else
-						{
-							players[i].mo = static_cast<APlayerPawn*>(r->mDObjects[pindex]);
-						}
-					}
-				}
-			}
-
 			DThinker::bSerialOverride = false;
 			assert(!founderrors);
 			if (founderrors)