From 86375c1b7981efc78ca3a5ad77440b075190ef33 Mon Sep 17 00:00:00 2001 From: Boondorl Date: Thu, 17 Jul 2025 01:28:36 -0400 Subject: [PATCH] Move travelling to two-step system Mark Actors meant to travel early so their flag can be written into the snapshot, followed by moving them over all at once at the same time as previously. On finish, destroy any marked Actors to prevent duplication in hubs. Switch to a unique function that does the verification instead of ChangeStatNum and lock out STAT_TRAVELLING entirely since it shouldn't be used for regular gameplay. --- src/common/objects/dobject.cpp | 1 + src/g_level.cpp | 120 +++++++++++++------------- src/g_levellocals.h | 3 + src/playsim/dthinker.cpp | 136 +++++++++++++++++++----------- src/playsim/dthinker.h | 4 +- wadsrc/static/zscript/doombase.zs | 1 + 6 files changed, 154 insertions(+), 111 deletions(-) diff --git a/src/common/objects/dobject.cpp b/src/common/objects/dobject.cpp index c9a0babed..261a87ca2 100644 --- a/src/common/objects/dobject.cpp +++ b/src/common/objects/dobject.cpp @@ -647,6 +647,7 @@ void DObject::Serialize(FSerializer &arc) SerializeFlag("spawned", OF_Spawned); SerializeFlag("networked", OF_Networked); SerializeFlag("clientside", OF_ClientSide); + SerializeFlag("travelling", OF_Travelling); ObjectFlags |= OF_SerialSuccess; diff --git a/src/g_level.cpp b/src/g_level.cpp index aa74aeee2..e0c70e9ee 100644 --- a/src/g_level.cpp +++ b/src/g_level.cpp @@ -1292,8 +1292,9 @@ bool FLevelLocals::DoCompleted (FString nextlevel, wbstartstruct_t &wminfo) G_PlayerFinishLevel (i, mode, changeflags); } } + StartTravel(); soundEngine->BlockNewSounds(false); - + if (mode == FINISH_SameHub) { // Remember the level's state for re-entry. if (!(flags2 & LEVEL2_FORGETSTATE)) @@ -1613,7 +1614,7 @@ void G_DoWorldDone (void) Printf ("No next map specified.\n"); nextlevel = primaryLevel->MapName; } - primaryLevel->StartTravel (); + primaryLevel->MoveTravellers(); G_DoLoadLevel (nextlevel, startpos, true, false); gameaction = ga_nothing; viewactive = true; @@ -1640,74 +1641,72 @@ void FLevelLocals::UnlinkActorFromLevel(AActor& mo) mo.DeleteAttachedLights(); } +void FLevelLocals::AddToTravellingList(DThinker* th) +{ + if (th == nullptr || (th->ObjectFlags & (OF_EuthanizeMe | OF_Travelling))) + return; + + auto mo = dyn_cast(th); + // No voodoo dolls allowed. + if (mo != nullptr && mo->player != nullptr && mo->player->mo != mo) + return; + + th->ObjectFlags |= OF_Travelling; + TravellingThinkers.Push(th); + if (mo != nullptr) + { + if (mo->player != nullptr) + AddToTravellingList(mo->player->Bot); + + if (!(mo->flags & MF_UNMORPHED)) + AddToTravellingList(mo->alternative); + + if (!mo->IsKindOf(NAME_Inventory) || mo->PointerVar(NAME_Owner) == nullptr) + { + for (AActor* inv = mo->Inventory; inv != nullptr; inv = inv->Inventory) + AddToTravellingList(inv); + } + } +} + void FLevelLocals::StartTravel() { - bTravelling = true; - if (!deathmatch) { for (size_t i = 0u; i < MAXPLAYERS; ++i) { - if (PlayerInGame(i) && Players[i]->health > 0 && !(Players[i]->mo->ObjectFlags & OF_EuthanizeMe)) - Players[i]->mo->ChangeStatNum(STAT_TRAVELLING); + if (PlayerInGame(i) && Players[i]->health > 0) + AddToTravellingList(Players[i]->mo); } } - // Start building the list of everything that needs to travel. Since things can only be added - // to STAT_TRAVELLING at this point, it's safe to assume nothing will be removed unless destroyed - // (which will clean up anything it owns as well). - auto it = GetThinkerIterator(NAME_None, STAT_TRAVELLING); - DThinker* th = nullptr; - unsigned count = 0u; - do + // Start building the list of everything that needs to travel. This will keep growing + // as we add more things so don't use the C++ iterators. + for (size_t i = 0u; i < TravellingThinkers.Size(); ++i) { - count = 0u; - it.Reinit(); - - while ((th = it.Next()) != nullptr) + auto th = TravellingThinkers[i]; + if (!(th->ObjectFlags & OF_EuthanizeMe)) { - if (th->ObjectFlags & OF_Travelling) - continue; - - ++count; - auto mo = dyn_cast(th); - if (mo != nullptr) - { - if (mo->player != nullptr) - { - // No voodoo dolls allowed. - if (mo->player->mo != mo) - { - mo->ChangeStatNum(mo->GetStatNum()); - continue; - } - - if (mo->player->Bot != nullptr) - mo->player->Bot->ChangeStatNum(STAT_TRAVELLING); - } - - if (!mo->IsKindOf(NAME_Inventory) || mo->PointerVar(NAME_Owner) == nullptr) - { - for (AActor* inv = mo->Inventory; inv != nullptr; inv = inv->Inventory) - inv->ChangeStatNum(STAT_TRAVELLING); - } - - if (!(mo->flags & MF_UNMORPHED) && mo->alternative != nullptr) - mo->alternative->ChangeStatNum(STAT_TRAVELLING); - } - - th->ObjectFlags |= OF_Travelling; IFOVERRIDENVIRTUALPTRNAME(th, NAME_Thinker, PreTravelled) VMCallVoid(func, th); } - } while (count); + } +} - // Now that the list is complete, unlink everything. We want to do this after the - // callback so someone can't attempt to relink things. - auto unlinker = GetThinkerIterator(NAME_Actor, STAT_TRAVELLING); - AActor* mo = nullptr; - while ((mo = unlinker.Next()) != nullptr) +// We do this after snapshotting so the flag can be saved out but it won't be saved into +// the actual STAT_TRAVELLING list when snapshotting. +void FLevelLocals::MoveTravellers() +{ + for (auto th : TravellingThinkers) { + if (th->ObjectFlags & OF_EuthanizeMe) + continue; + + th->ChangeStatNum(STAT_TRAVELLING); + auto mo = dyn_cast(th); + if (mo == nullptr) + continue; + UnlinkActorFromLevel(*mo); mo->BlockingMobj = nullptr; mo->BlockingLine = mo->MovementBlockingLine = nullptr; @@ -1718,7 +1717,7 @@ void FLevelLocals::StartTravel() mo->floorsector = mo->ceilingsector = nullptr; } - bTravelling = false; + TravellingThinkers.Clear(); } //========================================================================== @@ -1744,8 +1743,6 @@ void FLevelLocals::LinkActorToLevel(AActor& mo) int FLevelLocals::FinishTravel() { - // From this point onward things can't be added back into the list, only removed. Callbacks - // are done after setting everything up so that the Thinkers have initialized objects to work with. TArray toCallBack = {}; int failNum = 0; auto it = GetThinkerIterator(NAME_None, STAT_TRAVELLING); @@ -1850,13 +1847,16 @@ int FLevelLocals::FinishTravel() mo->Speed = mo->GetDefault()->Speed; } - Thinkers.DestroyThinkersInList(STAT_TRAVELLING); - ClientsideThinkers.DestroyThinkersInList(STAT_TRAVELLING); + // Clean up anything that wasn't linked back to avoid memory leaks. We also need to wipe any + // remaining thinkers that were set to travel when they left if recovering from a snapshot, + // otherwise they'll pile up infinitely. + Thinkers.CleanUpTravellers(savegamerestore); + ClientsideThinkers.CleanUpTravellers(savegamerestore); // Some ZScript will be called here so we have to do this last. for (size_t i = 0u; i < MAXPLAYERS; ++i) { - if (PlayerInGame(i)) + if (PlayerInGame(i) && !(Players[i]->mo->ObjectFlags & OF_EuthanizeMe) && toCallBack.Find(Players[i]->mo) < toCallBack.Size()) Players[i]->mo->SetState(Players[i]->mo->SpawnState); } diff --git a/src/g_levellocals.h b/src/g_levellocals.h index 7f0bd7c57..a10d53219 100644 --- a/src/g_levellocals.h +++ b/src/g_levellocals.h @@ -270,6 +270,8 @@ public: FPlayerStart *PickPlayerStart(int playernum, int flags = 0); bool DoCompleted(FString nextlevel, wbstartstruct_t &wminfo); void StartTravel(); + void AddToTravellingList(DThinker* th); + void MoveTravellers(); int FinishTravel(); void UnlinkActorFromLevel(AActor& mo); void LinkActorToLevel(AActor& mo); @@ -730,6 +732,7 @@ public: TArray ParticlesInSubsec; FThinkerCollection Thinkers; FThinkerCollection ClientsideThinkers; + TArray TravellingThinkers; TArray Scrolls; // NULL if no DScrollers in this level diff --git a/src/playsim/dthinker.cpp b/src/playsim/dthinker.cpp index bf8bdf33a..b0c4935cb 100644 --- a/src/playsim/dthinker.cpp +++ b/src/playsim/dthinker.cpp @@ -70,9 +70,6 @@ struct ProfileInfo static TMap Profiles; static unsigned int profilethinkers, profilelimit; DThinker *NextToThink; -// Denote that non-travelling Thinkers are about to start doing so. While active, only -// allow things to be added to the list, not removed. -bool bTravelling = false; //========================================================================== // @@ -365,6 +362,22 @@ void FThinkerCollection::DestroyAllThinkers(bool fullgc) // //========================================================================== +void FThinkerCollection::CleanUpTravellers(bool saveGame) +{ + DestroyThinkersInList(STAT_TRAVELLING); + for (size_t i = 0u; i <= MAX_STATNUM; ++i) + { + FreshThinkers[i].RemoveTravellers(saveGame); + Thinkers[i].RemoveTravellers(saveGame); + } +} + +//========================================================================== +// +// +// +//========================================================================== + void FThinkerCollection::OnLoad() { for (auto& list : FreshThinkers) @@ -662,6 +675,32 @@ void FThinkerList::SaveList(FSerializer &arc) // //========================================================================== +void FThinkerList::RemoveTravellers(bool saveGame) +{ + DThinker* node = GetHead(); + if (node == nullptr) + return; + + while (node != Sentinel) + { + NextToThink = node->NextThinker; + if ((node->ObjectFlags & OF_Travelling) && !(node->ObjectFlags & OF_EuthanizeMe)) + { + if (saveGame) + node->ObjectFlags &= ~OF_Travelling; + else + node->Destroy(); + } + node = NextToThink; + } +} + +//========================================================================== +// +// +// +//========================================================================== + void FThinkerList::OnLoad() { DThinker* node = GetHead(); @@ -930,16 +969,6 @@ void DThinker::ChangeStatNum(int statnum) static void ChangeStatNum(DThinker *self, int statnum) { - // Wait until after these Thinkers are processed to allow modifying them, - // otherwise relinking and callbacks will be completely messed up. This can - // only happen from map objects spawning in intentionally trying to mess with - // the list. - if (self->ObjectFlags & OF_Travelling) - { - Printf(TEXTCOLOR_RED "Travelling Thinkers cannot have their statnum changed\n"); - return; - } - // This will always break Actors, they should use STAT_TRAVELLING instead to // transition between levels. if (statnum == STAT_STATIC && self->IsKindOf(NAME_Actor)) @@ -947,44 +976,9 @@ static void ChangeStatNum(DThinker *self, int statnum) Printf(TEXTCOLOR_RED "Actors cannot be added to STAT_STATIC\n"); return; } - - if (bTravelling) - { - // Don't let things be moved out of the list, only into it. - if (statnum != STAT_TRAVELLING) - { - Printf(TEXTCOLOR_RED "Thinkers can only be moved into STAT_TRAVELLING while changing levels\n"); - return; - } - // These should be handled by the owning Actor, otherwise they'll lose them and become useless anyway. - if (self->IsKindOf(NAME_Inventory) && self->PointerVar(NAME_Owner) != nullptr) - { - Printf(TEXTCOLOR_RED "Owned Inventory items must travel with their owner on level change\n"); - return; - } - if (self->IsKindOf(NAME_Bot)) - { - Printf(TEXTCOLOR_RED "Bot Thinkers must travel with their owner on level change\n"); - return; - } - auto mo = dyn_cast(self); - if (mo != nullptr && (mo->flags & MF_UNMORPHED)) - { - Printf(TEXTCOLOR_RED "Unmorphed Actors must travel with their owner on level change\n"); - return; - } - // These need to be locked down since they have native fields that won't be cleared - // properly at the moment. - auto cls = self->GetClass()->NativeClass(); - if (cls->TypeName != NAME_Thinker && cls->TypeName != NAME_Actor) - { - Printf(TEXTCOLOR_RED "Native thinkers cannot travel\n"); - return; - } - } else if (statnum == STAT_TRAVELLING) { - Printf(TEXTCOLOR_RED "Thinkers cannot be added to STAT_TRAVELLING while the game isn't changing levels\n"); + Printf(TEXTCOLOR_RED "Thinkers cannot be added to STAT_TRAVELLING manually\n"); return; } @@ -1005,6 +999,50 @@ DEFINE_ACTION_FUNCTION_NATIVE(DThinker, ChangeStatNum, ChangeStatNum) // //========================================================================== +static void AddToTravellingList(DThinker* self) +{ + // These should be handled by the owning Actor, otherwise they'll lose them and become useless anyway. + if (self->IsKindOf(NAME_Inventory) && self->PointerVar(NAME_Owner) != nullptr) + { + Printf(TEXTCOLOR_RED "Owned Inventory items must travel with their owner on level change\n"); + return; + } + if (self->IsKindOf(NAME_Bot)) + { + Printf(TEXTCOLOR_RED "Bot Thinkers must travel with their owner on level change\n"); + return; + } + auto mo = dyn_cast(self); + if (mo != nullptr && (mo->flags & MF_UNMORPHED)) + { + Printf(TEXTCOLOR_RED "Unmorphed Actors must travel with their owner on level change\n"); + return; + } + // These need to be locked down since they have native fields that won't be cleared + // properly at the moment. + auto cls = self->GetClass()->NativeClass(); + if (cls->TypeName != NAME_Thinker && cls->TypeName != NAME_Actor) + { + Printf(TEXTCOLOR_RED "Native thinkers cannot travel\n"); + return; + } + + self->Level->AddToTravellingList(self); +} + +DEFINE_ACTION_FUNCTION_NATIVE(DThinker, AddToTravellingList, AddToTravellingList) +{ + PARAM_SELF_PROLOGUE(DThinker); + AddToTravellingList(self); + return 0; +} + +//========================================================================== +// +// +// +//========================================================================== + CCMD(profilethinkers) { const int argc = argv.argc(); diff --git a/src/playsim/dthinker.h b/src/playsim/dthinker.h index c585e4118..42278e2c4 100644 --- a/src/playsim/dthinker.h +++ b/src/playsim/dthinker.h @@ -50,8 +50,6 @@ class FThinkerIterator; enum { MAX_STATNUM = 127 }; -extern bool bTravelling; - // Doubly linked ring list of thinkers struct FThinkerList { @@ -62,6 +60,7 @@ struct FThinkerList bool IsEmpty() const; void DestroyThinkers(); bool DoDestroyThinkers(); + void RemoveTravellers(bool saveGame); void OnLoad(); int TickThinkers(FThinkerList *dest); // Returns: # of thinkers ticked int ProfileThinkers(FThinkerList *dest); @@ -84,6 +83,7 @@ struct FThinkerCollection void RunThinkers(FLevelLocals *Level); // The level is needed to tick the lights void RunClientsideThinkers(FLevelLocals* Level); void DestroyAllThinkers(bool fullgc = true); + void CleanUpTravellers(bool saveGame); void SerializeThinkers(FSerializer &arc, bool keepPlayers); void MarkRoots(); void OnLoad(); diff --git a/wadsrc/static/zscript/doombase.zs b/wadsrc/static/zscript/doombase.zs index 0d1fa09fd..a863e5ad1 100644 --- a/wadsrc/static/zscript/doombase.zs +++ b/wadsrc/static/zscript/doombase.zs @@ -241,6 +241,7 @@ class Thinker : Object native play virtual native void Tick(); virtual native void PostBeginPlay(); virtual void OnLoad() {} + native void AddToTravellingList(); native void ChangeStatNum(int stat); native clearscope int GetStatNum() const;