- made setting actor TID more explicit

Now it's no longer possible to manipulate TID hash from arbitrary location
For example, this prevents linking of destroyed object into the hash
TID member is still public but writing to it is limited to a few very specific cases like serialization and player traveling between levels

https://forum.zdoom.org/viewtopic.php?t=64476

src/actor.h

@@ -1180,12 +1180,14 @@ public:
 
 
 	// ThingIDs
+	void SetTID (int newTID);
+
+private:
 	void AddToHash ();
 	void RemoveFromHash ();
 
-
-private:
 	static inline int TIDHASH (int key) { return key & 127; }
+
 public:
 	static FSharedStringArena mStringPropertyData;
 private:

src/d_net.cpp

@@ -2400,12 +2400,11 @@ void Net_DoCommand (int type, uint8_t **stream, int player)
 							if (type >= DEM_SUMMON2 && type <= DEM_SUMMONFOE2)
 							{
 								spawned->Angles.Yaw = source->Angles.Yaw - angle;
-								spawned->tid = tid;
 								spawned->special = special;
 								for(i = 0; i < 5; i++) {
 									spawned->args[i] = args[i];
 								}
-								if(tid) spawned->AddToHash();
+								if(tid) spawned->SetTID(tid);
 							}
 						}
 					}

src/g_level.cpp

@@ -1369,7 +1369,7 @@ void FLevelLocals::StartTravel ()
 			{
 				pawn->UnlinkFromWorld (nullptr);
 				int tid = pawn->tid;	// Save TID
-				pawn->RemoveFromHash ();
+				pawn->SetTID(0);
 				pawn->tid = tid;		// Restore TID (but no longer linked into the hash chain)
 				pawn->ChangeStatNum (STAT_TRAVELLING);
 				pawn->DeleteAttachedLights();
@@ -1481,7 +1481,9 @@ int FLevelLocals::FinishTravel ()
 		}
 		pawn->LinkToWorld (nullptr);
 		pawn->ClearInterpolation();
-		pawn->AddToHash ();
+		const int tid = pawn->tid;	// Save TID (actor isn't linked into the hash chain yet)
+		pawn->tid = 0;				// Reset TID
+		pawn->SetTID(tid);			// Set TID (and link actor into the hash chain)
 		pawn->SetState(pawn->SpawnState);
 		pawn->player->SendPitchLimits();
 

src/maploader/maploader.cpp

@@ -112,8 +112,7 @@ void MapLoader::TranslateTeleportThings ()
 	{
 		if (!Level->SectorHasTags(dest->Sector))
 		{
-			dest->tid = 1;
+			dest->SetTID(1);
-			dest->AddToHash ();
 			foundSomething = true;
 		}
 	}

src/p_acs.cpp

@@ -3768,8 +3768,7 @@ int DLevelScript::DoSpawn (int type, const DVector3 &pos, int tid, DAngle angle,
 			if (force || P_TestMobjLocation (actor))
 			{
 				actor->Angles.Yaw = angle;
-				actor->tid = tid;
+				actor->SetTID(tid);
-				actor->AddToHash ();
 				if (actor->flags & MF_SPECIAL)
 					actor->flags |= MF_DROPPED;  // Don't respawn
 				actor->flags2 = oldFlags2;
@@ -5856,8 +5855,7 @@ int DLevelScript::CallFunction(int argCount, int funcIndex, int32_t *args)
 					AActor *puff = P_LineAttack(activator, angle, range, pitch, damage, damagetype, pufftype, fhflags);
 					if (puff != NULL && pufftid != 0)
 					{
-						puff->tid = pufftid;
+						puff->SetTID(pufftid);
-						puff->AddToHash();
 					}
 				}
 				else
@@ -5870,8 +5868,7 @@ int DLevelScript::CallFunction(int argCount, int funcIndex, int32_t *args)
 						AActor *puff = P_LineAttack(source, angle, range, pitch, damage, damagetype, pufftype, fhflags);
 						if (puff != NULL && pufftid != 0)
 						{
-							puff->tid = pufftid;
+							puff->SetTID(pufftid);
-							puff->AddToHash();
 						}
 					}
 				}
@@ -6291,9 +6288,7 @@ doplaysound:			if (funcIndex == ACSF_PlayActorSound)
 
 				if ((pickedActor->tid == 0) || (flags & PICKAF_FORCETID))
 				{
-					pickedActor->RemoveFromHash();
+					pickedActor->SetTID(args[4]);
-					pickedActor->tid = args[4];
-					pickedActor->AddToHash();
 				}
 				if (flags & PICKAF_RETURNTID)
 				{

src/p_lnspec.cpp

@@ -1299,11 +1299,9 @@ FUNC(LS_Thing_ChangeTID)
 {
 	if (arg0 == 0)
 	{
-		if (it != NULL && !(it->ObjectFlags & OF_EuthanizeMe))
+		if (it != nullptr)
 		{
-			it->RemoveFromHash ();
+			it->SetTID(arg1);
-			it->tid = arg1;
-			it->AddToHash ();
 		}
 	}
 	else
@@ -1317,12 +1315,7 @@ FUNC(LS_Thing_ChangeTID)
 			actor = next;
 			next = iterator.Next ();
 
-			if (!(actor->ObjectFlags & OF_EuthanizeMe))
+			actor->SetTID(arg1);
-			{
-				actor->RemoveFromHash ();
-				actor->tid = arg1;
-				actor->AddToHash ();
-			}
 		}
 	}
 	return true;

src/p_mobj.cpp

@@ -2906,6 +2906,8 @@ void P_NightmareRespawn (AActor *mobj)
 //
 void AActor::AddToHash ()
 {
+	assert(!(ObjectFlags & OF_EuthanizeMe));
+
 	if (tid == 0)
 	{
 		iprev = NULL;
@@ -2947,6 +2949,23 @@ void AActor::RemoveFromHash ()
 	tid = 0;
 }
 
+void AActor::SetTID (int newTID)
+{
+	RemoveFromHash();
+
+	if (ObjectFlags & OF_EuthanizeMe)
+	{
+		// Do not assign TID and do not link actor into the hash
+		// if it was already destroyed and will be freed by GC
+		return;
+	}
+
+	tid = newTID;
+
+	AddToHash();
+}
+
+
 //==========================================================================
 //
 // P_IsTIDUsed
@@ -5465,8 +5484,7 @@ AActor *FLevelLocals::SpawnMapThing (FMapThing *mthing, int position)
 	}
 
 	// [RH] Add ThingID to mobj and link it in with the others
-	mobj->tid = mthing->thingid;
+	mobj->SetTID(mthing->thingid);
-	mobj->AddToHash ();
 
 	mobj->PrevAngles.Yaw = mobj->Angles.Yaw = (double)mthing->angle;
 

src/p_things.cpp

@@ -94,8 +94,7 @@ bool FLevelLocals::EV_Thing_Spawn (int tid, AActor *source, int type, DAngle ang
 				}
 				if (mobj->flags & MF_SPECIAL)
 					mobj->flags |= MF_DROPPED;	// Don't respawn
-				mobj->tid = newtid;
+				mobj->SetTID(newtid);
-				mobj->AddToHash ();
 				mobj->flags2 = oldFlags2;
 			}
 			else
@@ -314,8 +313,7 @@ bool FLevelLocals::EV_Thing_Projectile (int tid, AActor *source, int type, const
 
 				if (mobj)
 				{
-					mobj->tid = newtid;
+					mobj->SetTID(newtid);
-					mobj->AddToHash ();
 					P_PlaySpawnSound(mobj, spot);
 					if (gravity)
 					{

src/scripting/vmthunks_actors.cpp

@@ -911,21 +911,19 @@ DEFINE_ACTION_FUNCTION_NATIVE(FLevelLocals, FindUniqueTid, P_FindUniqueTID)
 
 static void RemoveFromHash(AActor *self)
 {
-	self->RemoveFromHash();
+	self->SetTID(0);
 }
 
 DEFINE_ACTION_FUNCTION_NATIVE(AActor, RemoveFromHash, RemoveFromHash)
 {
 	PARAM_SELF_PROLOGUE(AActor);
-	self->RemoveFromHash();
+	RemoveFromHash(self);
 	return 0;
 }
 
 static void ChangeTid(AActor *self, int tid)
 {
-	self->RemoveFromHash();
+	self->SetTID(tid);
-	self->tid = tid;
-	self->AddToHash();
 }
 
 DEFINE_ACTION_FUNCTION_NATIVE(AActor, ChangeTid, ChangeTid)