From 650449bc2782db4b1dfc69f5ddd6ba6d936bed90 Mon Sep 17 00:00:00 2001 From: Pavel Solodovnikov Date: Sat, 20 Jan 2024 14:28:44 +0300 Subject: [PATCH 1/9] Move `mutating_list_iterate` to framework lib So that the utility is accessible in base libraries Signed-off-by: Pavel Solodovnikov --- lib/framework/object_list_iteration.h | 56 +++++++++++++++++++++++++++ src/droid.cpp | 1 + src/intdisplay.cpp | 1 + src/intelmap.cpp | 1 + src/keybind.cpp | 1 + src/loop.cpp | 1 + src/message.cpp | 1 + src/mission.cpp | 1 + src/multijoin.cpp | 1 + src/multilimit.cpp | 1 + src/multiplay.cpp | 1 + src/objmem.h | 32 +-------------- src/order.cpp | 1 + 13 files changed, 69 insertions(+), 30 deletions(-) create mode 100644 lib/framework/object_list_iteration.h diff --git a/lib/framework/object_list_iteration.h b/lib/framework/object_list_iteration.h new file mode 100644 index 00000000000..e505b560bcd --- /dev/null +++ b/lib/framework/object_list_iteration.h @@ -0,0 +1,56 @@ +/* + This file is part of Warzone 2100. + Copyright (C) 2024 Warzone 2100 Project + + Warzone 2100 is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + Warzone 2100 is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with Warzone 2100; if not, write to the Free Software + Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +*/ +/** @file + * Basic routines for list iteration of various in-game objects (resources, sounds, droids, etc.). + */ +#pragma once + +#include +#include +#include + +enum class IterationResult +{ + BREAK_ITERATION, + CONTINUE_ITERATION +}; + +// Common iteration helper for lists of game objects +// with an ability to execute loop body handlers which can +// possibly invalidate the any iterator in the range `[begin(), currentIterator]`. +template +void mutating_list_iterate(std::list& list, MaybeErasingLoopBodyHandler handler) +{ + static_assert(std::is_same, IterationResult>::value, + "Loop body handler should return IterationResult"); + + typename std::remove_reference_t::iterator it = list.begin(), itNext; + while (it != list.end()) + { + itNext = std::next(it); + // Can possibly invalidate `it` and anything before it. + const auto res = handler(*it); + if (res == IterationResult::BREAK_ITERATION) + { + break; + } + it = itNext; + } +} + diff --git a/src/droid.cpp b/src/droid.cpp index 0f89b651cd7..81ec3a6564e 100644 --- a/src/droid.cpp +++ b/src/droid.cpp @@ -27,6 +27,7 @@ #include "lib/framework/math_ext.h" #include "lib/framework/geometry.h" #include "lib/framework/strres.h" +#include "lib/framework/object_list_iteration.h" #include "lib/gamelib/gtime.h" #include "lib/ivis_opengl/piematrix.h" diff --git a/src/intdisplay.cpp b/src/intdisplay.cpp index faab07ce398..54bf5a0929b 100644 --- a/src/intdisplay.cpp +++ b/src/intdisplay.cpp @@ -25,6 +25,7 @@ */ #include "lib/framework/frame.h" #include "lib/framework/math_ext.h" +#include "lib/framework/object_list_iteration.h" /* Includes direct access to render library */ #include "lib/ivis_opengl/ivisdef.h" diff --git a/src/intelmap.cpp b/src/intelmap.cpp index 219847483ec..f25e354463f 100644 --- a/src/intelmap.cpp +++ b/src/intelmap.cpp @@ -27,6 +27,7 @@ #include "lib/framework/frame.h" #include "lib/framework/strres.h" +#include "lib/framework/object_list_iteration.h" #include "lib/widget/widget.h" #include "lib/widget/gridlayout.h" #include "lib/widget/button.h" diff --git a/src/keybind.cpp b/src/keybind.cpp index 14983aff56e..3949be9a495 100644 --- a/src/keybind.cpp +++ b/src/keybind.cpp @@ -23,6 +23,7 @@ #include "lib/framework/frame.h" #include "lib/framework/wzapp.h" #include "lib/framework/rational.h" +#include "lib/framework/object_list_iteration.h" #include "lib/framework/physfs_ext.h" #include "objects.h" #include "levels.h" diff --git a/src/loop.cpp b/src/loop.cpp index 51e5dd0b3ad..c53f651aaaa 100644 --- a/src/loop.cpp +++ b/src/loop.cpp @@ -27,6 +27,7 @@ #include "lib/framework/input.h" #include "lib/framework/strres.h" #include "lib/framework/wzapp.h" +#include "lib/framework/object_list_iteration.h" #include "lib/ivis_opengl/pieblitfunc.h" #include "lib/ivis_opengl/piestate.h" //ivis render code diff --git a/src/message.cpp b/src/message.cpp index 29171e52453..5a27ff69e52 100644 --- a/src/message.cpp +++ b/src/message.cpp @@ -29,6 +29,7 @@ #include "lib/framework/wzconfig.h" #include "lib/framework/frameresource.h" #include "lib/framework/strres.h" +#include "lib/framework/object_list_iteration.h" #include "lib/sound/audio.h" #include "lib/sound/audio_id.h" #include "lib/ivis_opengl/imd.h" diff --git a/src/mission.cpp b/src/mission.cpp index e4521ffadd8..a05a3e3deae 100644 --- a/src/mission.cpp +++ b/src/mission.cpp @@ -29,6 +29,7 @@ #include "lib/framework/frame.h" #include "lib/framework/wzapp.h" #include "lib/framework/math_ext.h" +#include "lib/framework/object_list_iteration.h" #include "lib/framework/physfs_ext.h" #include "lib/ivis_opengl/bitimage.h" #include "lib/ivis_opengl/pieblitfunc.h" diff --git a/src/multijoin.cpp b/src/multijoin.cpp index 3b260c9cf52..4025a160a0b 100644 --- a/src/multijoin.cpp +++ b/src/multijoin.cpp @@ -30,6 +30,7 @@ #include "lib/framework/frame.h" #include "lib/framework/strres.h" #include "lib/framework/math_ext.h" +#include "lib/framework/object_list_iteration.h" #include "lib/gamelib/gtime.h" #include "lib/ivis_opengl/textdraw.h" diff --git a/src/multilimit.cpp b/src/multilimit.cpp index 89e027669c7..efe04522cf4 100644 --- a/src/multilimit.cpp +++ b/src/multilimit.cpp @@ -25,6 +25,7 @@ #include "lib/framework/frame.h" #include "lib/framework/frameresource.h" #include "lib/framework/strres.h" +#include "lib/framework/object_list_iteration.h" #include "lib/widget/slider.h" #include "lib/widget/widget.h" #include "hci.h" diff --git a/src/multiplay.cpp b/src/multiplay.cpp index c4f4fc3dcee..e286cdab23a 100644 --- a/src/multiplay.cpp +++ b/src/multiplay.cpp @@ -32,6 +32,7 @@ #include "lib/framework/input.h" #include "lib/framework/strres.h" #include "lib/framework/physfs_ext.h" +#include "lib/framework/object_list_iteration.h" #include "lib/ivis_opengl/piepalette.h" // for pal_Init() #include "map.h" diff --git a/src/objmem.h b/src/objmem.h index 75043137548..b0f29f6bff0 100644 --- a/src/objmem.h +++ b/src/objmem.h @@ -26,10 +26,11 @@ #include "objectdef.h" +//#include "lib/framework/object_list_iteration.h" + #include #include #include -#include /* The lists of objects allocated */ template @@ -155,33 +156,4 @@ void objCount(int *droids, int *structures, int *features); void checkFactoryFlags(); #endif -enum class IterationResult -{ - BREAK_ITERATION, - CONTINUE_ITERATION -}; - -// Common iteration helper for lists of game objects -// with an ability to execute loop body handlers which can -// possibly invalidate the any iterator in the range `[begin(), currentIterator]`. -template -void mutating_list_iterate(std::list& list, MaybeErasingLoopBodyHandler handler) -{ - static_assert(std::is_same, IterationResult>::value, - "Loop body handler should return IterationResult"); - - typename std::remove_reference_t::iterator it = list.begin(), itNext; - while (it != list.end()) - { - itNext = std::next(it); - // Can possibly invalidate `it` and anything before it. - const auto res = handler(*it); - if (res == IterationResult::BREAK_ITERATION) - { - break; - } - it = itNext; - } -} - #endif // __INCLUDED_SRC_OBJMEM_H__ diff --git a/src/order.cpp b/src/order.cpp index e72d1d01de6..c4f44e3f5f9 100644 --- a/src/order.cpp +++ b/src/order.cpp @@ -29,6 +29,7 @@ #include "lib/framework/frame.h" #include "lib/framework/input.h" #include "lib/framework/math_ext.h" +#include "lib/framework/object_list_iteration.h" #include "lib/ivis_opengl/ivisdef.h" #include "objects.h" From 3b4a5b9e9ae104ab7442621296c7cc3ad5f5bcde Mon Sep 17 00:00:00 2001 From: Pavel Solodovnikov Date: Sun, 21 Jan 2024 11:17:46 +0300 Subject: [PATCH 2/9] framework: allocate instances of `RES_TYPE` via C++ `new` Replace `malloc/free` by `new/delete` to allocate `RES_TYPE` instances, otherwise we'll have problems when replacing `psRes` pointer by a C++ container (e.g. `std::list`), because `malloc` doesn't know anything about C++ constructors. Signed-off-by: Pavel Solodovnikov --- lib/framework/frameresource.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/framework/frameresource.cpp b/lib/framework/frameresource.cpp index b86976e2f53..b9a248be3da 100644 --- a/lib/framework/frameresource.cpp +++ b/lib/framework/frameresource.cpp @@ -231,7 +231,7 @@ static RES_TYPE *resAlloc(const char *pType) #endif // setup the structure - psT = (RES_TYPE *)malloc(sizeof(RES_TYPE)); + psT = new RES_TYPE(); sstrcpy(psT->aType, pType); psT->HashedType = HashString(psT->aType); // store a hased version for super speed ! psT->psRes = nullptr; @@ -775,7 +775,7 @@ void resReleaseAll() for (psT = psResTypes; psT != nullptr; psT = psNT) { psNT = psT->psNext; - free(psT); + delete psT; } psResTypes = nullptr; From d07820612e52166ab3cf495fa22691fc6ec9f7c6 Mon Sep 17 00:00:00 2001 From: Pavel Solodovnikov Date: Sun, 21 Jan 2024 17:50:01 +0300 Subject: [PATCH 3/9] object_list_iteration: allow passing list iterator to `mutating_list_iterate` Use a compile-time helper with some SFINAE tricks to allow passing callables which take an iterator instead of a pointer type as the argument. This can be quite convenient when one needs to erase from or insert into the list being iterated directly inside the handler's body, avoiding additional lookups to obtain an iterator to the current element. The SFINAE function signature checks are performed using the following trick: std::is_convertible::value This is based on the ability of raw function pointers and lambda expressions to be converted to the `std::function` type with a matching signature. Paired with a `std::enable_if_t`, this allows to easily compile-time-choose the correct overload of the `Invoke` helper function, which would call the handler and pass the correct argument to it. Signed-off-by: Pavel Solodovnikov --- lib/framework/object_list_iteration.h | 66 +++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 3 deletions(-) diff --git a/lib/framework/object_list_iteration.h b/lib/framework/object_list_iteration.h index e505b560bcd..d92bdf000c2 100644 --- a/lib/framework/object_list_iteration.h +++ b/lib/framework/object_list_iteration.h @@ -31,21 +31,81 @@ enum class IterationResult CONTINUE_ITERATION }; +/// +/// Utility class used to determine how to invoke `Callable` +/// in the `mutating_list_iterate` function depending on its signature. +/// +/// Currently two callable signatures are supported: +/// * `IterationResult(ObjectType*)` +/// * `IterationResult(std::list::iterator)` +/// +/// The latter overload is convenient when one needs to erase from or +/// insert into the list being iterated directly inside the handler's body, +/// avoiding additional lookups to obtain an iterator to the current element. +/// +/// +/// Can either be a function pointer or a lambda expression (or any other callable). +/// +template +struct LoopBodyHandlerCallStrategy +{ + // Since we are constrained by C++14, we'll need to use SFINAE to determine correct + // specialization of `Invoke` to call depending on input type of handler function. + // + // Here we use a bunch of compile-time tests to determine + // whether our callable is convertible to `std::function` of a compatible signature. + // + // This is the most simple way to constrain and choose a correct overload + // of `Invoke` function depending on the callable signature. + // + // Choosing the correct overload is done via `std::enable_if` helper from , + // which basically provides `using type = T` only when `Cond` is `true`. + template + static constexpr bool handler_accepts_ptr = std::is_convertible< + Callable, + std::function>::value; + template + static constexpr bool handler_accepts_iter = std::is_convertible< + Callable, + std::function::iterator)>>::value; + + // `Invoke` overload for Callable taking a list iterator as the argument + template + static std::enable_if_t, IterationResult> + Invoke(Callable handler, typename std::list::iterator iter) + { + return handler(iter); + } + + // `Invoke` overload for Callable taking a pointer to `ObjectType` as the argument + template + static std::enable_if_t, IterationResult> + Invoke(Callable handler, typename std::list::iterator iter) + { + return handler(*iter); + } +}; + // Common iteration helper for lists of game objects // with an ability to execute loop body handlers which can // possibly invalidate the any iterator in the range `[begin(), currentIterator]`. template void mutating_list_iterate(std::list& list, MaybeErasingLoopBodyHandler handler) { - static_assert(std::is_same, IterationResult>::value, - "Loop body handler should return IterationResult"); + using HandlerCallStrategy = LoopBodyHandlerCallStrategy; + + static_assert( + HandlerCallStrategy::handler_accepts_ptr + || HandlerCallStrategy::handler_accepts_iter, + "Unsupported loop body handler signature: " + "should return IterationResult and take either an ObjectType* or an iterator"); typename std::remove_reference_t::iterator it = list.begin(), itNext; while (it != list.end()) { itNext = std::next(it); // Can possibly invalidate `it` and anything before it. - const auto res = handler(*it); + const auto res = HandlerCallStrategy::Invoke(handler, it); if (res == IterationResult::BREAK_ITERATION) { break; From cde81d8b563d87297cac8664290abedad524991b Mon Sep 17 00:00:00 2001 From: Pavel Solodovnikov Date: Sun, 21 Jan 2024 18:01:52 +0300 Subject: [PATCH 4/9] frameresource: change resource data lists to use `std::list` Change `RES_TYPE::psRes` to use `std::list` instead of a C-style intrusive list. Also, remove `RES_DATA::psNext` pointer as it's not needed anymore. Signed-off-by: Pavel Solodovnikov --- lib/framework/frameresource.cpp | 147 +++++++++++--------------------- lib/framework/frameresource.h | 5 +- 2 files changed, 51 insertions(+), 101 deletions(-) diff --git a/lib/framework/frameresource.cpp b/lib/framework/frameresource.cpp index b9a248be3da..26e195b7bab 100644 --- a/lib/framework/frameresource.cpp +++ b/lib/framework/frameresource.cpp @@ -27,10 +27,13 @@ #include "string_ext.h" #include "physfs_ext.h" +#include "object_list_iteration.h" #include "file.h" #include "resly.h" +#include + // Local prototypes static RES_TYPE *psResTypes = nullptr; @@ -234,7 +237,6 @@ static RES_TYPE *resAlloc(const char *pType) psT = new RES_TYPE(); sstrcpy(psT->aType, pType); psT->HashedType = HashString(psT->aType); // store a hased version for super speed ! - psT->psRes = nullptr; return psT; } @@ -468,7 +470,6 @@ bool resLoadFile(const char *pType, const char *pFile) { RES_TYPE *psT = nullptr; void *pData = nullptr; - RES_DATA *psRes = nullptr; char aFileName[PATH_MAX]; UDWORD HashedName, HashedType = HashString(pType); @@ -490,7 +491,7 @@ bool resLoadFile(const char *pType, const char *pFile) // Check for duplicates HashedName = HashStringIgnoreCase(pFile); - for (psRes = psT->psRes; psRes; psRes = psRes->psNext) + for (const RES_DATA* psRes : psT->psRes) { if (psRes->HashedID == HashedName) { @@ -562,7 +563,7 @@ bool resLoadFile(const char *pType, const char *pFile) if (pData != nullptr) { // LastResourceFilename may have been changed (e.g. by TEXPAGE loading) - psRes = resDataInit(GetLastResourceFilename(), HashStringIgnoreCase(GetLastResourceFilename()), pData, resBlockID); + RES_DATA* psRes = resDataInit(GetLastResourceFilename(), HashStringIgnoreCase(GetLastResourceFilename()), pData, resBlockID); if (!psRes) { if (psT->release != nullptr) @@ -573,8 +574,7 @@ bool resLoadFile(const char *pType, const char *pFile) } // Add the resource to the list - psRes->psNext = psT->psRes; - psT->psRes = psRes; + psT->psRes.emplace_front(psRes); } return true; } @@ -583,7 +583,6 @@ bool resLoadFile(const char *pType, const char *pFile) void *resGetDataFromHash(const char *pType, UDWORD HashedID) { RES_TYPE *psT = nullptr; - RES_DATA *psRes = nullptr; // Find the correct type UDWORD HashedType = HashString(pType); @@ -602,24 +601,20 @@ void *resGetDataFromHash(const char *pType, UDWORD HashedID) return nullptr; } - for (psRes = psT->psRes; psRes != nullptr; psRes = psRes->psNext) + auto res = std::find_if(psT->psRes.begin(), psT->psRes.end(), [HashedID](const RES_DATA* rd) { - if (psRes->HashedID == HashedID) - { - /* We found it */ - break; - } - } + return rd->HashedID == HashedID; + }); - ASSERT(psRes != nullptr, "resGetDataFromHash: Unknown ID: %0x Type: %s", HashedID, pType); - if (psRes == nullptr) + ASSERT(res != psT->psRes.end(), "resGetDataFromHash: Unknown ID: %0x Type: %s", HashedID, pType); + if (res == psT->psRes.end()) { return nullptr; } - psRes->usage += 1; + (*res)->usage += 1; - return psRes->pData; + return (*res)->pData; } @@ -635,7 +630,6 @@ void *resGetData(const char *pType, const char *pID) bool resGetHashfromData(const char *pType, const void *pData, UDWORD *pHash) { RES_TYPE *psT; - RES_DATA *psRes; // Find the correct type UDWORD HashedType = HashString(pType); @@ -651,22 +645,18 @@ bool resGetHashfromData(const char *pType, const void *pData, UDWORD *pHash) ASSERT_OR_RETURN(false, psT, "Unknown type: %x", HashedType); // Find the resource - for (psRes = psT->psRes; psRes; psRes = psRes->psNext) + auto res = std::find_if(psT->psRes.begin(), psT->psRes.end(), [pData](const RES_DATA* rd) { + return rd->pData == pData; + }); - if (psRes->pData == pData) - { - break; - } - } - - if (psRes == nullptr) + if (res == psT->psRes.end()) { ASSERT(false, "resGetHashfromData:: couldn't find data for type %x\n", HashedType); return false; } - *pHash = psRes->HashedID; + *pHash = (*res)->HashedID; return true; } @@ -674,7 +664,6 @@ bool resGetHashfromData(const char *pType, const void *pData, UDWORD *pHash) const char *resGetNamefromData(const char *type, const void *data) { RES_TYPE *psT; - RES_DATA *psRes; UDWORD HashedType; if (type == nullptr || data == nullptr) @@ -701,28 +690,24 @@ const char *resGetNamefromData(const char *type, const void *data) } // Find the resource in the resource table - for (psRes = psT->psRes; psRes; psRes = psRes->psNext) + auto res = std::find_if(psT->psRes.begin(), psT->psRes.end(), [data](const RES_DATA* rd) { - if (psRes->pData == data) - { - break; - } - } + return rd->pData == data; + }); - if (psRes == nullptr) + if (res == psT->psRes.end()) { ASSERT(false, "resGetHashfromData:: couldn't find data for type %x\n", HashedType); return ""; } - return psRes->aID; + return (*res)->aID; } /* Simply returns true if a resource is present */ bool resPresent(const char *pType, const char *pID) { RES_TYPE *psT; - RES_DATA *psRes; // Find the correct type UDWORD HashedType = HashString(pType); @@ -742,26 +727,12 @@ bool resPresent(const char *pType, const char *pID) return false; } + UDWORD HashedID = HashStringIgnoreCase(pID); + auto res = std::find_if(psT->psRes.begin(), psT->psRes.end(), [HashedID](const RES_DATA* rd) { - UDWORD HashedID = HashStringIgnoreCase(pID); - - for (psRes = psT->psRes; psRes; psRes = psRes->psNext) - { - if (psRes->HashedID == HashedID) - { - /* We found it */ - break; - } - } - } - - /* Did we find it? */ - if (psRes != nullptr) - { - return (true); - } - - return (false); + return rd->HashedID == HashedID; + }); + return res != psT->psRes.end(); } @@ -785,28 +756,22 @@ void resReleaseAll() /* Release all the resources currently loaded but keep the resource load functions */ void resReleaseAllData() { - RES_TYPE *psT; - RES_DATA *psRes, *psNRes; - - for (psT = psResTypes; psT != nullptr; psT = psT->psNext) + for (RES_TYPE* psT = psResTypes; psT != nullptr; psT = psT->psNext) { - for (psRes = psT->psRes; psRes != nullptr; psRes = psNRes) + mutating_list_iterate(psT->psRes, [psT](RES_DATA* psRes) { if (psRes->usage == 0) { debug(LOG_NEVER, "resReleaseAllData: %s resource: %s(%04x) not used", psT->aType, psRes->aID, psRes->HashedID); } - if (psT->release != nullptr) { psT->release(psRes->pData); } - - psNRes = psRes->psNext; free(psRes); - } - - psT->psRes = nullptr; + return IterationResult::CONTINUE_ITERATION; + }); + psT->psRes.clear(); } } @@ -815,46 +780,30 @@ void resReleaseAllData() void resReleaseBlockData(SDWORD blockID) { RES_TYPE *psT, *psNT; - RES_DATA *psPRes, *psRes, *psNRes; for (psT = psResTypes; psT != nullptr; psT = psNT) { - psPRes = nullptr; - for (psRes = psT->psRes; psRes; psRes = psNRes) + mutating_list_iterate(psT->psRes, [psT, blockID](std::list::iterator resIt) { - ASSERT(psRes != nullptr, "resReleaseBlockData: null pointer passed into loop"); - - if (psRes->blockID == blockID) + RES_DATA* psRes = *resIt; + if (psRes->blockID != blockID) { - if (psRes->usage == 0) - { - debug(LOG_NEVER, "resReleaseBlockData: %s resource: %s(%04x) not used", psT->aType, psRes->aID, - psRes->HashedID); - } - if (psT->release != nullptr) - { - psT->release(psRes->pData); - } - - psNRes = psRes->psNext; - free(psRes); - - if (psPRes == nullptr) - { - psT->psRes = psNRes; - } - else - { - psPRes->psNext = psNRes; - } + return IterationResult::CONTINUE_ITERATION; } - else + if (psRes->usage == 0) { - psPRes = psRes; - psNRes = psRes->psNext; + debug(LOG_NEVER, "resReleaseBlockData: %s resource: %s(%04x) not used", psT->aType, psRes->aID, + psRes->HashedID); + } + if (psT->release != nullptr) + { + psT->release(psRes->pData); } - } + psT->psRes.erase(resIt); + free(psRes); + return IterationResult::CONTINUE_ITERATION; + }); psNT = psT->psNext; } } diff --git a/lib/framework/frameresource.h b/lib/framework/frameresource.h index e4eba404e41..43ec452411d 100644 --- a/lib/framework/frameresource.h +++ b/lib/framework/frameresource.h @@ -26,6 +26,8 @@ #include "lib/framework/frame.h" +#include + /** Maximum number of characters in a resource type. */ #define RESTYPE_MAXCHAR 20 @@ -48,7 +50,6 @@ struct RES_DATA SDWORD blockID; // which of the blocks is it in (so we can clear some of them...) UDWORD HashedID; // hashed version of the name of the id - RES_DATA *psNext; // next entry - most likely to be following on! UDWORD usage; // Reference count // ID of the resource - filename from the .wrf - e.g. "TRON.PIE" @@ -67,7 +68,7 @@ struct RES_TYPE RES_FREE release; // routine to release the data (NULL indicates none) // we must have a pointer to the data here so that we can do a resGetData(); - RES_DATA *psRes; // Linked list of data items of this type + std::list psRes; // Linked list of data items of this type UDWORD HashedType; // hashed version of the name of the id - // a null hashedtype indicates end of list RES_FILELOAD fileLoad; // This isn't really used any more ? From c46f396c7bb7a8fa952df9635d587edc65aa5041 Mon Sep 17 00:00:00 2001 From: Pavel Solodovnikov Date: Sun, 21 Jan 2024 20:17:39 +0300 Subject: [PATCH 5/9] frameresource: change `psResTypes` to use `std::list` Also, remove now unused `psNext` member field from `RES_TYPE`. Signed-off-by: Pavel Solodovnikov --- lib/framework/frameresource.cpp | 108 ++++++++++++-------------------- lib/framework/frameresource.h | 1 - 2 files changed, 40 insertions(+), 69 deletions(-) diff --git a/lib/framework/frameresource.cpp b/lib/framework/frameresource.cpp index 26e195b7bab..52e04525784 100644 --- a/lib/framework/frameresource.cpp +++ b/lib/framework/frameresource.cpp @@ -35,7 +35,7 @@ #include // Local prototypes -static RES_TYPE *psResTypes = nullptr; +static std::list psResTypes; /* The initial resource directory and the current resource directory */ char aResDir[PATH_MAX]; @@ -148,9 +148,9 @@ void resDoResLoadCallback() /* Initialise the resource module */ bool resInitialise() { - ASSERT(psResTypes == nullptr, + ASSERT(psResTypes.empty(), "resInitialise: resource module hasn't been shut down??"); - psResTypes = nullptr; + psResTypes.clear(); resBlockID = 0; resLoadCallback = nullptr; @@ -163,7 +163,7 @@ bool resInitialise() /* Shutdown the resource module */ void resShutDown() { - if (psResTypes != nullptr) + if (!psResTypes.empty()) { debug(LOG_WZ, "resShutDown: warning resources still allocated"); resReleaseAll(); @@ -223,18 +223,16 @@ bool resLoad(const char *pResFile, SDWORD blockID) /* Allocate a RES_TYPE structure */ static RES_TYPE *resAlloc(const char *pType) { - RES_TYPE *psT; - #ifdef DEBUG // Check for a duplicate type - for (psT = psResTypes; psT; psT = psT->psNext) + for (RES_TYPE* psT : psResTypes) { ASSERT(strcmp(psT->aType, pType) != 0, "Duplicate function for type: %s", pType); } #endif // setup the structure - psT = new RES_TYPE(); + auto psT = new RES_TYPE(); sstrcpy(psT->aType, pType); psT->HashedType = HashString(psT->aType); // store a hased version for super speed ! @@ -251,8 +249,7 @@ bool resAddBufferLoad(const char *pType, RES_BUFFERLOAD buffLoad, RES_FREE relea psT->fileLoad = nullptr; psT->release = release; - psT->psNext = psResTypes; - psResTypes = psT; + psResTypes.emplace_front(psT); return true; } @@ -267,8 +264,7 @@ bool resAddFileLoad(const char *pType, RES_FILELOAD fileLoad, RES_FREE release) psT->fileLoad = fileLoad; psT->release = release; - psT->psNext = psResTypes; - psResTypes = psT; + psResTypes.emplace_front(psT); return true; } @@ -468,26 +464,27 @@ static void makeLocaleFile(char *fileName, size_t maxlen) // given string must */ bool resLoadFile(const char *pType, const char *pFile) { - RES_TYPE *psT = nullptr; void *pData = nullptr; char aFileName[PATH_MAX]; UDWORD HashedName, HashedType = HashString(pType); // Find the resource-type - for (psT = psResTypes; psT != nullptr; psT = psT->psNext) + auto resTypeIt = std::find_if(psResTypes.begin(), psResTypes.end(), [pType, HashedType](const RES_TYPE* psT) { if (psT->HashedType == HashedType) { ASSERT(strcmp(psT->aType, pType) == 0, "Hash collision \"%s\" vs \"%s\"", psT->aType, pType); - break; + return true; } - } + return false; + }); - if (psT == nullptr) + if (resTypeIt == psResTypes.end()) { debug(LOG_WZ, "resLoadFile: Unknown type: %s", pType); return false; } + RES_TYPE* psT = *resTypeIt; // Check for duplicates HashedName = HashStringIgnoreCase(pFile); @@ -582,24 +579,19 @@ bool resLoadFile(const char *pType, const char *pFile) /* Return the resource for a type and hashedname */ void *resGetDataFromHash(const char *pType, UDWORD HashedID) { - RES_TYPE *psT = nullptr; // Find the correct type UDWORD HashedType = HashString(pType); - for (psT = psResTypes; psT != nullptr; psT = psT->psNext) + auto resTypeIt = std::find_if(psResTypes.begin(), psResTypes.end(), [HashedType](const RES_TYPE* psT) { - if (psT->HashedType == HashedType) - { - /* We found it */ - break; - } - } - - ASSERT(psT != nullptr, "resGetDataFromHash: Unknown type: %s", pType); - if (psT == nullptr) + return psT->HashedType == HashedType; + }); + ASSERT(resTypeIt != psResTypes.end(), "resGetDataFromHash: Unknown type: %s", pType); + if (resTypeIt == psResTypes.end()) { return nullptr; } + RES_TYPE* psT = *resTypeIt; auto res = std::find_if(psT->psRes.begin(), psT->psRes.end(), [HashedID](const RES_DATA* rd) { @@ -629,20 +621,15 @@ void *resGetData(const char *pType, const char *pID) bool resGetHashfromData(const char *pType, const void *pData, UDWORD *pHash) { - RES_TYPE *psT; - // Find the correct type UDWORD HashedType = HashString(pType); - for (psT = psResTypes; psT != nullptr; psT = psT->psNext) + auto resTypeIt = std::find_if(psResTypes.begin(), psResTypes.end(), [HashedType](const RES_TYPE* psT) { - if (psT->HashedType == HashedType) - { - break; - } - } - - ASSERT_OR_RETURN(false, psT, "Unknown type: %x", HashedType); + return psT->HashedType == HashedType; + }); + ASSERT_OR_RETURN(false, resTypeIt != psResTypes.end(), "Unknown type: %x", HashedType); + RES_TYPE* psT = *resTypeIt; // Find the resource auto res = std::find_if(psT->psRes.begin(), psT->psRes.end(), [pData](const RES_DATA* rd) @@ -663,7 +650,6 @@ bool resGetHashfromData(const char *pType, const void *pData, UDWORD *pHash) const char *resGetNamefromData(const char *type, const void *data) { - RES_TYPE *psT; UDWORD HashedType; if (type == nullptr || data == nullptr) @@ -675,19 +661,16 @@ const char *resGetNamefromData(const char *type, const void *data) HashedType = HashString(type); // Find the resource table for the given type - for (psT = psResTypes; psT != nullptr; psT = psT->psNext) + auto resTypeIt = std::find_if(psResTypes.begin(), psResTypes.end(), [HashedType](const RES_TYPE* psT) { - if (psT->HashedType == HashedType) - { - break; - } - } - - if (psT == nullptr) + return psT->HashedType == HashedType; + }); + if (resTypeIt == psResTypes.end()) { ASSERT(false, "resGetHashfromData: Unknown type: %x", HashedType); return ""; } + RES_TYPE* psT = *resTypeIt; // Find the resource in the resource table auto res = std::find_if(psT->psRes.begin(), psT->psRes.end(), [data](const RES_DATA* rd) @@ -707,25 +690,20 @@ const char *resGetNamefromData(const char *type, const void *data) /* Simply returns true if a resource is present */ bool resPresent(const char *pType, const char *pID) { - RES_TYPE *psT; - // Find the correct type UDWORD HashedType = HashString(pType); - for (psT = psResTypes; psT != nullptr; psT = psT->psNext) + auto resTypeIt = std::find_if(psResTypes.begin(), psResTypes.end(), [HashedType](const RES_TYPE* psT) { - if (psT->HashedType == HashedType) - { - break; - } - } - + return psT->HashedType == HashedType; + }); /* Bow out if unrecognised type */ - ASSERT(psT != nullptr, "resPresent: Unknown type"); - if (psT == nullptr) + ASSERT(resTypeIt != psResTypes.end(), "resPresent: Unknown type"); + if (resTypeIt == psResTypes.end()) { return false; } + RES_TYPE* psT = *resTypeIt; UDWORD HashedID = HashStringIgnoreCase(pID); auto res = std::find_if(psT->psRes.begin(), psT->psRes.end(), [HashedID](const RES_DATA* rd) @@ -739,24 +717,21 @@ bool resPresent(const char *pType, const char *pID) /* Release all the resources currently loaded and the resource load functions */ void resReleaseAll() { - RES_TYPE *psT, *psNT; - resReleaseAllData(); - for (psT = psResTypes; psT != nullptr; psT = psNT) + for (RES_TYPE* psT : psResTypes) { - psNT = psT->psNext; delete psT; } - psResTypes = nullptr; + psResTypes.clear(); } /* Release all the resources currently loaded but keep the resource load functions */ void resReleaseAllData() { - for (RES_TYPE* psT = psResTypes; psT != nullptr; psT = psT->psNext) + for (RES_TYPE* psT : psResTypes) { mutating_list_iterate(psT->psRes, [psT](RES_DATA* psRes) { @@ -779,9 +754,7 @@ void resReleaseAllData() // release the data for a particular block ID void resReleaseBlockData(SDWORD blockID) { - RES_TYPE *psT, *psNT; - - for (psT = psResTypes; psT != nullptr; psT = psNT) + for (RES_TYPE* psT : psResTypes) { mutating_list_iterate(psT->psRes, [psT, blockID](std::list::iterator resIt) { @@ -804,6 +777,5 @@ void resReleaseBlockData(SDWORD blockID) free(psRes); return IterationResult::CONTINUE_ITERATION; }); - psNT = psT->psNext; } } diff --git a/lib/framework/frameresource.h b/lib/framework/frameresource.h index 43ec452411d..b902baf8f64 100644 --- a/lib/framework/frameresource.h +++ b/lib/framework/frameresource.h @@ -72,7 +72,6 @@ struct RES_TYPE UDWORD HashedType; // hashed version of the name of the id - // a null hashedtype indicates end of list RES_FILELOAD fileLoad; // This isn't really used any more ? - RES_TYPE *psNext; }; From a4910b850fa642d6b0ce53a2600a6a277967b05f Mon Sep 17 00:00:00 2001 From: Pavel Solodovnikov Date: Sun, 21 Jan 2024 21:55:49 +0300 Subject: [PATCH 6/9] audio.cpp: convert `g_psSampleList` and `g_psSampleQueue` to `std::list` Also, remove `psPrev` and `psNext` pointers from `AUDIO_SAMPLE`. Signed-off-by: Pavel Solodovnikov --- lib/sound/audio.cpp | 265 +++++++++++++------------------------------- lib/sound/track.h | 2 - 2 files changed, 76 insertions(+), 191 deletions(-) diff --git a/lib/sound/audio.cpp b/lib/sound/audio.cpp index 06de2bb0e4b..fca041fe74c 100644 --- a/lib/sound/audio.cpp +++ b/lib/sound/audio.cpp @@ -23,6 +23,7 @@ #include "lib/gamelib/gtime.h" #include "lib/ivis_opengl/pietypes.h" #include "lib/framework/physfs_ext.h" +#include "lib/framework/object_list_iteration.h" #include "oggopus.h" #include "oggvorbis.h" @@ -32,13 +33,16 @@ #include "audio_id.h" #include "openal_error.h" #include "mixer.h" + +#include + // defines #define NO_SAMPLE - 2 #define MAX_SAME_SAMPLES 2 // global variables -static AUDIO_SAMPLE *g_psSampleList = nullptr; -static AUDIO_SAMPLE *g_psSampleQueue = nullptr; +static std::list g_psSampleList; +static std::list g_psSampleQueue; static bool g_bAudioEnabled = false; static bool g_bAudioPaused = false; static AUDIO_SAMPLE g_sPreviousSample; @@ -49,20 +53,7 @@ static int g_iPreviousSampleTime = 0; */ unsigned int audio_GetSampleQueueCount() { - //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - AUDIO_SAMPLE *psSample = nullptr; - unsigned int count = 0; - //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - - // loop through SampleQueue to count how many we have. - psSample = g_psSampleQueue; - while (psSample != nullptr) - { - count++; - psSample = psSample->psNext; - } - - return count; + return static_cast(g_psSampleQueue.size()); } /** Counts the number of samples in the SampleList @@ -70,20 +61,7 @@ unsigned int audio_GetSampleQueueCount() */ unsigned int audio_GetSampleListCount() { - //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - AUDIO_SAMPLE *psSample = nullptr; - unsigned int count = 0; - //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - - // loop through SampleList to count how many we have. - psSample = g_psSampleList; - while (psSample != nullptr) - { - ++count; - psSample = psSample->psNext; - } - - return count; + return static_cast(g_psSampleList.size()); } //* // ======================================================================================================================= @@ -124,7 +102,6 @@ bool audio_Init(AUDIO_CALLBACK pStopTrackCallback, HRTFMode hrtf, bool really_en bool audio_Shutdown(void) { //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - AUDIO_SAMPLE *psSample = nullptr, *psSampleTemp = nullptr; bool bOK; //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -138,26 +115,20 @@ bool audio_Shutdown(void) bOK = sound_Shutdown(); // empty sample list - psSample = g_psSampleList; - while (psSample != nullptr) + for (AUDIO_SAMPLE* psSample : g_psSampleList) { - psSampleTemp = psSample->psNext; free(psSample); - psSample = psSampleTemp; } // empty sample queue - psSample = g_psSampleQueue; - while (psSample != nullptr) + for (AUDIO_SAMPLE* psSample : g_psSampleQueue) { - psSampleTemp = psSample->psNext; free(psSample); - psSample = psSampleTemp; } // free sample heap - g_psSampleList = nullptr; - g_psSampleQueue = nullptr; + g_psSampleList.clear(); + g_psSampleQueue.clear(); return bOK; } @@ -211,42 +182,18 @@ bool audio_GetPreviousQueueTrackRadarBlipPos(SDWORD *iX, SDWORD *iY) // ======================================================================================================================= // ======================================================================================================================= // -static void audio_AddSampleToHead(AUDIO_SAMPLE **ppsSampleList, AUDIO_SAMPLE *psSample) +static void audio_AddSampleToHead(std::list& ppsSampleList, AUDIO_SAMPLE *psSample) { - psSample->psNext = (*ppsSampleList); - psSample->psPrev = nullptr; - if ((*ppsSampleList) != nullptr) - { - (*ppsSampleList)->psPrev = psSample; - } - (*ppsSampleList) = psSample; + ppsSampleList.emplace_front(psSample); } //* // ======================================================================================================================= // ======================================================================================================================= // -static void audio_AddSampleToTail(AUDIO_SAMPLE **ppsSampleList, AUDIO_SAMPLE *psSample) +static void audio_AddSampleToTail(std::list& ppsSampleList, AUDIO_SAMPLE *psSample) { - //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - AUDIO_SAMPLE *psSampleTail = nullptr; - //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - - if ((*ppsSampleList) == nullptr) - { - (*ppsSampleList) = psSample; - return; - } - - psSampleTail = (*ppsSampleList); - while (psSampleTail->psNext != nullptr) - { - psSampleTail = psSampleTail->psNext; - } - - psSampleTail->psNext = psSample; - psSample->psPrev = psSampleTail; - psSample->psNext = nullptr; + ppsSampleList.emplace_back(psSample); } //* @@ -257,32 +204,16 @@ static void audio_AddSampleToTail(AUDIO_SAMPLE **ppsSampleList, AUDIO_SAMPLE *ps // ======================================================================================================================= // ======================================================================================================================= // -static void audio_RemoveSample(AUDIO_SAMPLE **ppsSampleList, AUDIO_SAMPLE *psSample) +static void audio_RemoveSample(std::list& ppsSampleList, AUDIO_SAMPLE *psSample) { if (psSample == nullptr) { return; } - if (psSample == (*ppsSampleList)) - { - // first sample in list - (*ppsSampleList) = psSample->psNext; - } - - if (psSample->psPrev != nullptr) - { - psSample->psPrev->psNext = psSample->psNext; - } - - if (psSample->psNext != nullptr) - { - psSample->psNext->psPrev = psSample->psPrev; - } - - // set sample pointers NULL for safety - psSample->psPrev = nullptr; - psSample->psNext = nullptr; + auto it = std::find(ppsSampleList.begin(), ppsSampleList.end(), psSample); + ASSERT(it != ppsSampleList.end(), "audio_RemoveSample: sample not found in list"); + ppsSampleList.erase(it); } //* @@ -293,7 +224,6 @@ static bool audio_CheckSameQueueTracksPlaying(SDWORD iTrack) { //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ SDWORD iCount; - AUDIO_SAMPLE *psSample = nullptr; bool bOK = true; //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -306,8 +236,7 @@ static bool audio_CheckSameQueueTracksPlaying(SDWORD iTrack) iCount = 0; // loop through queue sounds and check whether too many already in it - psSample = g_psSampleQueue; - while (psSample != nullptr) + for (const AUDIO_SAMPLE* psSample : g_psSampleQueue) { if (psSample->iTrack == iTrack) { @@ -319,8 +248,6 @@ static bool audio_CheckSameQueueTracksPlaying(SDWORD iTrack) bOK = false; break; } - - psSample = psSample->psNext; } return bOK; @@ -364,7 +291,7 @@ static AUDIO_SAMPLE *audio_QueueSample(SDWORD iTrack) psSample->bFinishedPlaying = false; // add to queue - audio_AddSampleToTail(&g_psSampleQueue, psSample); + audio_AddSampleToTail(g_psSampleQueue, psSample); return psSample; } @@ -516,14 +443,14 @@ static void audio_UpdateQueue(void) } // check queue for members - if (g_psSampleQueue == nullptr) + if (g_psSampleQueue.empty()) { return; } // remove queue head - psSample = g_psSampleQueue; - audio_RemoveSample(&g_psSampleQueue, psSample); + psSample = g_psSampleQueue.front(); + audio_RemoveSample(g_psSampleQueue, psSample); // add sample to list if able to play if (!sound_Play2DTrack(psSample, true)) @@ -533,7 +460,7 @@ static void audio_UpdateQueue(void) return; } - audio_AddSampleToHead(&g_psSampleList, psSample); + audio_AddSampleToHead(g_psSampleList, psSample); // update last queue sound coords if (psSample->x != SAMPLE_COORD_INVALID && psSample->y != SAMPLE_COORD_INVALID @@ -552,7 +479,6 @@ void audio_Update() //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Vector3f playerPos; float angle; - AUDIO_SAMPLE *psSample, *psSampleTemp; //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // if audio not enabled return true to carry on game without audio @@ -572,25 +498,20 @@ void audio_Update() sound_SetPlayerOrientation(angle); // loop through 3D sounds and remove if finished or update position - psSample = g_psSampleList; - while (psSample != nullptr) + mutating_list_iterate(g_psSampleList, [](AUDIO_SAMPLE* psSample) { // remove finished samples from list if (psSample->bFinishedPlaying == true) { - psSampleTemp = psSample->psNext; - audio_RemoveSample(&g_psSampleList, psSample); + audio_RemoveSample(g_psSampleList, psSample); free(psSample); - psSample = psSampleTemp; } - - // check looping sound callbacks for finished condition - else + else // check looping sound callbacks for finished condition { if (psSample->psObj != nullptr) { if (audio_ObjectDead(psSample->psObj) - || (psSample->pCallback != nullptr && psSample->pCallback(psSample->psObj) == false)) + || (psSample->pCallback != nullptr && psSample->pCallback(psSample->psObj) == false)) { sound_StopTrack(psSample); psSample->psObj = nullptr; @@ -601,10 +522,9 @@ void audio_Update() sound_SetObjectPosition(psSample); } } - // next sample - psSample = psSample->psNext; } - } + return IterationResult::CONTINUE_ITERATION; + }); sound_Update(); return; @@ -644,7 +564,6 @@ static bool audio_CheckSame3DTracksPlaying(SDWORD iTrack, SDWORD iX, SDWORD iY, { //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ SDWORD iCount, iDx, iDy, iDz, iDistSq, iMaxDistSq, iRad; - AUDIO_SAMPLE *psSample = nullptr; bool bOK = true; //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -657,8 +576,7 @@ static bool audio_CheckSame3DTracksPlaying(SDWORD iTrack, SDWORD iX, SDWORD iY, iCount = 0; // loop through 3D sounds and check whether too many already in earshot - psSample = g_psSampleList; - while (psSample != nullptr) + for (const AUDIO_SAMPLE* psSample : g_psSampleList) { if (psSample->iTrack == iTrack) { @@ -679,8 +597,6 @@ static bool audio_CheckSame3DTracksPlaying(SDWORD iTrack, SDWORD iX, SDWORD iY, break; } } - - psSample = psSample->psNext; } return bOK; @@ -764,7 +680,7 @@ static bool audio_Play3DTrack(SDWORD iX, SDWORD iY, SDWORD iZ, int iTrack, SIMPL return false; } - audio_AddSampleToHead(&g_psSampleList, psSample); + audio_AddSampleToHead(g_psSampleList, psSample); return true; } @@ -884,10 +800,6 @@ AUDIO_STREAM *audio_PlayStream(const char *fileName, float volume, const std::fu // void audio_StopObjTrack(SIMPLE_OBJECT *psObj, int iTrack) { - //~~~~~~~~~~~~~~~~~~~~~~ - AUDIO_SAMPLE *psSample; - //~~~~~~~~~~~~~~~~~~~~~~ - // return if audio not enabled if (g_bAudioEnabled == false) { @@ -895,8 +807,7 @@ void audio_StopObjTrack(SIMPLE_OBJECT *psObj, int iTrack) } // find sample - psSample = g_psSampleList; - while (psSample != nullptr) + for (AUDIO_SAMPLE* psSample : g_psSampleList) { // If track has been found stop it and return if (psSample->psObj == psObj && psSample->iTrack == iTrack) @@ -904,9 +815,6 @@ void audio_StopObjTrack(SIMPLE_OBJECT *psObj, int iTrack) sound_StopTrack(psSample); return; } - - // get next sample from linked list - psSample = psSample->psNext; } } static UDWORD lastTimeBuildFailedPlayed = 0; @@ -970,7 +878,7 @@ void audio_PlayTrack(int iTrack) return; } - audio_AddSampleToHead(&g_psSampleList, psSample); + audio_AddSampleToHead(g_psSampleList, psSample); } //* @@ -1011,10 +919,6 @@ void audio_ResumeAll(void) // void audio_StopAll(void) { - //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - AUDIO_SAMPLE *psSample; - //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - // return if audio not enabled if (g_bAudioEnabled == false) { @@ -1024,7 +928,7 @@ void audio_StopAll(void) // // * empty list - audio_Update will free samples because callbacks have to come in first // - for (psSample = g_psSampleList; psSample != nullptr; psSample = psSample->psNext) + for (AUDIO_SAMPLE* psSample : g_psSampleList) { // Stop this sound sample sound_StopTrack(psSample); @@ -1046,19 +950,12 @@ void audio_StopAll(void) } // empty sample queue - psSample = g_psSampleQueue; - while (psSample != nullptr) + for (AUDIO_SAMPLE* psSample : g_psSampleQueue) { - AUDIO_SAMPLE *psSampleTemp = psSample; - - // Advance the sample iterator (before invalidating it) - psSample = psSample->psNext; - // Destroy the sample - free(psSampleTemp); + free(psSample); } - - g_psSampleQueue = nullptr; + g_psSampleQueue.clear(); } //* @@ -1102,35 +999,30 @@ void audio_RemoveObj(SIMPLE_OBJECT const *psObj) unsigned int count = 0; // loop through queued sounds and check if a sample needs to be removed - AUDIO_SAMPLE *psSample = g_psSampleQueue; - while (psSample != nullptr) + mutating_list_iterate(g_psSampleQueue, [psObj, &count](AUDIO_SAMPLE* psSample) { - if (psSample->psObj == psObj) + if (psSample->psObj != psObj) { - // The current audio sample seems to refer to an object - // that is about to be destroyed. So destroy this - // sample as well. - AUDIO_SAMPLE *toRemove = psSample; + return IterationResult::CONTINUE_ITERATION; + } + // The current audio sample seems to refer to an object + // that is about to be destroyed. So destroy this + // sample as well. + AUDIO_SAMPLE* toRemove = psSample; - // Make sure to keep our linked list iterator valid - psSample = psSample->psNext; + debug(LOG_MEMORY, "audio_RemoveObj: callback %p sample %d\n", reinterpret_cast(toRemove->pCallback), toRemove->iTrack); + // Remove sound from global active list + sound_RemoveActiveSample(toRemove); //remove from global active list. - debug(LOG_MEMORY, "audio_RemoveObj: callback %p sample %d\n", reinterpret_cast(toRemove->pCallback), toRemove->iTrack); - // Remove sound from global active list - sound_RemoveActiveSample(toRemove); //remove from global active list. + // Perform the actual task of destroying this sample + audio_RemoveSample(g_psSampleQueue, toRemove); + free(toRemove); - // Perform the actual task of destroying this sample - audio_RemoveSample(&g_psSampleQueue, toRemove); - free(toRemove); + // Increment the deletion count + ++count; - // Increment the deletion count - ++count; - } - else - { - psSample = psSample->psNext; - } - } + return IterationResult::CONTINUE_ITERATION; + }); if (count) { @@ -1141,35 +1033,30 @@ void audio_RemoveObj(SIMPLE_OBJECT const *psObj) count = 0; // loop through list of currently playing sounds and check if a sample needs to be removed - psSample = g_psSampleList; - while (psSample != nullptr) + mutating_list_iterate(g_psSampleList, [psObj, &count](AUDIO_SAMPLE* psSample) { - if (psSample->psObj == psObj) + if (psSample->psObj != psObj) { - // The current audio sample seems to refer to an object - // that is about to be destroyed. So destroy this - // sample as well. - AUDIO_SAMPLE *toRemove = psSample; + return IterationResult::CONTINUE_ITERATION; + } + // The current audio sample seems to refer to an object + // that is about to be destroyed. So destroy this + // sample as well. + AUDIO_SAMPLE* toRemove = psSample; - // Make sure to keep our linked list iterator valid - psSample = psSample->psNext; + debug(LOG_MEMORY, "audio_RemoveObj: callback %p sample %d\n", reinterpret_cast(toRemove->pCallback), toRemove->iTrack); + // Stop this sound sample + sound_RemoveActiveSample(toRemove); //remove from global active list. - debug(LOG_MEMORY, "audio_RemoveObj: callback %p sample %d\n", reinterpret_cast(toRemove->pCallback), toRemove->iTrack); - // Stop this sound sample - sound_RemoveActiveSample(toRemove); //remove from global active list. + // Perform the actual task of destroying this sample + audio_RemoveSample(g_psSampleList, toRemove); + free(toRemove); - // Perform the actual task of destroying this sample - audio_RemoveSample(&g_psSampleList, toRemove); - free(toRemove); + // Increment the deletion count + ++count; - // Increment the deletion count - ++count; - } - else - { - psSample = psSample->psNext; - } - } + return IterationResult::CONTINUE_ITERATION; + }); if (count) { diff --git a/lib/sound/track.h b/lib/sound/track.h index 00e7f06fca3..1b54d9c0dcb 100644 --- a/lib/sound/track.h +++ b/lib/sound/track.h @@ -61,8 +61,6 @@ struct AUDIO_SAMPLE bool bFinishedPlaying; AUDIO_CALLBACK pCallback; SIMPLE_OBJECT *psObj; - AUDIO_SAMPLE *psPrev; - AUDIO_SAMPLE *psNext; }; struct TRACK From c1eb169261ef5f51074614ef1e97292cfe4ba762 Mon Sep 17 00:00:00 2001 From: Pavel Solodovnikov Date: Tue, 23 Jan 2024 13:01:07 +0300 Subject: [PATCH 7/9] object_list_iteration.h: add early return to `mutating_list_iterate` This is sort of a hack/workaround to prevent crashes related to `TargetMissing_` static instance of `BASE_OBJECT` defined in the `multibot.cpp`. By the time this object gets destroyed, audio-related global vars are already destroyed and this object references one of them in its destructor via `audio_RemoveObj(this)` call. This should be addressed later, of course. But still, not crashing is better than crashing and early return is never bad. Signed-off-by: Pavel Solodovnikov --- lib/framework/object_list_iteration.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/framework/object_list_iteration.h b/lib/framework/object_list_iteration.h index d92bdf000c2..f4dd377b851 100644 --- a/lib/framework/object_list_iteration.h +++ b/lib/framework/object_list_iteration.h @@ -100,6 +100,11 @@ void mutating_list_iterate(std::list& list, MaybeErasingLoopBodyHan "Unsupported loop body handler signature: " "should return IterationResult and take either an ObjectType* or an iterator"); + if (list.empty()) + { + return; + } + typename std::remove_reference_t::iterator it = list.begin(), itNext; while (it != list.end()) { From 2cd2eb2e23417290290d6bba518f8a9eaabe88a6 Mon Sep 17 00:00:00 2001 From: Pavel Solodovnikov Date: Tue, 23 Jan 2024 13:09:56 +0300 Subject: [PATCH 8/9] objmem.h: remove commented include Signed-off-by: Pavel Solodovnikov --- src/objmem.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/objmem.h b/src/objmem.h index b0f29f6bff0..c17033d8034 100644 --- a/src/objmem.h +++ b/src/objmem.h @@ -26,8 +26,6 @@ #include "objectdef.h" -//#include "lib/framework/object_list_iteration.h" - #include #include #include From 12585b9fb828be76532bfb08848658aab744bc76 Mon Sep 17 00:00:00 2001 From: Pavel Solodovnikov Date: Tue, 23 Jan 2024 16:40:10 +0300 Subject: [PATCH 9/9] Compilation adjustments for GCC/Clang These compilers are more strict than MSVC, so fix the compilation errors coming from the linux build. Signed-off-by: Pavel Solodovnikov --- lib/framework/frameresource.cpp | 1 + lib/framework/object_list_iteration.h | 9 +++++---- src/objmem.h | 1 - 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/framework/frameresource.cpp b/lib/framework/frameresource.cpp index 52e04525784..62183602881 100644 --- a/lib/framework/frameresource.cpp +++ b/lib/framework/frameresource.cpp @@ -33,6 +33,7 @@ #include "resly.h" #include +#include // Local prototypes static std::list psResTypes; diff --git a/lib/framework/object_list_iteration.h b/lib/framework/object_list_iteration.h index f4dd377b851..804f5a345be 100644 --- a/lib/framework/object_list_iteration.h +++ b/lib/framework/object_list_iteration.h @@ -24,6 +24,7 @@ #include #include #include +#include enum class IterationResult { @@ -67,7 +68,7 @@ struct LoopBodyHandlerCallStrategy template static constexpr bool handler_accepts_iter = std::is_convertible< Callable, - std::function::iterator)>>::value; + std::function::iterator)>>::value; // `Invoke` overload for Callable taking a list iterator as the argument template @@ -95,8 +96,8 @@ void mutating_list_iterate(std::list& list, MaybeErasingLoopBodyHan using HandlerCallStrategy = LoopBodyHandlerCallStrategy; static_assert( - HandlerCallStrategy::handler_accepts_ptr - || HandlerCallStrategy::handler_accepts_iter, + HandlerCallStrategy::template handler_accepts_ptr + || HandlerCallStrategy::template handler_accepts_iter, "Unsupported loop body handler signature: " "should return IterationResult and take either an ObjectType* or an iterator"); @@ -110,7 +111,7 @@ void mutating_list_iterate(std::list& list, MaybeErasingLoopBodyHan { itNext = std::next(it); // Can possibly invalidate `it` and anything before it. - const auto res = HandlerCallStrategy::Invoke(handler, it); + const auto res = HandlerCallStrategy::template Invoke(handler, it); if (res == IterationResult::BREAK_ITERATION) { break; diff --git a/src/objmem.h b/src/objmem.h index c17033d8034..3d3c4122d04 100644 --- a/src/objmem.h +++ b/src/objmem.h @@ -28,7 +28,6 @@ #include #include -#include /* The lists of objects allocated */ template