From 2fb48f30ff900ec0214e46412721c2e6d1b4c9fe Mon Sep 17 00:00:00 2001 From: "alexey.lysiuk" Date: Fri, 10 May 2019 11:55:46 +0300 Subject: [PATCH] - 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 # Conflicts: # src/actor.h # src/maploader/maploader.cpp --- src/actor.h | 6 ++++-- src/d_net.cpp | 3 +-- src/g_level.cpp | 6 ++++-- src/p_acs.cpp | 13 ++++--------- src/p_lnspec.cpp | 13 +++---------- src/p_mobj.cpp | 22 ++++++++++++++++++++-- src/p_things.cpp | 6 ++---- src/p_xlat.cpp | 3 +-- src/scripting/vmthunks_actors.cpp | 8 +++----- 9 files changed, 42 insertions(+), 38 deletions(-) diff --git a/src/actor.h b/src/actor.h index 861045b28..efa286aa2 100644 --- a/src/actor.h +++ b/src/actor.h @@ -1240,13 +1240,15 @@ public: // ThingIDs static void ClearTIDHashes (); + void SetTID (int newTID); + +private: void AddToHash (); void RemoveFromHash (); - -private: static AActor *TIDHash[128]; static inline int TIDHASH (int key) { return key & 127; } + public: static FSharedStringArena mStringPropertyData; private: diff --git a/src/d_net.cpp b/src/d_net.cpp index a6e821e02..76982f6a2 100644 --- a/src/d_net.cpp +++ b/src/d_net.cpp @@ -2426,12 +2426,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); } } } diff --git a/src/g_level.cpp b/src/g_level.cpp index c031c6195..c8e64171a 100644 --- a/src/g_level.cpp +++ b/src/g_level.cpp @@ -1284,7 +1284,7 @@ void G_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(); @@ -1396,7 +1396,9 @@ int G_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(); diff --git a/src/p_acs.cpp b/src/p_acs.cpp index 6b45e2b97..ebcc9cd22 100644 --- a/src/p_acs.cpp +++ b/src/p_acs.cpp @@ -3885,8 +3885,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->AddToHash (); + actor->SetTID(tid); if (actor->flags & MF_SPECIAL) actor->flags |= MF_DROPPED; // Don't respawn actor->flags2 = oldFlags2; @@ -5987,8 +5986,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->AddToHash(); + puff->SetTID(pufftid); } } else @@ -6001,8 +5999,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->AddToHash(); + puff->SetTID(pufftid); } } } @@ -6422,9 +6419,7 @@ doplaysound: if (funcIndex == ACSF_PlayActorSound) if ((pickedActor->tid == 0) || (flags & PICKAF_FORCETID)) { - pickedActor->RemoveFromHash(); - pickedActor->tid = args[4]; - pickedActor->AddToHash(); + pickedActor->SetTID(args[4]); } if (flags & PICKAF_RETURNTID) { diff --git a/src/p_lnspec.cpp b/src/p_lnspec.cpp index 62f9baf34..6f7272b6e 100644 --- a/src/p_lnspec.cpp +++ b/src/p_lnspec.cpp @@ -1306,11 +1306,9 @@ FUNC(LS_Thing_ChangeTID) { if (arg0 == 0) { - if (it != NULL && !(it->ObjectFlags & OF_EuthanizeMe)) + if (it != nullptr) { - it->RemoveFromHash (); - it->tid = arg1; - it->AddToHash (); + it->SetTID(arg1); } } else @@ -1324,12 +1322,7 @@ FUNC(LS_Thing_ChangeTID) actor = next; next = iterator.Next (); - if (!(actor->ObjectFlags & OF_EuthanizeMe)) - { - actor->RemoveFromHash (); - actor->tid = arg1; - actor->AddToHash (); - } + actor->SetTID(arg1); } } return true; diff --git a/src/p_mobj.cpp b/src/p_mobj.cpp index 6698bff1e..23f92b2bb 100644 --- a/src/p_mobj.cpp +++ b/src/p_mobj.cpp @@ -2931,6 +2931,8 @@ void AActor::ClearTIDHashes () // void AActor::AddToHash () { + assert(!(ObjectFlags & OF_EuthanizeMe)); + if (tid == 0) { iprev = NULL; @@ -2971,6 +2973,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 @@ -5485,8 +5504,7 @@ AActor *P_SpawnMapThing (FMapThing *mthing, int position) } // [RH] Add ThingID to mobj and link it in with the others - mobj->tid = mthing->thingid; - mobj->AddToHash (); + mobj->SetTID(mthing->thingid); mobj->PrevAngles.Yaw = mobj->Angles.Yaw = (double)mthing->angle; diff --git a/src/p_things.cpp b/src/p_things.cpp index f126a1471..5371b73b2 100644 --- a/src/p_things.cpp +++ b/src/p_things.cpp @@ -105,8 +105,7 @@ bool P_Thing_Spawn (int tid, AActor *source, int type, DAngle angle, bool fog, i } if (mobj->flags & MF_SPECIAL) mobj->flags |= MF_DROPPED; // Don't respawn - mobj->tid = newtid; - mobj->AddToHash (); + mobj->SetTID(newtid); mobj->flags2 = oldFlags2; } else @@ -328,8 +327,7 @@ bool P_Thing_Projectile (int tid, AActor *source, int type, const char *type_nam if (mobj) { - mobj->tid = newtid; - mobj->AddToHash (); + mobj->SetTID(newtid); P_PlaySpawnSound(mobj, spot); if (gravity) { diff --git a/src/p_xlat.cpp b/src/p_xlat.cpp index 4f3b45611..0c0abace1 100644 --- a/src/p_xlat.cpp +++ b/src/p_xlat.cpp @@ -311,8 +311,7 @@ void P_TranslateTeleportThings () { if (!tagManager.SectorHasTags(dest->Sector)) { - dest->tid = 1; - dest->AddToHash (); + dest->SetTID(1); foundSomething = true; } } diff --git a/src/scripting/vmthunks_actors.cpp b/src/scripting/vmthunks_actors.cpp index de350e21c..d66481b3e 100644 --- a/src/scripting/vmthunks_actors.cpp +++ b/src/scripting/vmthunks_actors.cpp @@ -906,21 +906,19 @@ DEFINE_ACTION_FUNCTION_NATIVE(AActor, 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->tid = tid; - self->AddToHash(); + self->SetTID(tid); } DEFINE_ACTION_FUNCTION_NATIVE(AActor, ChangeTid, ChangeTid)