From 6665dd4912e08fc55b84b38b1a2b8e519f055d5b Mon Sep 17 00:00:00 2001 From: Krisztiaan Date: Fri, 28 Aug 2026 00:28:59 +0200 Subject: [PATCH 1/4] Fix demand-loaded building artwork ownership --- code/builtype.cpp | 43 ++++++++++++------- .../demand-loaded-building-art-ownership.md | 31 +++++++++++++ manual/content/formats/mix.md | 2 +- .../content/keys/demandload--buildingtype.md | 8 +--- manual/content/keys/freebuildup.md | 6 +-- 5 files changed, 63 insertions(+), 27 deletions(-) create mode 100644 manual/changes/demand-loaded-building-art-ownership.md diff --git a/code/builtype.cpp b/code/builtype.cpp index 9bb422b3..d6c095c9 100644 --- a/code/builtype.cpp +++ b/code/builtype.cpp @@ -98,6 +98,12 @@ void const * BuildingTypeClass::WrenchShapes; BSurface * CloakingSurface; +static void Free_Demand_Loaded_Shape(void const *& data) +{ + delete [] (char *)data; + data = NULL; +} + Cell const BuildingTypeClass::OccupyLists[BSIZE_COUNT][24] = { /* BSIZE_11, */ { Cell(0,0),REFRESH_EOL }, /* BSIZE_21, */ { Cell(0,0),Cell(1,0),REFRESH_EOL }, @@ -366,12 +372,10 @@ BuildingTypeClass::BuildingTypeClass(char const * ininame) : BuildingTypeClass::~BuildingTypeClass(void) { if (IsDemandLoad && ImageData != NULL) { - delete (ShapeSet *)ImageData; - ImageData = NULL; + Free_Demand_Loaded_Shape(ImageData); } if (IsDemandLoadBuildup && BuildupData != NULL) { - delete (ShapeSet *)BuildupData; - BuildupData = NULL; + Free_Demand_Loaded_Shape(BuildupData); } Detach_This_From_All(this, true); BuildingTypes.Delete(this); @@ -603,8 +607,7 @@ void BuildingTypeClass::Init(TheaterType theater) classptr->ImageData = MFCD::Retrieve(fullname); } else { if (classptr->ImageData != NULL) { - delete (ShapeSet *)classptr->ImageData; - classptr->ImageData = NULL; + Free_Demand_Loaded_Shape(classptr->ImageData); } } @@ -617,8 +620,7 @@ void BuildingTypeClass::Init(TheaterType theater) classptr->BuildupData = MFCD::Retrieve(fullname); } else { if (classptr->BuildupData != NULL) { - delete (ShapeSet *)classptr->BuildupData; - classptr->BuildupData = NULL; + Free_Demand_Loaded_Shape(classptr->BuildupData); } } @@ -633,14 +635,12 @@ void BuildingTypeClass::Init(TheaterType theater) } else if (classptr->IsNewTheater) { if (classptr->IsDemandLoad) { if (classptr->ImageData != NULL) { - delete (ShapeSet *)classptr->ImageData; - classptr->ImageData = NULL; + Free_Demand_Loaded_Shape(classptr->ImageData); } } if (classptr->IsDemandLoadBuildup) { if (classptr->BuildupData != NULL) { - delete (ShapeSet *)classptr->BuildupData; - classptr->BuildupData = NULL; + Free_Demand_Loaded_Shape(classptr->BuildupData); } } classptr->Fetch_Building_Normal_Image(theater); @@ -1131,6 +1131,13 @@ bool BuildingTypeClass::Read_INI(CCINIClass const & ini) { char buffer[128]; + if (IsDemandLoad && ImageData != NULL) { + Free_Demand_Loaded_Shape(ImageData); + } + if (IsDemandLoadBuildup && BuildupData != NULL) { + Free_Demand_Loaded_Shape(BuildupData); + } + if (BASECLASS::Read_INI(ini)) { HasSpotlight = ini.Get_Bool(Name(), "HasSpotlight", HasSpotlight); @@ -1267,6 +1274,9 @@ bool BuildingTypeClass::Read_INI(CCINIClass const & ini) IsDemandLoad = ArtINI.Get_Bool(Graphic_Name(), "DemandLoad", IsDemandLoad); IsDemandLoadBuildup = ArtINI.Get_Bool(Graphic_Name(), "DemandLoadBuildup", IsDemandLoadBuildup); IsFreeBuildup = ArtINI.Get_Bool(Graphic_Name(), "FreeBuildup", IsFreeBuildup); + if (IsDemandLoad) { + ImageData = NULL; + } OccupyList = OccupyLists[Size]; ExitList = ExitLists[Size]; @@ -1722,7 +1732,9 @@ void BuildingTypeClass::Post_Load(void) BASECLASS::Post_Load(); Fetch_Building_Voxel_Image(); - Fetch_Normal_Image(); + if (!IsDemandLoad) { + Fetch_Normal_Image(); + } ToTile = NULL; OccupyList = OccupyLists[Size]; @@ -2047,10 +2059,9 @@ void const * BuildingTypeClass::Get_Buildup_Data(void) const /// void BuildingTypeClass::Free_Buildup_Data(void) { - if (IsFreeBuildup) { + if (IsFreeBuildup && IsDemandLoadBuildup) { if (BuildupData != NULL) { - delete (ShapeSet *)BuildupData; - BuildupData = NULL; + Free_Demand_Loaded_Shape(BuildupData); } } } diff --git a/manual/changes/demand-loaded-building-art-ownership.md b/manual/changes/demand-loaded-building-art-ownership.md new file mode 100644 index 00000000..de1d5a3d --- /dev/null +++ b/manual/changes/demand-loaded-building-art-ownership.md @@ -0,0 +1,31 @@ +--- +title: Keep demand-loaded structure artwork under its owner's lifetime +category: fix +release: 0.1.0 +targets: +- type: key + id: DemandLoad + scope: buildingtype + effect: changed +- type: key + id: DemandLoadBuildup + effect: changed +- type: key + id: FreeBuildup + effect: changed +--- + +A structure type with `DemandLoad=yes` now detaches the archive-owned shape found before +that setting is read. The type loads its own copy when the shape is first needed and +releases only that copy when the theater changes or the type is destroyed. A theater-aware +structure previously tried to release the archive's shared memory during theater setup, +which could stop a skirmish before play began. + +Demand-loaded structure shapes and construction animations are now released as the byte +arrays that the file loader allocated. Their former scalar release did not match that +allocation and could corrupt the heap during theater changes, construction-art cleanup or +shutdown. + +`FreeBuildup=yes` now releases construction artwork only when `DemandLoadBuildup=yes` gave +the structure type its own copy. On its own the setting leaves archive-owned artwork in +place instead of freeing shared memory and leaving later construction animations empty. diff --git a/manual/content/formats/mix.md b/manual/content/formats/mix.md index 34edbe54..1a148416 100644 --- a/manual/content/formats/mix.md +++ b/manual/content/formats/mix.md @@ -32,7 +32,7 @@ Whether an archive is cached decides how its members can be reached: - A member of a cached archive can be handed out as a pointer straight into the memory the archive is already holding. Nothing is allocated for the member and nothing is copied. Shapes, fonts, palettes and sound samples are fetched this way, so those files have to live in an archive that was cached — a loose file, or a member of an archive that was mounted without being cached, is not found by that path at all. - Opening a member as a file works either way. From a cached archive the file object becomes a window onto that same memory and a read copies out of it; from an archive that is not cached the archive file itself is opened and every read is biased to the member's position within it. -The memory belongs to the archive rather than to whatever asked for the member, so a pointer fetched the first way stays good only while that archive stays cached and mounted, and handing it back is not something its holder may do. [`FreeBuildup`](/keys/freebuildup/) describes what a type that hands one back costs the rest of the game. +The memory belongs to the archive rather than to whatever asked for the member, so a pointer fetched the first way stays good only while that archive stays cached and mounted, and handing it back is not something its holder may do. [`FreeBuildup`](/keys/freebuildup/) releases only a separately loaded copy of construction artwork. :::danger[An archive too short to hold a header is mounted from uninitialized memory] The number of members and the size of the data block are taken from the first bytes of the file without testing that any bytes were read. An archive that cannot supply them — an empty file most obviously — is mounted with a member count and an index taken from whatever that memory last held. Allocating an index for an implausible count is not survivable; where the count is small enough to allocate, the archive joins the search carrying meaningless entries, and a request that matches one is handed an offset and a size that describe no file. diff --git a/manual/content/keys/demandload--buildingtype.md b/manual/content/keys/demandload--buildingtype.md index d9d1c254..bf758dfc 100644 --- a/manual/content/keys/demandload--buildingtype.md +++ b/manual/content/keys/demandload--buildingtype.md @@ -8,12 +8,8 @@ when_omitted: value: "no" --- -A structure's shape is fetched from the archives twice while the rules are read: once under its [Image ID](/keys/image/), before this setting has been read at all, and once afterwards under the [main-shape basename](/keys/image/#scope-buildingtype). Only the second fetch is skipped. A structure whose two names agree — the ordinary case — is therefore already holding its artwork by the time the flag is consulted, and setting it changes nothing about when the shape is loaded. Deferral takes effect only where the first name resolves to no file. +A structure's shape is first found in the archives under its [Image ID](/keys/image/), before this setting has been read. With the flag set, the structure type detaches that archive-owned shape without releasing it, records the [main-shape basename](/keys/image/#scope-buildingtype) for the current theater, and leaves its own shape empty. -Where it does take effect, the shape is read from disk the first time something asks to draw a structure of the type, and is then held for the rest of the session. A type nothing ever draws allocates nothing, and a structure whose shape cannot be found is not drawn at all. +The shape is read from disk the first time something asks to draw a structure of the type, and is held until the theater changes or the type is destroyed. A type nothing ever draws allocates nothing, and a structure whose shape cannot be found is not drawn at all. The construction animation is a separate setting, [`DemandLoadBuildup`](/keys/demandloadbuildup/). The deploying, door, under-door, bib and Z-shape overlay artwork is fetched with the rules whatever this is set to. - -:::danger[The release hands back memory the type may never have allocated] -A type carrying this flag releases whatever its shape pointer holds when the type is destroyed, and again as the theater is set up if the type is also [`Theater=yes`](/keys/theater/) or [`NewTheater=yes`](/keys/newtheater/). Only a shape this flag actually deferred is a block the type allocated for itself; the pointer left by the earlier fetch belongs to the archive it was read from, and handing that back corrupts the heap: the game may fault there or at a later, unrelated allocation. -::: diff --git a/manual/content/keys/freebuildup.md b/manual/content/keys/freebuildup.md index d4df3af2..6da106d3 100644 --- a/manual/content/keys/freebuildup.md +++ b/manual/content/keys/freebuildup.md @@ -7,8 +7,6 @@ when_omitted: value: "no" --- -With the flag set, the construction artwork is released at three moments: as each structure of the type is created and has been asked whether it may ever be sold, as a structure of the type finishes its buildup and opens, and as a structure of the type is taken off the map. Only [`DemandLoadBuildup=yes`](/keys/demandloadbuildup/) fetches it again afterwards. +With this flag and [`DemandLoadBuildup=yes`](/keys/demandloadbuildup/) both set, the construction artwork is released at three moments: as each structure of the type is created and has been asked whether it may ever be sold, as a structure of the type finishes its buildup and opens, and as a structure of the type is taken off the map. The next structure that needs it loads it again. -:::danger[On its own the release hands back memory the type never allocated] -Without `DemandLoadBuildup=yes` the artwork was read straight out of the archive that holds it, and what is released is a pointer into that archive's own block. Handing it back corrupts the heap, and the game may fault there or at a later, unrelated allocation. Nothing reloads the artwork either, so every structure of the type is drawn as nothing while it builds, and every one after the first is marked unsellable as well. Set the two together or leave both off. -::: +Without `DemandLoadBuildup=yes`, this flag has no effect. The construction artwork remains attached to its archive and is not released or reloaded. From c4d0bc3bd98da080856af3af548007f3b43a80a9 Mon Sep 17 00:00:00 2001 From: Krisztiaan Date: Fri, 28 Aug 2026 15:30:01 +0200 Subject: [PATCH 2/4] Credit Krisztiaan for demand-load fix --- manual/changes/demand-loaded-building-art-ownership.md | 1 + 1 file changed, 1 insertion(+) diff --git a/manual/changes/demand-loaded-building-art-ownership.md b/manual/changes/demand-loaded-building-art-ownership.md index de1d5a3d..0cc330ca 100644 --- a/manual/changes/demand-loaded-building-art-ownership.md +++ b/manual/changes/demand-loaded-building-art-ownership.md @@ -13,6 +13,7 @@ targets: - type: key id: FreeBuildup effect: changed +credit: [Krisztiaan] --- A structure type with `DemandLoad=yes` now detaches the archive-owned shape found before From d846252ff6d022584d29c291a4661974683f2b61 Mon Sep 17 00:00:00 2001 From: Krisztiaan Date: Sat, 29 Aug 2026 13:16:35 +0200 Subject: [PATCH 3/4] Fix demand-loaded animation and overlay ownership --- code/animtype.cpp | 28 ++++++++++++++----- code/builtype.cpp | 3 ++ code/overtype.cpp | 28 ++++++++++++++----- .../demand-loaded-building-art-ownership.md | 22 +++++++++++++-- manual/content/formats/mix.md | 2 +- manual/content/keys/demandload--animtype.md | 8 ++---- .../content/keys/demandload--overlaytype.md | 10 ++----- manual/content/keys/freeafterplaying.md | 2 +- 8 files changed, 71 insertions(+), 32 deletions(-) diff --git a/code/animtype.cpp b/code/animtype.cpp index 3907c68c..b4ce05c6 100644 --- a/code/animtype.cpp +++ b/code/animtype.cpp @@ -64,6 +64,16 @@ #include "warhead.h" +/// +/// Releases shape data that this type loaded through the file layer. +/// +static void Free_Demand_Loaded_Shape(void const *& data) +{ + delete [] (char *)data; + data = NULL; +} + + /*********************************************************************************************** * AnimTypeClass::AnimTypeClass -- Constructor for animation types. * * * @@ -172,8 +182,7 @@ AnimTypeClass::AnimTypeClass(char const *ininame) : AnimTypeClass::~AnimTypeClass(void) { if (IsDemandLoad && ImageData) { - delete [] (char*) ImageData; - ImageData = NULL; + Free_Demand_Loaded_Shape(ImageData); } AbstractTypePtrTracker.Delete(this); @@ -219,8 +228,7 @@ void AnimTypeClass::Init(TheaterType theater) } else { if (anim->IsTheater || anim->IsNewTheater) { if (anim->ImageData != NULL) { - delete [] (char*) anim->ImageData; - anim->ImageData = NULL; + Free_Demand_Loaded_Shape(anim->ImageData); } } } @@ -358,6 +366,10 @@ AnimType AnimTypeClass::From_Name(char const * name) /// bool; Was the animation type's data read? bool AnimTypeClass::Read_INI(CCINIClass const & ini) { + if (IsDemandLoad && ImageData != NULL) { + Free_Demand_Loaded_Shape(ImageData); + } + if (BASECLASS::Read_INI(ini)) { if (!GraphicName.empty()) { if (ImageData == NULL) { @@ -407,6 +419,9 @@ bool AnimTypeClass::Read_INI(CCINIClass const & ini) IsDemandLoad = ini.Get_Bool(Name(), "DemandLoad", IsDemandLoad); IsFreeAfterPlaying = ini.Get_Bool(Name(), "FreeAfterPlaying", IsFreeAfterPlaying); + if (IsDemandLoad) { + ImageData = NULL; + } Elasticity = ini.Get_Float(Name(), "Elasticity", Elasticity); MaxXYVel = ini.Get_Float(Name(), "MaxXYVel", MaxXYVel); @@ -472,9 +487,9 @@ void AnimTypeClass::Post_Load(void) BASECLASS::Post_Load(); Fetch_Voxel_Image(); - Fetch_Normal_Image(); if (!IsDemandLoad) { + Fetch_Normal_Image(); if (IsTheater) { char fullname[_MAX_FNAME+_MAX_EXT]; // Fully constructed iconset name. _makepath(fullname, NULL, NULL, Name(), Theaters[Scen->Theater].Suffix); @@ -681,7 +696,6 @@ void AnimTypeClass::Free_Image(void) { if (IsDemandLoad && ImageData != NULL && IsFreeAfterPlaying) { DebugString("Freeing loaded image for %s\n", Full_Name()); - delete [] (char*) ImageData; - ImageData = NULL; + Free_Demand_Loaded_Shape(ImageData); } } diff --git a/code/builtype.cpp b/code/builtype.cpp index d6c095c9..4a73a9ac 100644 --- a/code/builtype.cpp +++ b/code/builtype.cpp @@ -98,6 +98,9 @@ void const * BuildingTypeClass::WrenchShapes; BSurface * CloakingSurface; +/// +/// Releases shape data that this type loaded through the file layer. +/// static void Free_Demand_Loaded_Shape(void const *& data) { delete [] (char *)data; diff --git a/code/overtype.cpp b/code/overtype.cpp index 931fa566..d41cc30c 100644 --- a/code/overtype.cpp +++ b/code/overtype.cpp @@ -77,6 +77,16 @@ #include "tracker.h" +/// +/// Releases shape data that this type loaded through the file layer. +/// +static void Free_Demand_Loaded_Shape(void const *& data) +{ + delete [] (char *)data; + data = NULL; +} + + /*********************************************************************************************** * OverlayTypeClass::OverlayTypeClass -- Constructor for overlay type objects. * * * @@ -133,8 +143,7 @@ OverlayTypeClass::OverlayTypeClass(char const * ininame) : OverlayTypeClass::~OverlayTypeClass(void) { if (DemandLoad && ImageData != NULL) { - delete (ShapeSet *)ImageData; - ImageData = NULL; + Free_Demand_Loaded_Shape(ImageData); } Detach_This_From_All(this, true); OverlayTypes.Delete(this); @@ -301,8 +310,7 @@ void OverlayTypeClass::Init(TheaterType theater) } else { if (overlay.IsTheater || overlay.IsNewTheater) { if (overlay.ImageData != NULL) { - delete [] (char*) overlay.ImageData; - overlay.ImageData = NULL; + Free_Demand_Loaded_Shape(overlay.ImageData); } } } @@ -321,6 +329,10 @@ bool OverlayTypeClass::Read_INI(CCINIClass const & ini) { char fullname[_MAX_FNAME+_MAX_EXT]; + if (DemandLoad && ImageData != NULL) { + Free_Demand_Loaded_Shape(ImageData); + } + if (BASECLASS::Read_INI(ini)) { Land = ini.Get_LandType(IniName, "Land", Land); DamagePoints = ini.Get_Int(IniName, "Strength", DamagePoints); @@ -336,6 +348,9 @@ bool OverlayTypeClass::Read_INI(CCINIClass const & ini) DamageLevels = ArtINI.Get_Int(GraphicName, "DamageLevels", DamageLevels); DemandLoad = ArtINI.Get_Bool(GraphicName, "DemandLoad", DemandLoad); + if (DemandLoad) { + ImageData = NULL; + } if (IsTiberium) { Armor = ARMOR_WOOD; @@ -417,9 +432,9 @@ void OverlayTypeClass::Post_Load(void) BASECLASS::Post_Load(); Fetch_Voxel_Image(); - Fetch_Normal_Image(); if (!DemandLoad) { + Fetch_Normal_Image(); char fullname[_MAX_FNAME+_MAX_EXT]; if (IsTheater) { _makepath(fullname, NULL, NULL, GraphicName, Theaters[Scen->Theater].Suffix); @@ -546,10 +561,9 @@ void const * OverlayTypeClass::Get_Image_Data(void) const DebugString("Demand loading image for %s\n", (char const *)GivenName); if (IsTheater) { _makepath(fullname, NULL, NULL, GraphicName, Theaters[Scen->Theater].Suffix); - } else { + _makepath(fullname, NULL, NULL, GraphicName, ".SHP"); if (IsNewTheater) { - _makepath(fullname, NULL, NULL, GraphicName, ".SHP"); _this->Theater_Naming_Convention( fullname, Scen->Theater); } } diff --git a/manual/changes/demand-loaded-building-art-ownership.md b/manual/changes/demand-loaded-building-art-ownership.md index 0cc330ca..b007818e 100644 --- a/manual/changes/demand-loaded-building-art-ownership.md +++ b/manual/changes/demand-loaded-building-art-ownership.md @@ -1,19 +1,27 @@ --- -title: Keep demand-loaded structure artwork under its owner's lifetime +title: Keep demand-loaded artwork under its owner's lifetime category: fix -release: 0.1.0 +release: 0.2.0 targets: - type: key id: DemandLoad scope: buildingtype effect: changed +- type: key + id: DemandLoad + scope: animtype + effect: changed +- type: key + id: DemandLoad + scope: overlaytype + effect: changed - type: key id: DemandLoadBuildup effect: changed - type: key id: FreeBuildup effect: changed -credit: [Krisztiaan] +credit: [Krisztiaan, ZivDero] --- A structure type with `DemandLoad=yes` now detaches the archive-owned shape found before @@ -30,3 +38,11 @@ shutdown. `FreeBuildup=yes` now releases construction artwork only when `DemandLoadBuildup=yes` gave the structure type its own copy. On its own the setting leaves archive-owned artwork in place instead of freeing shared memory and leaving later construction animations empty. + +Demand-loaded animations and overlays now detach their archive-owned shapes after reading +their settings and after restoring a saved game. They release only the separate copies +loaded when first drawn, rather than handing archive memory back during theater changes or +shutdown. + +An ordinary demand-loaded overlay now builds its deferred filename from its Image ID. Its +first draw loads the `.SHP` instead of reading through an uninitialized filename. diff --git a/manual/content/formats/mix.md b/manual/content/formats/mix.md index 1a148416..b5e846f0 100644 --- a/manual/content/formats/mix.md +++ b/manual/content/formats/mix.md @@ -32,7 +32,7 @@ Whether an archive is cached decides how its members can be reached: - A member of a cached archive can be handed out as a pointer straight into the memory the archive is already holding. Nothing is allocated for the member and nothing is copied. Shapes, fonts, palettes and sound samples are fetched this way, so those files have to live in an archive that was cached — a loose file, or a member of an archive that was mounted without being cached, is not found by that path at all. - Opening a member as a file works either way. From a cached archive the file object becomes a window onto that same memory and a read copies out of it; from an archive that is not cached the archive file itself is opened and every read is biased to the member's position within it. -The memory belongs to the archive rather than to whatever asked for the member, so a pointer fetched the first way stays good only while that archive stays cached and mounted, and handing it back is not something its holder may do. [`FreeBuildup`](/keys/freebuildup/) releases only a separately loaded copy of construction artwork. +The memory belongs to the archive rather than to whatever asked for the member, so a pointer fetched the first way stays good only while that archive stays cached and mounted, and handing it back is not something its holder may do. Demand-loaded structure, animation, overlay and construction shapes release only the separate copies they read through the file layer. :::danger[An archive too short to hold a header is mounted from uninitialized memory] The number of members and the size of the data block are taken from the first bytes of the file without testing that any bytes were read. An archive that cannot supply them — an empty file most obviously — is mounted with a member count and an index taken from whatever that memory last held. Allocating an index for an implausible count is not survivable; where the count is small enough to allocate, the archive joins the search carrying meaningless entries, and a request that matches one is handed an offset and a size that describe no file. diff --git a/manual/content/keys/demandload--animtype.md b/manual/content/keys/demandload--animtype.md index 1efbe056..28e0011b 100644 --- a/manual/content/keys/demandload--animtype.md +++ b/manual/content/keys/demandload--animtype.md @@ -8,12 +8,8 @@ when_omitted: value: "no" --- -The flag is read after the animation's shape has already been fetched from the archives under its [Image ID](/keys/image/#scope-animtype), so an animation whose artwork is present is holding it before the setting is consulted and nothing is deferred. What the flag then skips is the later work: the shape is not fetched again when the theater is set up, and the theater's own copy is not fetched again when a saved game is restored — though the restore does re-attach the archive's copy, which puts the release below back in reach. +The flag is read after the animation's shape has already been found in the archives under its [Image ID](/keys/image/#scope-animtype). With the flag set, the animation type detaches that archive-owned shape without releasing it and leaves its own shape empty. Restoring a saved game also leaves the shape empty rather than attaching the archive's copy. An animation that reaches a draw with no shape reads one from disk at that moment, and the frame count and loop end the type left unset are taken from it then rather than at load time. The name built for that read is the Image ID if the animation has one and the AnimType ID otherwise, with a `.SHP` extension; a [`Theater=yes`](/keys/theater/#scope-animtype) animation instead uses the AnimType ID with the theater's own extension, dropping the Image ID, and a [`NewTheater=yes`](/keys/newtheater/#scope-animtype) one has the built name rewritten for the theater. -On a `Theater=yes` or `NewTheater=yes` animation the shape the type is holding is released as the theater is set up rather than replaced. It is released with the type as well, and on a [`FreeAfterPlaying=yes`](/keys/freeafterplaying/) animation as soon as the animation finishes playing — the only combination that gives artwork back during a match. - -:::danger[The release hands back memory the type may never have allocated] -Both releases run on whatever the shape pointer holds. Artwork the animation fetched for itself is a block it may give back; the pointer left by the earlier archive fetch is not, and handing that back corrupts the heap: the game may fault there or at a later, unrelated allocation. -::: +On a `Theater=yes` or `NewTheater=yes` animation, the separately loaded shape is released when the theater changes. It is also released with the type, and on a [`FreeAfterPlaying=yes`](/keys/freeafterplaying/) animation as soon as the animation finishes playing — the only combination that gives artwork back during a match. diff --git a/manual/content/keys/demandload--overlaytype.md b/manual/content/keys/demandload--overlaytype.md index bc149ee7..911a5fa5 100644 --- a/manual/content/keys/demandload--overlaytype.md +++ b/manual/content/keys/demandload--overlaytype.md @@ -8,12 +8,8 @@ when_omitted: value: "no" --- -The flag is read after the overlay's shape has already been fetched from the archives under its [Image ID](/keys/image/), so an overlay whose artwork is present is holding it before the setting is consulted. What the flag then skips is the later work: the shape is not fetched again as the theater is set up, and the theater-named copy is not fetched again when a saved game is restored — though the restore does re-attach the archive's copy, which puts the release below back in reach. An overlay that reaches a draw with no shape reads one from disk at that moment and holds it for the rest of the session. +The flag is read after the overlay's shape has already been found in the archives under its [Image ID](/keys/image/). With the flag set, the overlay type detaches that archive-owned shape without releasing it and leaves its own shape empty. Restoring a saved game also leaves the shape empty rather than attaching the archive's copy. -:::caution[Only a theater-aware overlay can be demand loaded] -The deferred read builds its filename only for a [`Theater=yes`](/keys/theater/) or [`NewTheater=yes`](/keys/newtheater/) overlay. On any other overlay no name is built at all: the read is made against a name buffer nothing ever wrote, and the type is left with no artwork of its own to draw. -::: +An overlay that reaches a draw with no shape reads one from disk at that moment. An ordinary overlay uses its Image ID with a `.SHP` extension, a [`Theater=yes`](/keys/theater/) overlay uses the theater's extension, and a [`NewTheater=yes`](/keys/newtheater/) overlay rewrites the ordinary name for the current theater. -:::danger[The release hands back memory the type may never have allocated] -The type releases whatever its shape pointer holds when it is destroyed, and again as the theater is set up if it is [`Theater=yes`](/keys/theater/) or [`NewTheater=yes`](/keys/newtheater/). Only a shape this flag actually deferred is a block the type allocated for itself; the pointer left by the earlier fetch belongs to the archive it was read from, and handing that back corrupts the heap: the game may fault there or at a later, unrelated allocation. -::: +The separately loaded shape is held until the type is destroyed. A theater-aware overlay also releases it when the theater changes so the next draw loads the matching copy. diff --git a/manual/content/keys/freeafterplaying.md b/manual/content/keys/freeafterplaying.md index 85856321..c5744914 100644 --- a/manual/content/keys/freeafterplaying.md +++ b/manual/content/keys/freeafterplaying.md @@ -13,4 +13,4 @@ Where it does apply, the artwork is released as the animation object is destroye An animation that has chained through [`Next=`](/keys/next/) releases the artwork of the type it was holding when it was destroyed, not the type it started as, so the earlier types in a chain keep theirs for the rest of the scenario. -[Animation shape](/keys/demandload/#scope-animtype) covers the release itself, including what happens when the artwork the type gives back was never the type's to give. +[Animation shape](/keys/demandload/#scope-animtype) covers how the shape is attached, loaded and released. From 3752dc360482309ff35f569df1cec89f5eab389c Mon Sep 17 00:00:00 2001 From: Krisztiaan Date: Sat, 29 Aug 2026 16:11:02 +0200 Subject: [PATCH 4/4] Address demand-load ownership review --- code/animtype.cpp | 4 ++++ code/builtype.cpp | 11 ++++++++++- code/overtype.cpp | 4 ++++ .../demand-loaded-building-art-ownership.md | 14 ++++++++------ manual/content/keys/demandload--buildingtype.md | 2 +- manual/content/keys/demandloadbuildup.md | 2 +- manual/content/keys/freebuildup.md | 4 ++-- 7 files changed, 30 insertions(+), 11 deletions(-) diff --git a/code/animtype.cpp b/code/animtype.cpp index b4ce05c6..14a418ee 100644 --- a/code/animtype.cpp +++ b/code/animtype.cpp @@ -366,6 +366,10 @@ AnimType AnimTypeClass::From_Name(char const * name) /// bool; Was the animation type's data read? bool AnimTypeClass::Read_INI(CCINIClass const & ini) { + if (!ini.Section_Present(IniName)) { + return(false); + } + if (IsDemandLoad && ImageData != NULL) { Free_Demand_Loaded_Shape(ImageData); } diff --git a/code/builtype.cpp b/code/builtype.cpp index 4a73a9ac..9cf0a21b 100644 --- a/code/builtype.cpp +++ b/code/builtype.cpp @@ -1134,6 +1134,10 @@ bool BuildingTypeClass::Read_INI(CCINIClass const & ini) { char buffer[128]; + if (!ini.Section_Present(IniName)) { + return(false); + } + if (IsDemandLoad && ImageData != NULL) { Free_Demand_Loaded_Shape(ImageData); } @@ -1280,6 +1284,9 @@ bool BuildingTypeClass::Read_INI(CCINIClass const & ini) if (IsDemandLoad) { ImageData = NULL; } + if (IsDemandLoadBuildup) { + BuildupData = NULL; + } OccupyList = OccupyLists[Size]; ExitList = ExitLists[Size]; @@ -1735,7 +1742,9 @@ void BuildingTypeClass::Post_Load(void) BASECLASS::Post_Load(); Fetch_Building_Voxel_Image(); - if (!IsDemandLoad) { + if (IsDemandLoad) { + ImageData = NULL; + } else { Fetch_Normal_Image(); } diff --git a/code/overtype.cpp b/code/overtype.cpp index d41cc30c..69a9cfc7 100644 --- a/code/overtype.cpp +++ b/code/overtype.cpp @@ -329,6 +329,10 @@ bool OverlayTypeClass::Read_INI(CCINIClass const & ini) { char fullname[_MAX_FNAME+_MAX_EXT]; + if (!ini.Section_Present(IniName)) { + return(false); + } + if (DemandLoad && ImageData != NULL) { Free_Demand_Loaded_Shape(ImageData); } diff --git a/manual/changes/demand-loaded-building-art-ownership.md b/manual/changes/demand-loaded-building-art-ownership.md index b007818e..a5ffd1e6 100644 --- a/manual/changes/demand-loaded-building-art-ownership.md +++ b/manual/changes/demand-loaded-building-art-ownership.md @@ -24,11 +24,11 @@ targets: credit: [Krisztiaan, ZivDero] --- -A structure type with `DemandLoad=yes` now detaches the archive-owned shape found before -that setting is read. The type loads its own copy when the shape is first needed and -releases only that copy when the theater changes or the type is destroyed. A theater-aware -structure previously tried to release the archive's shared memory during theater setup, -which could stop a skirmish before play began. +A structure type with `DemandLoad=yes` now detaches the archive-owned shape found while +reading its rules or restoring a saved game. The type loads its own copy when the shape is +first needed and releases only that copy. A theater-aware structure previously tried to +release the archive's shared memory during theater setup, which could stop a skirmish before +play began. Demand-loaded structure shapes and construction animations are now released as the byte arrays that the file loader allocated. Their former scalar release did not match that @@ -37,7 +37,9 @@ shutdown. `FreeBuildup=yes` now releases construction artwork only when `DemandLoadBuildup=yes` gave the structure type its own copy. On its own the setting leaves archive-owned artwork in -place instead of freeing shared memory and leaving later construction animations empty. +place instead of freeing shared memory. Later structures retain their construction and +deconstruction animation and sellability, and their nominal crew become technicians when +the structure is destroyed. Demand-loaded animations and overlays now detach their archive-owned shapes after reading their settings and after restoring a saved game. They release only the separate copies diff --git a/manual/content/keys/demandload--buildingtype.md b/manual/content/keys/demandload--buildingtype.md index bf758dfc..b7994bc9 100644 --- a/manual/content/keys/demandload--buildingtype.md +++ b/manual/content/keys/demandload--buildingtype.md @@ -10,6 +10,6 @@ when_omitted: A structure's shape is first found in the archives under its [Image ID](/keys/image/), before this setting has been read. With the flag set, the structure type detaches that archive-owned shape without releasing it, records the [main-shape basename](/keys/image/#scope-buildingtype) for the current theater, and leaves its own shape empty. -The shape is read from disk the first time something asks to draw a structure of the type, and is held until the theater changes or the type is destroyed. A type nothing ever draws allocates nothing, and a structure whose shape cannot be found is not drawn at all. +The shape is read from disk the first time something asks to draw a structure of the type. It is released when that type's rules section is read again, when theater setup revisits a `Theater=yes` or `NewTheater=yes` type, or when the type is destroyed. A type nothing ever draws allocates nothing, and a structure whose shape cannot be found is not drawn at all. The construction animation is a separate setting, [`DemandLoadBuildup`](/keys/demandloadbuildup/). The deploying, door, under-door, bib and Z-shape overlay artwork is fetched with the rules whatever this is set to. diff --git a/manual/content/keys/demandloadbuildup.md b/manual/content/keys/demandloadbuildup.md index 27669cca..00a13d5b 100644 --- a/manual/content/keys/demandloadbuildup.md +++ b/manual/content/keys/demandloadbuildup.md @@ -11,4 +11,4 @@ By default the file [`Buildup=`](/keys/buildup/) names is fetched from the archi The deferred read builds the filename with a `.SHP` extension and the structure-art theater rewrite whatever the type's own theater settings say, so a `Theater=yes` structure that defers its construction artwork looks for the plain file rather than the theater-suffixed one. It gives the step count and the rate their ordinary treatment — half the frames in the file, divided into [`BuildupTime`](/keys/builduptime/) — where the theater fetch does not. -This is also the setting that makes [`FreeBuildup=yes`](/keys/freebuildup/) safe to use, since only artwork read this way is a block the type may give back. +Only artwork read this way is released by [`FreeBuildup=yes`](/keys/freebuildup/). Without `DemandLoadBuildup=yes`, `FreeBuildup=yes` has no effect. diff --git a/manual/content/keys/freebuildup.md b/manual/content/keys/freebuildup.md index 6da106d3..e880649f 100644 --- a/manual/content/keys/freebuildup.md +++ b/manual/content/keys/freebuildup.md @@ -7,6 +7,6 @@ when_omitted: value: "no" --- -With this flag and [`DemandLoadBuildup=yes`](/keys/demandloadbuildup/) both set, the construction artwork is released at three moments: as each structure of the type is created and has been asked whether it may ever be sold, as a structure of the type finishes its buildup and opens, and as a structure of the type is taken off the map. The next structure that needs it loads it again. +With this flag and [`DemandLoadBuildup=yes`](/keys/demandloadbuildup/) both set, construction artwork is released after it is used to measure a type's draw area, as each structure of the type is created and checked for sellability, when a structure finishes its buildup, and when a structure is destroyed. The next request loads it again. -Without `DemandLoadBuildup=yes`, this flag has no effect. The construction artwork remains attached to its archive and is not released or reloaded. +Without `DemandLoadBuildup=yes`, this flag has no effect. The construction artwork remains attached to its archive and is not released or reloaded. This fixes prior OpenTS behavior that stripped the shared artwork after the first structure: later structures now retain their construction and deconstruction animation, remain sellable, and count as having construction artwork when selecting survivors, including conversion of nominal crew to technicians.