From 02de10f65732639b0b28e1a98d7927a0708f1e0b Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Mon, 19 Nov 2018 17:05:00 +0100 Subject: [PATCH 1/4] - cleaned up the PointerSubstitution code Since the only thing it gets used for is swapping out PlayerPawns it can safely skip all global variables that never point to a live player, which allowed to remove quite a bit of code here that stood in the way of scriptifying more content --- src/b_bot.h | 2 +- src/d_player.h | 1 - src/dobject.cpp | 54 +++++++++-------------------- src/dobject.h | 8 ++--- src/p_user.cpp | 32 ----------------- src/r_data/r_interpolate.cpp | 20 ----------- src/scripting/backend/vmbuilder.cpp | 2 +- 7 files changed, 23 insertions(+), 96 deletions(-) diff --git a/src/b_bot.h b/src/b_bot.h index 27433b533..a3a62b08f 100644 --- a/src/b_bot.h +++ b/src/b_bot.h @@ -116,7 +116,7 @@ public: botinfo_t *botinfo; int spawn_tries; int wanted_botnum; - TObjPtr firstthing; + TObjPtr firstthing; TObjPtr body1; TObjPtr body2; diff --git a/src/d_player.h b/src/d_player.h index 2a8daa4f9..ecb3a71ea 100644 --- a/src/d_player.h +++ b/src/d_player.h @@ -372,7 +372,6 @@ public: player_t &operator= (const player_t &p); void Serialize(FSerializer &arc); - size_t FixPointers (const DObject *obj, DObject *replacement); size_t PropagateMark(); void SetLogNumber (int num); diff --git a/src/dobject.cpp b/src/dobject.cpp index 64236bcba..c0139c82d 100644 --- a/src/dobject.cpp +++ b/src/dobject.cpp @@ -307,7 +307,6 @@ DObject::~DObject () if (!(ObjectFlags & OF_Released)) { // Find all pointers that reference this object and NULL them. - StaticPointerSubstitution(this, NULL); Release(); } } @@ -478,11 +477,15 @@ size_t DObject::PointerSubstitution (DObject *old, DObject *notOld) //========================================================================== // -// +// This once was the main method for pointer cleanup, but +// nowadays its only use is swapping out PlayerPawns. +// This requires pointer fixing throughout all objects and a few +// global variables, but it only needs to look at pointers that +// can point to a player. // //========================================================================== -size_t DObject::StaticPointerSubstitution (DObject *old, DObject *notOld, bool scandefaults) +size_t DObject::StaticPointerSubstitution (AActor *old, AActor *notOld) { DObject *probe; size_t changed = 0; @@ -497,24 +500,12 @@ size_t DObject::StaticPointerSubstitution (DObject *old, DObject *notOld, bool s last = probe; } - if (scandefaults) - { - for (auto p : PClassActor::AllActorClasses) - { - auto def = GetDefaultByType(p); - if (def != nullptr) - { - def->DObject::PointerSubstitution(old, notOld); - } - } - } - // Go through the bodyque. for (i = 0; i < BODYQUESIZE; ++i) { if (bodyque[i] == old) { - bodyque[i] = static_cast(notOld); + bodyque[i] = notOld; changed++; } } @@ -523,36 +514,25 @@ size_t DObject::StaticPointerSubstitution (DObject *old, DObject *notOld, bool s for (i = 0; i < MAXPLAYERS; i++) { if (playeringame[i]) - changed += players[i].FixPointers (old, notOld); - } - - for (auto &s : level.sectorPortals) - { - if (s.mSkybox == old) { - s.mSkybox = static_cast(notOld); - changed++; + APlayerPawn *replacement = static_cast(notOld); + auto &p = players[i]; + + if (p.mo == old) p.mo = replacement, changed++; + if (p.poisoner.pp == old) p.poisoner = replacement, changed++; + if (p.attacker.pp == old) p.attacker = replacement, changed++; + if (p.camera.pp == old) p.camera = replacement, changed++; + if (p.ConversationNPC.pp == old) p.ConversationNPC = replacement, changed++; + if (p.ConversationPC == old) p.ConversationPC = replacement, changed++; } } // Go through sectors. for (auto &sec : level.sectors) { -#define SECTOR_CHECK(f,t) \ -if (sec.f.pp == static_cast(old)) { sec.f = static_cast(notOld); changed++; } - SECTOR_CHECK( SoundTarget, AActor ); - SECTOR_CHECK( SecActTarget, AActor ); - SECTOR_CHECK( floordata, DSectorEffect ); - SECTOR_CHECK( ceilingdata, DSectorEffect ); - SECTOR_CHECK( lightingdata, DSectorEffect ); -#undef SECTOR_CHECK + if (sec.SoundTarget == old) sec.SoundTarget = notOld; } - // Go through bot stuff. - if (bglobal.firstthing.pp == (AActor *)old) bglobal.firstthing = (AActor *)notOld, ++changed; - if (bglobal.body1.pp == (AActor *)old) bglobal.body1 = (AActor *)notOld, ++changed; - if (bglobal.body2.pp == (AActor *)old) bglobal.body2 = (AActor *)notOld, ++changed; - return changed; } diff --git a/src/dobject.h b/src/dobject.h index 7aeedc57c..fb561e2c6 100644 --- a/src/dobject.h +++ b/src/dobject.h @@ -185,6 +185,8 @@ protected: \ #include "dobjgc.h" +class AActor; + class DObject { public: @@ -249,11 +251,9 @@ public: inline FString &StringVar(FName field); template T*& PointerVar(FName field); - // If you need to replace one object with another and want to - // change any pointers from the old object to the new object, - // use this method. + // This is only needed for swapping out PlayerPawns and absolutely nothing else! virtual size_t PointerSubstitution (DObject *old, DObject *notOld); - static size_t StaticPointerSubstitution (DObject *old, DObject *notOld, bool scandefaults = false); + static size_t StaticPointerSubstitution (AActor *old, AActor *notOld); PClass *GetClass() const { diff --git a/src/p_user.cpp b/src/p_user.cpp index a601f1db2..48ad0ee67 100644 --- a/src/p_user.cpp +++ b/src/p_user.cpp @@ -396,38 +396,6 @@ player_t &player_t::operator=(const player_t &p) return *this; } -// This function supplements the pointer cleanup in dobject.cpp, because -// player_t is not derived from DObject. (I tried it, and DestroyScan was -// unable to properly determine the player object's type--possibly -// because it gets staticly allocated in an array.) -// -// This function checks all the DObject pointers in a player_t and NULLs any -// that match the pointer passed in. If you add any pointers that point to -// DObject (or a subclass), add them here too. - -size_t player_t::FixPointers (const DObject *old, DObject *rep) -{ - APlayerPawn *replacement = static_cast(rep); - size_t changed = 0; - - // The construct *& is used in several of these to avoid the read barriers - // that would turn the pointer we want to check to NULL if the old object - // is pending deletion. - if (mo == old) mo = replacement, changed++; - if (*&poisoner == old) poisoner = replacement, changed++; - if (*&attacker == old) attacker = replacement, changed++; - if (*&camera == old) camera = replacement, changed++; - if (*&Bot == old) Bot = static_cast(rep), changed++; - if (ReadyWeapon == old) ReadyWeapon = static_cast(rep), changed++; - if (PendingWeapon == old) PendingWeapon = static_cast(rep), changed++; - if (*&PremorphWeapon == old) PremorphWeapon = static_cast(rep), changed++; - if (psprites == old) psprites = static_cast(rep), changed++; - if (*&ConversationNPC == old) ConversationNPC = replacement, changed++; - if (*&ConversationPC == old) ConversationPC = replacement, changed++; - if (*&MUSINFOactor == old) MUSINFOactor = replacement, changed++; - return changed; -} - size_t player_t::PropagateMark() { GC::Mark(mo); diff --git a/src/r_data/r_interpolate.cpp b/src/r_data/r_interpolate.cpp index 8f73fdcac..e23c012c6 100644 --- a/src/r_data/r_interpolate.cpp +++ b/src/r_data/r_interpolate.cpp @@ -67,7 +67,6 @@ public: void Interpolate(double smoothratio); virtual void Serialize(FSerializer &arc); - size_t PointerSubstitution (DObject *old, DObject *notOld); size_t PropagateMark(); }; @@ -535,25 +534,6 @@ void DSectorPlaneInterpolation::Serialize(FSerializer &arc) ("attached", attached); } -//========================================================================== -// -// -// -//========================================================================== - -size_t DSectorPlaneInterpolation::PointerSubstitution (DObject *old, DObject *notOld) -{ - int subst = 0; - for(unsigned i=0; i *ReturnRegs) { - int paramcount = 0; + unsigned paramcount = 0; for (auto &func : emitters) { paramcount += func(build); From ad0400113511afaaf8574bfad5b30ddf38070b52 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Mon, 19 Nov 2018 18:13:23 +0100 Subject: [PATCH 2/4] - fixed some issues with the bodyque and moved this variable into FLevelLocals * it was never saved in savegames, leaving the state of dead bodies undefined * it shouldn't be subjected to pointer substitution because all it contains is old dead bodies, not live ones. --- src/dobject.cpp | 11 ----------- src/dobjgc.cpp | 11 ++--------- src/dobjgc.h | 7 +++++-- src/doomstat.h | 2 -- src/g_game.cpp | 14 ++++++-------- src/g_game.h | 5 +---- src/g_level.cpp | 18 ++++++++++++++++++ src/g_levellocals.h | 6 ++++++ src/p_mobj.cpp | 5 +---- src/p_saveg.cpp | 4 +++- src/p_setup.cpp | 6 +++--- src/r_data/r_translate.cpp | 3 ++- 12 files changed, 47 insertions(+), 45 deletions(-) diff --git a/src/dobject.cpp b/src/dobject.cpp index c0139c82d..fc4896054 100644 --- a/src/dobject.cpp +++ b/src/dobject.cpp @@ -38,7 +38,6 @@ #include "actor.h" #include "doomstat.h" // Ideally, DObjects can be used independant of Doom. #include "d_player.h" // See p_user.cpp to find out why this doesn't work. -#include "g_game.h" // Needed for bodyque. #include "c_dispatch.h" #include "dsectoreffect.h" #include "serializer.h" @@ -500,16 +499,6 @@ size_t DObject::StaticPointerSubstitution (AActor *old, AActor *notOld) last = probe; } - // Go through the bodyque. - for (i = 0; i < BODYQUESIZE; ++i) - { - if (bodyque[i] == old) - { - bodyque[i] = notOld; - changed++; - } - } - // Go through players. for (i = 0; i < MAXPLAYERS; i++) { diff --git a/src/dobjgc.cpp b/src/dobjgc.cpp index 38eed8fb4..cd1b49a10 100644 --- a/src/dobjgc.cpp +++ b/src/dobjgc.cpp @@ -325,15 +325,8 @@ static void MarkRoot() FCanvasTextureInfo::Mark(); Mark(E_FirstEventHandler); Mark(E_LastEventHandler); - for (auto &s : level.sectorPortals) - { - Mark(s.mSkybox); - } - // Mark dead bodies. - for (i = 0; i < BODYQUESIZE; ++i) - { - Mark(bodyque[i]); - } + level.Mark(); + // Mark players. for (i = 0; i < MAXPLAYERS; i++) { diff --git a/src/dobjgc.h b/src/dobjgc.h index 41ae9ba23..82db1de20 100644 --- a/src/dobjgc.h +++ b/src/dobjgc.h @@ -224,7 +224,10 @@ template inline T barrier_cast(TObjPtr &o) return static_cast(static_cast(o)); } -template inline void GC::Mark(TObjPtr &obj) +namespace GC { - GC::Mark(&obj.o); + template inline void Mark(TObjPtr &obj) + { + GC::Mark(&obj.o); + } } diff --git a/src/doomstat.h b/src/doomstat.h index 8086a8f0f..6f0007f4c 100644 --- a/src/doomstat.h +++ b/src/doomstat.h @@ -197,8 +197,6 @@ EXTERN_CVAR (Float, mouse_sensitivity) // debug flag to cancel adaptiveness extern bool singletics; -extern int bodyqueslot; - // Needed to store the number of the dummy sky flat. diff --git a/src/g_game.cpp b/src/g_game.cpp index 54ca664cf..9be9b5455 100644 --- a/src/g_game.cpp +++ b/src/g_game.cpp @@ -70,6 +70,7 @@ #include "p_spec.h" #include "serializer.h" #include "vm.h" +#include "dobjgc.h" #include "g_hub.h" #include "g_levellocals.h" @@ -211,9 +212,6 @@ FString savedescription; // [RH] Name of screenshot file to generate (usually NULL) FString shotfile; -AActor* bodyque[BODYQUESIZE]; -int bodyqueslot; - FString savename; FString BackupSaveName; @@ -1676,13 +1674,14 @@ DEFINE_ACTION_FUNCTION(DObject, G_PickPlayerStart) static void G_QueueBody (AActor *body) { // flush an old corpse if needed - int modslot = bodyqueslot%BODYQUESIZE; + int modslot = level.bodyqueslot%level.BODYQUESIZE; + level.bodyqueslot = modslot + 1; - if (bodyqueslot >= BODYQUESIZE && bodyque[modslot] != NULL) + if (level.bodyqueslot >= level.BODYQUESIZE && level.bodyque[modslot] != NULL) { - bodyque[modslot]->Destroy (); + level.bodyque[modslot]->Destroy (); } - bodyque[modslot] = body; + level.bodyque[modslot] = body; // Copy the player's translation, so that if they change their color later, only // their current body will change and not all their old corpses. @@ -1706,7 +1705,6 @@ static void G_QueueBody (AActor *body) body->Scale.Y *= skin.Scale.Y / defaultActor->Scale.Y; } - bodyqueslot++; } // diff --git a/src/g_game.h b/src/g_game.h index 7ac49a413..6f5f93261 100644 --- a/src/g_game.h +++ b/src/g_game.h @@ -30,6 +30,7 @@ struct event_t; +#include "dobjgc.h" // // GAME @@ -94,10 +95,6 @@ void G_AddViewPitch (int look, bool mouse = false); // Adds to consoleplayer's viewangle if allowed void G_AddViewAngle (int yaw, bool mouse = false); -#define BODYQUESIZE 32 -class AActor; -extern AActor *bodyque[BODYQUESIZE]; -extern int bodyqueslot; class AInventory; extern const AInventory *SendItemUse, *SendItemDrop; extern int SendItemDropAmount; diff --git a/src/g_level.cpp b/src/g_level.cpp index b1a7cb3ad..775d226fa 100644 --- a/src/g_level.cpp +++ b/src/g_level.cpp @@ -1956,6 +1956,24 @@ void FLevelLocals::Tick () // //========================================================================== +void FLevelLocals::Mark() +{ + for (auto &s : sectorPortals) + { + GC::Mark(s.mSkybox); + } + // Mark dead bodies. + for (auto &p : bodyque) + { + GC::Mark(p); + } +} + +//========================================================================== +// +// +//========================================================================== + void FLevelLocals::AddScroller (int secnum) { if (secnum < 0) diff --git a/src/g_levellocals.h b/src/g_levellocals.h index 1e3aed96f..86050b68c 100644 --- a/src/g_levellocals.h +++ b/src/g_levellocals.h @@ -47,6 +47,7 @@ struct FLevelLocals { void Tick (); + void Mark(); void AddScroller (int secnum); void SetInterMusic(const char *nextmap); void SetMusicVolume(float v); @@ -85,6 +86,11 @@ struct FLevelLocals TArray gamenodes; node_t *headgamenode; TArray rejectmatrix; + + static const int BODYQUESIZE = 32; + TObjPtr bodyque[BODYQUESIZE]; + int bodyqueslot; + TArray sectorPortals; TArray linePortals; diff --git a/src/p_mobj.cpp b/src/p_mobj.cpp index ff48ab79d..9d3b7d931 100644 --- a/src/p_mobj.cpp +++ b/src/p_mobj.cpp @@ -5772,10 +5772,7 @@ APlayerPawn *P_SpawnPlayer (FPlayerStart *mthing, int playernum, int flags) } DObject::StaticPointerSubstitution (oldactor, p->mo); - // PointerSubstitution() will also affect the bodyque, so undo that now. - for (int ii=0; ii < BODYQUESIZE; ++ii) - if (bodyque[ii] == p->mo) - bodyque[ii] = oldactor; + E_PlayerRespawned(int(p - players)); FBehavior::StaticStartTypedScripts (SCRIPT_Respawn, p->mo, true); } diff --git a/src/p_saveg.cpp b/src/p_saveg.cpp index a8dff6868..e5c111a8f 100644 --- a/src/p_saveg.cpp +++ b/src/p_saveg.cpp @@ -971,7 +971,9 @@ void G_SerializeLevel(FSerializer &arc, bool hubload) ("level.skytexture2", level.skytexture2) ("level.fogdensity", level.fogdensity) ("level.outsidefogdensity", level.outsidefogdensity) - ("level.skyfog", level.skyfog); + ("level.skyfog", level.skyfog) + ("level.bodyqueslot", level.bodyqueslot) + .Array("level.bodyque", level.bodyque, level.BODYQUESIZE); // Hub transitions must keep the current total time if (!hubload) diff --git a/src/p_setup.cpp b/src/p_setup.cpp index 8d502b42b..820596ecb 100644 --- a/src/p_setup.cpp +++ b/src/p_setup.cpp @@ -4048,12 +4048,12 @@ void P_SetupLevel(const char *lumpname, int position, bool newGame) FixHoles(); } - bodyqueslot = 0; + level.bodyqueslot = 0; // phares 8/10/98: Clear body queue so the corpses from previous games are // not assumed to be from this one. - for (i = 0; i < BODYQUESIZE; i++) - bodyque[i] = NULL; + for(auto & p : level.bodyque) + p = nullptr; CreateSections(level.sections); diff --git a/src/r_data/r_translate.cpp b/src/r_data/r_translate.cpp index eeca41864..c11f9be94 100644 --- a/src/r_data/r_translate.cpp +++ b/src/r_data/r_translate.cpp @@ -49,6 +49,7 @@ #include "vm.h" #include "v_text.h" #include "m_crc32.h" +#include "g_levellocals.h" #include "gi.h" @@ -926,7 +927,7 @@ void R_InitTranslationTables () // Each player corpse has its own translation so they won't change // color if the player who created them changes theirs. - for (i = 0; i < BODYQUESIZE; ++i) + for (i = 0; i < level.BODYQUESIZE; ++i) { PushIdentityTable(TRANSLATION_PlayerCorpses); } From e90ed0a01cd64152ccf0ad03e7a95d6132fbd6bc Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Mon, 19 Nov 2018 18:26:23 +0100 Subject: [PATCH 3/4] removed the quite redundant GetStateForButtonName function Since it forwards directly to FindState and has no script bindings there is no need to keep it, it'd only complicate the full scriptification of the weapon class if it stuck around. --- src/g_inventory/a_weapons.cpp | 11 ----------- src/g_inventory/a_weapons.h | 2 -- src/p_pspr.cpp | 2 +- 3 files changed, 1 insertion(+), 14 deletions(-) diff --git a/src/g_inventory/a_weapons.cpp b/src/g_inventory/a_weapons.cpp index c026eda04..a72a10d7e 100644 --- a/src/g_inventory/a_weapons.cpp +++ b/src/g_inventory/a_weapons.cpp @@ -467,17 +467,6 @@ FState *AWeapon::GetReadyState () return nullptr; } -//=========================================================================== -// -// AWeapon :: GetStateForButtonName -// -//=========================================================================== - -FState *AWeapon::GetStateForButtonName (FName button) -{ - return FindState(button); -} - /* Weapon slots ***********************************************************/ diff --git a/src/g_inventory/a_weapons.h b/src/g_inventory/a_weapons.h index 4c6f6ab45..1e0634ddd 100644 --- a/src/g_inventory/a_weapons.h +++ b/src/g_inventory/a_weapons.h @@ -132,8 +132,6 @@ public: FState *GetUpState (); FState *GetDownState (); FState *GetReadyState (); - - FState *GetStateForButtonName (FName button); enum { diff --git a/src/p_pspr.cpp b/src/p_pspr.cpp index 948cdf8a0..76587f6cb 100644 --- a/src/p_pspr.cpp +++ b/src/p_pspr.cpp @@ -892,7 +892,7 @@ static void P_CheckWeaponButtons (player_t *player) if ((player->WeaponState & ButtonChecks[i].StateFlag) && (player->cmd.ucmd.buttons & ButtonChecks[i].ButtonFlag)) { - FState *state = weapon->GetStateForButtonName(ButtonChecks[i].StateName); + FState *state = weapon->FindState(ButtonChecks[i].StateName); // [XA] don't change state if still null, so if the modder // sets WRF_xxx to true but forgets to define the corresponding // state, the weapon won't disappear. ;) From a5ee673c915424a86d50d87332884ac303b78543 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Mon, 19 Nov 2018 17:54:38 +0100 Subject: [PATCH 4/4] - exported ADecal to ZScript as a non-native class. Its one function is still native but this was by far the easiest of the remaining actor classes to export. --- src/g_shared/a_decals.cpp | 31 +++++++++----------------- wadsrc/static/zscript/shared/decal.txt | 10 ++++++++- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/g_shared/a_decals.cpp b/src/g_shared/a_decals.cpp index 19216ecd2..bb503fec0 100644 --- a/src/g_shared/a_decals.cpp +++ b/src/g_shared/a_decals.cpp @@ -43,6 +43,7 @@ #include "serializer.h" #include "doomdata.h" #include "g_levellocals.h" +#include "vm.h" static double DecalWidth, DecalLeft, DecalRight; static double SpreadZ; @@ -753,29 +754,20 @@ DBaseDecal *ShootDecal(const FDecalTemplate *tpl, AActor *basisactor, sector_t * return NULL; } -class ADecal : public AActor +DEFINE_ACTION_FUNCTION(ADecal, SpawnDecal) { - DECLARE_CLASS (ADecal, AActor); -public: - void BeginPlay (); -}; + PARAM_SELF_PROLOGUE(AActor); -IMPLEMENT_CLASS(ADecal, false, false) - -void ADecal::BeginPlay () -{ const FDecalTemplate *tpl = nullptr; - Super::BeginPlay (); - - if (args[0] < 0) + if (self->args[0] < 0) { - FName name = ENamedName(-args[0]); + FName name = ENamedName(-self->args[0]); tpl = DecalLibrary.GetDecalByName(name.GetChars()); } else { - int decalid = args[0] + (args[1] << 8); // [KS] High byte for decals. + int decalid = self->args[0] + (self->args[1] << 8); // [KS] High byte for decals. tpl = DecalLibrary.GetDecalByNum(decalid); } @@ -784,23 +776,22 @@ void ADecal::BeginPlay () { if (!tpl->PicNum.Exists()) { - Printf("Decal actor at (%f,%f) does not have a valid texture\n", X(), Y()); + Printf("Decal actor at (%f,%f) does not have a valid texture\n", self->X(), self->Y()); } else { // Look for a wall within 64 units behind the actor. If none can be // found, then no decal is created, and this actor is destroyed // without effectively doing anything. - if (NULL == ShootDecal(tpl, this, Sector, X(), Y(), Z(), Angles.Yaw + 180, 64., true)) + if (NULL == ShootDecal(tpl, self, self->Sector, self->X(), self->Y(), self->Z(), self->Angles.Yaw + 180, 64., true)) { - DPrintf (DMSG_WARNING, "Could not find a wall to stick decal to at (%f,%f)\n", X(), Y()); + DPrintf (DMSG_WARNING, "Could not find a wall to stick decal to at (%f,%f)\n", self->X(), self->Y()); } } } else { - DPrintf (DMSG_ERROR, "Decal actor at (%f,%f) does not have a good template\n", X(), Y()); + DPrintf (DMSG_ERROR, "Decal actor at (%f,%f) does not have a good template\n", self->X(), self->Y()); } - // This actor doesn't need to stick around anymore. - Destroy(); + return 0; } diff --git a/wadsrc/static/zscript/shared/decal.txt b/wadsrc/static/zscript/shared/decal.txt index 0614f6235..7317ecce1 100644 --- a/wadsrc/static/zscript/shared/decal.txt +++ b/wadsrc/static/zscript/shared/decal.txt @@ -1,3 +1,11 @@ -class Decal : Actor native +class Decal : Actor { + native void SpawnDecal(); + + override void BeginPlay() + { + Super.BeginPlay(); + SpawnDecal(); + Destroy(); + } }