From ea1d6634f7e0f0374fab3f35973eb6b556a52125 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Fri, 17 Mar 2017 12:09:38 +0100 Subject: [PATCH] - moved the Zones array into FLevelLocals. - replaced TStaticArray with regular TArrays. They had incomplete implementations preventing proper cleanup of the level loading code. It makes more sense to add the missing methods to the regular TArray and use that. This also makes some changes to how the game nodes are used to avoid creating a copy: If the head node's pointer is stored in a separate variable, no code needs to check which of the two arrays gets used. --- src/g_levellocals.h | 28 +++++--- src/gl/scene/gl_clipper.h | 1 - src/p_glnodes.cpp | 22 +++--- src/p_maputl.cpp | 13 +--- src/p_saveg.cpp | 29 ++------ src/p_saveg.h | 3 - src/p_sectors.cpp | 16 ++--- src/p_setup.cpp | 61 ++++++----------- src/r_state.h | 9 --- src/s_sound.cpp | 4 +- src/tarray.h | 137 +++++--------------------------------- 11 files changed, 80 insertions(+), 243 deletions(-) diff --git a/src/g_levellocals.h b/src/g_levellocals.h index 48f852902..433d8a3a4 100644 --- a/src/g_levellocals.h +++ b/src/g_levellocals.h @@ -29,17 +29,25 @@ struct FLevelLocals FString F1Pic; EMapType maptype; - TStaticArray vertexes; - TStaticArray sectors; - TStaticArray lines; - TStaticArray sides; - TStaticArray segs; - TStaticPointableArray subsectors; - TStaticPointableArray gamesubsectors; - TStaticPointableArray nodes; - TStaticPointableArray gamenodes; + TArray vertexes; + TArray sectors; + TArray lines; + TArray sides; + TArray segs; + TArray subsectors; + TArray nodes; + TArray gamesubsectors; + TArray gamenodes; + node_t *headgamenode; TArray sectorPortals; + TArray Zones; + + // These are copies of the loaded map data that get used by the savegame code to skip unaltered fields + // Without such a mechanism the savegame format would become too slow and large because more than 80-90% are normally still unaltered. + TArray loadsectors; + TArray loadlines; + TArray loadsides; uint32_t flags; @@ -102,7 +110,7 @@ struct FLevelLocals } node_t *HeadGamenode() const { - return &gamenodes[gamenodes.Size() - 1]; + return headgamenode; } }; diff --git a/src/gl/scene/gl_clipper.h b/src/gl/scene/gl_clipper.h index 9494a1243..55a072909 100644 --- a/src/gl/scene/gl_clipper.h +++ b/src/gl/scene/gl_clipper.h @@ -31,7 +31,6 @@ class ClipNode class Clipper { unsigned starttime; - TStaticArray anglecache; FMemArena nodearena; ClipNode * freelist = nullptr; diff --git a/src/p_glnodes.cpp b/src/p_glnodes.cpp index 47700b1d1..9901f8d8a 100644 --- a/src/p_glnodes.cpp +++ b/src/p_glnodes.cpp @@ -247,15 +247,14 @@ static bool LoadGLVertexes(FileReader * lump) mapglvertex_t* mgl = (mapglvertex_t *)(gldata + GL_VERT_OFFSET); unsigned numvertexes = firstglvertex + (gllen - GL_VERT_OFFSET)/sizeof(mapglvertex_t); - TStaticArray oldvertexes = std::move(level.vertexes); - level.vertexes.Alloc(numvertexes); + auto oldvertexes = &level.vertexes[0]; + level.vertexes.Resize(numvertexes); - memcpy(&level.vertexes[0], &oldvertexes[0], firstglvertex * sizeof(vertex_t)); for(auto &line : level.lines) { // Remap vertex pointers in linedefs - line.v1 = &level.vertexes[line.v1 - &oldvertexes[0]]; - line.v2 = &level.vertexes[line.v2 - &oldvertexes[0]]; + line.v1 = &level.vertexes[line.v1 - oldvertexes]; + line.v2 = &level.vertexes[line.v2 - oldvertexes]; } for (i = firstglvertex; i < (int)numvertexes; i++) @@ -940,13 +939,14 @@ bool P_CheckNodes(MapData * map, bool rebuilt, int buildtime) if (!rebuilt && !P_CheckForGLNodes()) { ret = true; // we are not using the level's original nodes if we get here. - for (auto &sub : level.gamesubsectors) + for (auto &sub : level.subsectors) { sub.sector = sub.firstline->sidedef->sector; } - level.nodes.Clear(); - level.subsectors.Clear(); + // The nodes and subsectors need to be preserved for gameplay related purposes. + level.gamenodes = std::move(level.nodes); + level.gamesubsectors = std::move(level.subsectors); level.segs.Clear(); // Try to load GL nodes (cached or GWA) @@ -992,12 +992,6 @@ bool P_CheckNodes(MapData * map, bool rebuilt, int buildtime) DPrintf(DMSG_NOTIFY, "Not caching nodes (time = %f)\n", buildtime/1000.f); } } - - if (level.gamenodes.Size() == 0) - { - level.gamenodes.Point(level.nodes); - level.gamesubsectors.Point(level.subsectors); - } return ret; } diff --git a/src/p_maputl.cpp b/src/p_maputl.cpp index 25d23ff9f..ca2f14d10 100644 --- a/src/p_maputl.cpp +++ b/src/p_maputl.cpp @@ -2101,14 +2101,10 @@ int P_VanillaPointOnLineSide(double x, double y, const line_t* line) subsector_t *P_PointInSubsector(double x, double y) { - node_t *node; int side; - // single subsector is a special case - if (level.gamenodes.Size() == 0) - return &level.gamesubsectors[0]; - - node = level.HeadGamenode(); + auto node = level.HeadGamenode(); + if (node == nullptr) return &level.subsectors[0]; fixed_t xx = FLOAT2FIXED(x); fixed_t yy = FLOAT2FIXED(y); @@ -2131,11 +2127,8 @@ subsector_t *P_PointInSubsector(double x, double y) sector_t *P_PointInSectorBuggy(double x, double y) { // single subsector is a special case - if (level.gamenodes.Size() == 0) - return level.gamesubsectors[0].sector; - auto node = level.HeadGamenode(); - + if (node == nullptr) return level.subsectors[0].sector; do { // Use original buggy point-on-side test when spawning diff --git a/src/p_saveg.cpp b/src/p_saveg.cpp index 52609ca2e..72d87855a 100644 --- a/src/p_saveg.cpp +++ b/src/p_saveg.cpp @@ -65,11 +65,6 @@ #include "g_levellocals.h" #include "events.h" -static TStaticArray loadsectors; -static TStaticArray loadlines; -static TStaticArray loadsides; - - //========================================================================== // // @@ -991,10 +986,10 @@ void G_SerializeLevel(FSerializer &arc, bool hubload) FBehavior::StaticSerializeModuleStates(arc); // The order here is important: First world state, then portal state, then thinkers, and last polyobjects. - arc.Array("linedefs", &level.lines[0], &loadlines[0], level.lines.Size()); - arc.Array("sidedefs", &level.sides[0], &loadsides[0], level.sides.Size()); - arc.Array("sectors", &level.sectors[0], &loadsectors[0], level.sectors.Size()); - arc("zones", Zones); + arc("linedefs", level.lines, level.loadlines); + arc("sidedefs", level.sides, level.loadsides); + arc("sectors", level.sectors, level.loadsectors); + arc("zones", level.Zones); arc("lineportals", linePortals); arc("sectorportals", level.sectorPortals); if (arc.isReading()) P_CollectLinkedPortals(); @@ -1029,19 +1024,3 @@ void G_SerializeLevel(FSerializer &arc, bool hubload) Renderer->EndSerialize(arc); } - -// Create a backup of the map data so the savegame code can toss out all fields that haven't changed in order to reduce processing time and file size. - -void P_BackupMapData() -{ - loadsectors = level.sectors; - loadlines = level.lines; - loadsides = level.sides; -} - -void P_FreeMapDataBackup() -{ - loadsectors.Clear(); - loadlines.Clear(); - loadsides.Clear(); -} diff --git a/src/p_saveg.h b/src/p_saveg.h index 715fd6a19..6c4c74f46 100644 --- a/src/p_saveg.h +++ b/src/p_saveg.h @@ -46,7 +46,4 @@ void P_WriteACSDefereds (FSerializer &); void G_SerializeLevel(FSerializer &arc, bool hubLoad); -void P_BackupMapData(); -void P_FreeMapDataBackup(); - #endif // __P_SAVEG_H__ diff --git a/src/p_sectors.cpp b/src/p_sectors.cpp index 834736719..7dac68e1a 100644 --- a/src/p_sectors.cpp +++ b/src/p_sectors.cpp @@ -1879,18 +1879,18 @@ DEFINE_ACTION_FUNCTION(_Sector, NextLowestFloorAt) DEFINE_ACTION_FUNCTION(_Sector, SetEnvironmentID) { - PARAM_SELF_STRUCT_PROLOGUE(sector_t); - PARAM_INT(envnum); - Zones[self->ZoneNumber].Environment = S_FindEnvironment(envnum); - return 0; + PARAM_SELF_STRUCT_PROLOGUE(sector_t); + PARAM_INT(envnum); + level.Zones[self->ZoneNumber].Environment = S_FindEnvironment(envnum); + return 0; } DEFINE_ACTION_FUNCTION(_Sector, SetEnvironment) { - PARAM_SELF_STRUCT_PROLOGUE(sector_t); - PARAM_STRING(env); - Zones[self->ZoneNumber].Environment = S_FindEnvironment(env); - return 0; + PARAM_SELF_STRUCT_PROLOGUE(sector_t); + PARAM_STRING(env); + level.Zones[self->ZoneNumber].Environment = S_FindEnvironment(env); + return 0; } DEFINE_ACTION_FUNCTION(_Sector, GetGlowHeight) diff --git a/src/p_setup.cpp b/src/p_setup.cpp index 04987fe61..5c395e34b 100644 --- a/src/p_setup.cpp +++ b/src/p_setup.cpp @@ -125,14 +125,6 @@ inline bool P_LoadBuildMap(uint8_t *mapdata, size_t len, FMapThing **things, int // TArray vertexdatas; -//int numnodes; -//node_t* nodes; - -TArray Zones; - -//node_t * gamenodes; -//int numgamenodes; - bool hasglnodes; TArray MapThingsConverted; @@ -799,7 +791,7 @@ void P_FloodZones () P_FloodZone (&sec, z++); } } - Zones.Resize(z); + level.Zones.Resize(z); reverb = S_FindEnvironment(level.DefaultEnvironment); if (reverb == NULL) { @@ -808,7 +800,7 @@ void P_FloodZones () } for (i = 0; i < z; ++i) { - Zones[i].Environment = reverb; + level.Zones[i].Environment = reverb; } } @@ -961,7 +953,6 @@ void LoadZNodes(FileReaderBase &data, int glnodes) { // Read extra vertices added during node building uint32_t orgVerts, newVerts; - TStaticArray newvertarray; unsigned int i; data >> orgVerts >> newVerts; @@ -970,35 +961,26 @@ void LoadZNodes(FileReaderBase &data, int glnodes) // We can't use them. throw CRecoverableError("Incorrect number of vertexes in nodes.\n"); } - bool fix; - if (orgVerts + newVerts == level.vertexes.Size()) + auto oldvertexes = &level.vertexes[0]; + if (orgVerts + newVerts != level.vertexes.Size()) { - newvertarray = std::move(level.vertexes); - fix = false; - } - else - { - newvertarray.Alloc(orgVerts + newVerts); - memcpy (&newvertarray[0], &level.vertexes[0], orgVerts * sizeof(vertex_t)); - fix = true; + level.vertexes.Reserve(newVerts); } for (i = 0; i < newVerts; ++i) { fixed_t x, y; data >> x >> y; - newvertarray[i + orgVerts].set(x, y); + level.vertexes[i + orgVerts].set(x, y); } - if (fix) + if (oldvertexes != &level.vertexes[0]) { for (auto &line : level.lines) { - line.v1 = line.v1 - &level.vertexes[0] + &newvertarray[0]; - line.v2 = line.v2 - &level.vertexes[0] + &newvertarray[0]; + line.v1 = line.v1 - oldvertexes + &level.vertexes[0]; + line.v2 = line.v2 - oldvertexes + &level.vertexes[0]; } } - level.vertexes = std::move(newvertarray); - // Read the subsectors uint32_t numSubs, currSeg; @@ -3424,7 +3406,6 @@ void P_FreeLevelData () // [RH] Clear all ThingID hash chains. AActor::ClearTIDHashes(); - P_FreeMapDataBackup(); interpolator.ClearInterpolations(); // [RH] Nothing to interpolate on a fresh level. Renderer->CleanLevelData(); FPolyObj::ClearAllSubsectorLinks(); // can't be done as part of the polyobj deletion process. @@ -3450,11 +3431,14 @@ void P_FreeLevelData () level.sectors.Clear(); level.lines.Clear(); level.sides.Clear(); + level.loadsectors.Clear(); + level.loadlines.Clear(); + level.loadsides.Clear(); level.vertexes.Clear(); level.nodes.Clear(); - level.gamenodes.Clear(); + level.gamenodes.Reset(); level.subsectors.Clear(); - level.gamesubsectors.Clear(); + level.gamesubsectors.Reset(); if (blockmaplump != NULL) { @@ -3497,7 +3481,7 @@ void P_FreeLevelData () polyobjs = NULL; } po_NumPolyobjs = 0; - Zones.Clear(); + level.Zones.Clear(); P_FreeStrifeConversations (); level.Scrolls.Clear(); P_ClearUDMFKeys(); @@ -3895,13 +3879,6 @@ void P_SetupLevel (const char *lumpname, int position) } } - // duplicate the nodes in the game* arrays so that replacement nodes do not overwrite them. - // let the game data take ownership, because that is guaranteed to remain intact after here. - level.gamesubsectors = std::move(level.subsectors); - level.subsectors.Point(level.gamesubsectors); - level.gamenodes = std::move(level.nodes); - level.nodes.Point(level.gamenodes); - if (RequireGLNodes) { // Build GL nodes if we want a textured automap or GL nodes are forced to be built. @@ -3916,6 +3893,9 @@ void P_SetupLevel (const char *lumpname, int position) hasglnodes = P_CheckForGLNodes(); } + // set the head node for gameplay purposes. If the separate gamenodes array is not empty, use that, otherwise use the render nodes. + level.headgamenode = level.gamenodes.Size() > 0 ? &level.gamenodes[level.gamenodes.Size() - 1] : &level.nodes[level.nodes.Size() - 1]; + times[10].Clock(); P_LoadBlockMap (map); times[10].Unclock(); @@ -4119,7 +4099,10 @@ void P_SetupLevel (const char *lumpname, int position) MapThingsUserDataIndex.Clear(); MapThingsUserData.Clear(); - P_BackupMapData(); + // Create a backup of the map data so the savegame code can toss out all fields that haven't changed in order to reduce processing time and file size. + level.loadsectors = level.sectors; + level.loadlines = level.lines; + level.loadsides = level.sides; } diff --git a/src/r_state.h b/src/r_state.h index 56e377d3a..309f70607 100644 --- a/src/r_state.h +++ b/src/r_state.h @@ -46,15 +46,6 @@ extern uint32_t NumStdSprites; extern TArray vertexdatas; -//extern int numnodes; -//extern node_t* nodes; - -extern TArray Zones; - -//extern node_t * gamenodes; -//extern int numgamenodes; - - // // POV data. // diff --git a/src/s_sound.cpp b/src/s_sound.cpp index 5e9760a3c..a767fdcaa 100644 --- a/src/s_sound.cpp +++ b/src/s_sound.cpp @@ -2109,8 +2109,8 @@ static void S_SetListener(SoundListener &listener, AActor *listenactor) listener.velocity.Zero(); listener.position = listenactor->SoundPos(); listener.underwater = listenactor->waterlevel == 3; - assert(Zones.Size() > listenactor->Sector->ZoneNumber); - listener.Environment = Zones[listenactor->Sector->ZoneNumber].Environment; + assert(level.Zones.Size() > listenactor->Sector->ZoneNumber); + listener.Environment = level.Zones[listenactor->Sector->ZoneNumber].Environment; listener.valid = true; } else diff --git a/src/tarray.h b/src/tarray.h index 636d7a1d1..c8731f984 100644 --- a/src/tarray.h +++ b/src/tarray.h @@ -366,6 +366,14 @@ public: } Count = amount; } + void Alloc(unsigned int amount) + { + // first destroys all content and then rebuilds the array. + if (Count > 0) DoDelete(0, Count - 1); + Count = 0; + Resize(amount); + ShrinkToFit(); + } // Reserves amount entries at the end of the array, but does nothing // with them. unsigned int Reserve (unsigned int amount) @@ -395,6 +403,11 @@ public: Count = 0; } } + void Reset() + { + Clear(); + ShrinkToFit(); + } private: T *Array; unsigned int Count; @@ -466,13 +479,9 @@ public: } }; -// A non-resizable array -// This is meant for data that can be replaced but is otherwise static as long as it exists. -// The reason for it is to replace any global pointer/counter pairs with something that can -// be reliably accessed by the scripting VM and which can use 'for' iterator syntax. +// This is not a real dynamic array but just a wrapper around a pointer reference. +// Used for wrapping some memory allocated elsewhere into a VM compatible data structure. -// This is split into two, so that it also can be used to wrap arrays that are not directly allocated. -// This first class only gets a reference to some data but won't own it. template class TStaticPointedArray { @@ -526,122 +535,6 @@ public: unsigned int Count; }; -// This second type owns its data, it can delete and reallocate it, but it cannot -// resize the array or repurpose its old contents if new ones are about to be created. -template -class TStaticArray : public TStaticPointedArray -{ -public: - - //////// - // This is a dummy constructor that does nothing. The purpose of this - // is so you can create a global TArray in the data segment that gets - // used by code before startup without worrying about the constructor - // resetting it after it's already been used. You MUST NOT use it for - // heap- or stack-allocated TArrays. - enum ENoInit - { - NoInit - }; - TStaticArray(ENoInit dummy) - { - } - //////// - TStaticArray() - { - this->Count = 0; - this->Array = NULL; - } - TStaticArray(TStaticArray &&other) - { - this->Array = other.Array; - this->Count = other.Count; - other.Array = nullptr; - other.Count = 0; - } - // This is not supposed to be copyable. - TStaticArray(const TStaticArray &other) = delete; - - ~TStaticArray() - { - Clear(); - } - void Clear() - { - if (this->Array) delete[] this->Array; - this->Count = 0; - this->Array = nullptr; - } - void Alloc(unsigned int amount) - { - // intentionally first deletes and then reallocates. - if (this->Array) delete[] this->Array; - this->Array = new T[amount]; - this->Count = amount; - } - TStaticArray &operator=(const TStaticArray &other) - { - Alloc(other.Size()); - memcpy(this->Array, other.Array, this->Count * sizeof(T)); - return *this; - } - TStaticArray &operator=(TStaticArray &&other) - { - if (this->Array) delete[] this->Array; - this->Array = other.Array; - this->Count = other.Count; - other.Array = nullptr; - other.Count = 0; - return *this; - } -}; - -template -class TStaticPointableArray : public TStaticArray -{ - bool pointed = false; -public: - - //////// - TStaticPointableArray() - { - } - // This is not supposed to be copyable. - TStaticPointableArray(const TStaticArray &other) = delete; - - void Clear() - { - if (!pointed && this->Array) delete[] this->Array; - this->Count = 0; - this->Array = nullptr; - } - void Alloc(unsigned int amount) - { - // intentionally first deletes and then reallocates. - if (!pointed && this->Array) delete[] this->Array; - this->Array = new T[amount]; - this->Count = amount; - pointed = false; - } - void Point(TStaticArray & other) - { - if (!pointed && this->Array) delete[] this->Array; - this->Array = other.Array; - this->Count = other.Count; - pointed = true; - } - TStaticPointableArray &operator=(TStaticArray &&other) - { - if (!pointed && this->Array) delete[] this->Array; - this->Array = other.Array; - this->Count = other.Count; - other.Array = nullptr; - other.Count = 0; - pointed = false; - return *this; - } -}; - // TAutoGrowArray ----------------------------------------------------------- // An array with accessors that automatically grow the array as needed. // It can still be used as a normal TArray if needed. ACS uses this for