From 850529b51bebba3fc1f658fab8f85693524995ee 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 --- src/actor.h | 6 ++++-- src/d_net.cpp | 3 +-- src/g_level.cpp | 6 ++++-- src/maploader/maploader.cpp | 3 +-- src/p_acs.cpp | 13 ++++--------- src/p_lnspec.cpp | 13 +++---------- src/p_mobj.cpp | 22 ++++++++++++++++++++-- src/p_things.cpp | 6 ++---- src/scripting/vmthunks_actors.cpp | 8 +++----- 9 files changed, 42 insertions(+), 38 deletions(-) diff --git a/src/actor.h b/src/actor.h index 2bc8802847..4c0a062978 100644 --- a/src/actor.h +++ b/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: diff --git a/src/d_net.cpp b/src/d_net.cpp index 28047ca9a3..d04fda6664 100644 --- a/src/d_net.cpp +++ b/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); } } } diff --git a/src/g_level.cpp b/src/g_level.cpp index 380e21ab9a..0475571f70 100644 --- a/src/g_level.cpp +++ b/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(); diff --git a/src/maploader/maploader.cpp b/src/maploader/maploader.cpp index fb5866d79c..a3d543c95c 100644 --- a/src/maploader/maploader.cpp +++ b/src/maploader/maploader.cpp @@ -112,8 +112,7 @@ void MapLoader::TranslateTeleportThings () { if (!Level->SectorHasTags(dest->Sector)) { - dest->tid = 1; - dest->AddToHash (); + dest->SetTID(1); foundSomething = true; } } diff --git a/src/p_acs.cpp b/src/p_acs.cpp index 03986f7da4..5d883019bd 100644 --- a/src/p_acs.cpp +++ b/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->AddToHash (); + actor->SetTID(tid); 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->AddToHash(); + puff->SetTID(pufftid); } } 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->AddToHash(); + puff->SetTID(pufftid); } } } @@ -6291,9 +6288,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 8e0a56ec01..4bdfcbebd7 100644 --- a/src/p_lnspec.cpp +++ b/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->tid = arg1; - it->AddToHash (); + it->SetTID(arg1); } } else @@ -1317,12 +1315,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 fbcb308066..0b306f9c20 100644 --- a/src/p_mobj.cpp +++ b/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->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 da8efa4807..0874f07747 100644 --- a/src/p_things.cpp +++ b/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->AddToHash (); + mobj->SetTID(newtid); 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->AddToHash (); + mobj->SetTID(newtid); P_PlaySpawnSound(mobj, spot); if (gravity) { diff --git a/src/scripting/vmthunks_actors.cpp b/src/scripting/vmthunks_actors.cpp index 0eac413eef..d37600444f 100644 --- a/src/scripting/vmthunks_actors.cpp +++ b/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->tid = tid; - self->AddToHash(); + self->SetTID(tid); } DEFINE_ACTION_FUNCTION_NATIVE(AActor, ChangeTid, ChangeTid)