From dcb393bc440cb612fff2ba8652bcb557606e7759 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Fri, 2 Apr 2021 10:28:18 +0200 Subject: [PATCH] - started reorganizing SW's memory management. Need to get rid of all those unmanaged allocations and present game data in an easily serializable form. This adds a managed TPointer class that replicates the useful parts of std::unique_pointer but steers clear of its properties that often render it useless. --- source/common/engine/serializer.h | 26 +++++++ source/common/utility/tarray.h | 122 ++++++++++++++++++++++++++++++ source/games/sw/src/game.h | 30 +++++--- source/games/sw/src/save.cpp | 6 +- source/games/sw/src/sprite.cpp | 8 +- 5 files changed, 175 insertions(+), 17 deletions(-) diff --git a/source/common/engine/serializer.h b/source/common/engine/serializer.h index 22f32f73c..b22d0e11b 100644 --- a/source/common/engine/serializer.h +++ b/source/common/engine/serializer.h @@ -269,6 +269,32 @@ FSerializer &Serialize(FSerializer &arc, const char *key, TArray &value, return arc; } +template +FSerializer& Serialize(FSerializer& arc, const char* key, TPointer& value, TPointer* def) +{ + if (arc.isWriting()) + { + if (value.Data() == nullptr && key) return arc; + } + bool res = arc.BeginArray(key); + if (arc.isReading()) + { + if (!res || arc.ArraySize() == 0) + { + value.Clear(); + return arc; + } + value.Alloc(); + } + if (value.Data()) + { + Serialize(arc, nullptr, *value, def ? def->Data() : nullptr); + } + arc.EndArray(); + return arc; +} + + template FSerializer& Serialize(FSerializer& arc, const char* key, FixedBitArray& value, FixedBitArray* def) { diff --git a/source/common/utility/tarray.h b/source/common/utility/tarray.h index c44e06c43..c8508a022 100644 --- a/source/common/utility/tarray.h +++ b/source/common/utility/tarray.h @@ -1368,6 +1368,128 @@ protected: }; +// Pointer wrapper without the unpleasant side effects of std::unique_ptr, mainly the inability to copy it. +// This class owns the object with no means to release it, and copying the pointer copies the object. +template +class TPointer +{ +public: + + //////// + TPointer() + { + Ptr = nullptr; + } + TPointer(const T& other) + { + Alloc(); + *Ptr = other; + } + TPointer(T&& other) + { + Alloc(); + *Ptr = other; + } + TPointer(const TPointer& other) + { + DoCopy(other); + } + TPointer(TPointer&& other) + { + Ptr = other.Ptr; + other.Ptr = nullptr; + } + TPointer& operator= (const T& other) + { + if (&other != this) + { + Alloc(); + *Ptr = other; + } + return *this; + } + TPointer& operator= (const TPointer& other) + { + if (&other != this) + { + DoCopy(other); + } + return *this; + } + TPointer& operator= (TPointer&& other) + { + if (&other != this) + { + if (Ptr) delete Ptr; + Ptr = other.Ptr; + other.Ptr = nullptr; + } + return *this; + } + ~TPointer() + { + if (Ptr) delete Ptr; + Ptr = nullptr; + } + // Check equality of two pointers + bool operator==(const TPointer& other) const + { + return *Ptr == *other.Ptr; + } + + T& operator* () const + { + assert(Ptr); + return *Ptr; + } + + T* operator->() { return Ptr; } + + // returns raw pointer + T* Data() const + { + return Ptr; + } + + operator T* () const + { + return Ptr; + } + + void Alloc() + { + if (!Ptr) Ptr = new T; + } + + void Clear() + { + if (Ptr) delete Ptr; + Ptr = nullptr; + } + + void Swap(TPointer& other) + { + std::swap(Ptr, other.Ptr); + } + +private: + T* Ptr; + + void DoCopy(const TPointer& other) + { + if (other.Ptr == nullptr) + { + Clear(); + } + else + { + Alloc(); + *Ptr = *other.Ptr; + } + } +}; + + //========================================================================== // diff --git a/source/games/sw/src/game.h b/source/games/sw/src/game.h index c7dda49e2..5452cc9c1 100644 --- a/source/games/sw/src/game.h +++ b/source/games/sw/src/game.h @@ -1115,12 +1115,22 @@ typedef struct // User Extension record // -typedef struct +struct USER { + // These are for easy zero-init of USERs without having to be on the lookout for non-trivial members. + void* operator new(size_t alloc) + { + return M_Calloc(alloc, 1); + } + void operator delete (void* mem) + { + M_Free(mem); + } + // // Variables that can be used by actors and Player // - ROTATORp rotator; + TPointer rotator; // wall vars for lighting int WallCount; @@ -1269,7 +1279,9 @@ typedef struct int16_t oangdiff; // Used for interpolating sprite angles uint8_t filler; -} USER,*USERp; +}; + +using USERp = USER*; struct USERSAVE { @@ -1440,21 +1452,15 @@ typedef struct } RANGE,*RANGEp; -inline void ClearUser(USER* user) -{ - *user = {}; -} - +#pragma message ("Remove NewUser/FreeUser!") inline USER* NewUser() { - auto u = (USER*)M_Calloc(sizeof(USER), 1);// new USER; - ClearUser(u); - return u; + return new USER; } inline void FreeUser(int num) { - if (User[num]) M_Free(User[num]);// delete User[num]; + if (User[num]) delete User[num]; User[num] = nullptr; } diff --git a/source/games/sw/src/save.cpp b/source/games/sw/src/save.cpp index 6feeff775..ae452686e 100644 --- a/source/games/sw/src/save.cpp +++ b/source/games/sw/src/save.cpp @@ -747,7 +747,11 @@ bool GameInterface::LoadGame() while (SpriteNum != -1) { User[SpriteNum] = u = NewUser(); + // We need to be careful with allocated content in User when loading a binary save state. + // This needs to be refactored out ASAP. + u->rotator.Clear(); MREAD(u,sizeof(USER),1,fil); + memset((void*)&u->rotator, 0, sizeof(u->rotator)); if (u->WallShade) { @@ -757,7 +761,7 @@ bool GameInterface::LoadGame() if (u->rotator) { - u->rotator = (ROTATORp)CallocMem(sizeof(*u->rotator), 1); + u->rotator.Alloc(); MREAD(u->rotator,sizeof(*u->rotator),1,fil); if (u->rotator->origx) diff --git a/source/games/sw/src/sprite.cpp b/source/games/sw/src/sprite.cpp index 3eb4309e3..5caa2b056 100644 --- a/source/games/sw/src/sprite.cpp +++ b/source/games/sw/src/sprite.cpp @@ -790,7 +790,8 @@ KillSprite(int16_t SpriteNum) FreeMem(u->rotator->origx); if (u->rotator->origy) FreeMem(u->rotator->origy); - FreeMem(u->rotator); + + u->rotator.Clear(); } FreeUser(SpriteNum); @@ -909,7 +910,6 @@ SpawnUser(short SpriteNum, short id, STATEp state) u->WaitTics = 0; u->OverlapZ = Z(4); u->WallShade = NULL; - u->rotator = NULL; u->bounce = 0; u->motion_blur_num = 0; @@ -2404,7 +2404,7 @@ SpriteSetup(void) for (w = startwall, wallcount = 0; w <= endwall; w++) wallcount++; - u->rotator = (ROTATORp)CallocMem(sizeof(ROTATOR), 1); + u->rotator.Alloc(); u->rotator->num_walls = wallcount; u->rotator->open_dest = SP_TAG5(sp); u->rotator->speed = SP_TAG7(sp); @@ -2460,7 +2460,7 @@ SpriteSetup(void) u->WaitTics = time*15; // 1/8 of a sec u->Tics = 0; - u->rotator = (ROTATORp)CallocMem(sizeof(ROTATOR), 1); + u->rotator.Alloc(); u->rotator->open_dest = SP_TAG5(sp); u->rotator->speed = SP_TAG7(sp); u->rotator->vel = SP_TAG8(sp);