Fix stuck soldiers leading to crashes in drawing code

libs/common/include/helpers/PtrSpan.h

@@ -25,8 +25,8 @@ namespace detail {
 } // namespace detail
 
 /// View type (access to current underlying container) which provides iterable access to a range of pointer-like
-/// elements Those can be raw pointers or smart pointers and are assumed to be non-NULL Iteration yields references to
-/// the pointed-to type
+/// elements. Those can be raw pointers or smart pointers and are assumed to be non-NULL.
+/// Iteration yields references to the pointed-to type
 template<class TRange>
 class NonNullPtrSpan
 {

libs/common/include/helpers/Range.h

@@ -1,40 +1,51 @@
-// Copyright (C) 2005 - 2021 Settlers Freaks (sf-team at siedler25.org)
+// Copyright (C) 2005 - 2024 Settlers Freaks (sf-team at siedler25.org)
 //
 // SPDX-License-Identifier: GPL-2.0-or-later
 
 #pragma once
 
+#include "RTTR_Assert.h"
 #include <boost/config.hpp>
+#include <iterator>
 #include <type_traits>
 
 namespace helpers {
-/// Can be used in range-based for loops to iterate over each value in [0, endValue)
+/// Similar to Pythons `range()` function can can be used in range-based for loops:
+/// Yield each value in [startValue, endValue) or [0, endValue) if startValue is not given
 /// Requires: T must be an integral
 template<typename T>
-struct Range
+class range
 {
     static_assert(std::is_integral<T>::value, "Must be an integral!");
+    const T startValue_;
+    const T endValue_;
+
+public:
+    explicit range(T endValue) : range(0, endValue) {}
+    explicit range(T startValue, T endValue) : startValue_(startValue), endValue_(endValue)
+    {
+        RTTR_Assert(startValue <= endValue);
+    }
+
     class iterator
     {
         T value;
 
     public:
+        using iterator_category = std::forward_iterator_tag;
+        using value_type = T;
+        using difference_type = std::make_signed_t<std::common_type_t<T, int>>;
+        using pointer = T*;
+        using reference = T&;
+
         explicit BOOST_FORCEINLINE constexpr iterator(T value) : value(value) {}
         BOOST_FORCEINLINE constexpr T operator*() const noexcept { return value; }
         BOOST_FORCEINLINE void operator++() noexcept { ++value; }
         BOOST_FORCEINLINE constexpr bool operator!=(iterator rhs) const noexcept { return value != rhs.value; }
     };
 
-    BOOST_FORCEINLINE constexpr iterator begin() const noexcept { return iterator(0); }
-    BOOST_FORCEINLINE constexpr iterator end() const noexcept { return iterator(endValue); }
-
-    const T endValue;
+    BOOST_FORCEINLINE constexpr iterator begin() const noexcept { return iterator(startValue_); }
+    BOOST_FORCEINLINE constexpr iterator end() const noexcept { return iterator(endValue_); }
 };
 
-template<typename T>
-constexpr Range<T> range(T endValue)
-{
-    return {endValue};
-}
-
 } // namespace helpers

libs/libGamedata/gameData/DescIdx.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2005 - 2021 Settlers Freaks (sf-team at siedler25.org)
+// Copyright (C) 2005 - 2024 Settlers Freaks (sf-team at siedler25.org)
 //
 // SPDX-License-Identifier: GPL-2.0-or-later
 
@@ -11,11 +11,17 @@
 template<class T>
 struct DescIdx
 {
+    using value_type = uint8_t;
+    using index_type = T;
+
     /// Invalid index
-    static constexpr uint8_t INVALID = 0xFF;
-    uint8_t value;
-    explicit constexpr DescIdx(uint8_t value = INVALID) noexcept : value(value) {}
-    constexpr bool operator!() const noexcept { return value == INVALID; }
+    static constexpr value_type INVALID = 0xFF;
+
+    value_type value = INVALID;
+
+    constexpr DescIdx() noexcept = default;
+    explicit constexpr DescIdx(value_type value) noexcept : value(value) {}
+    constexpr explicit operator bool() const noexcept { return value != INVALID; }
     constexpr bool operator==(DescIdx rhs) const noexcept { return value == rhs.value; }
     constexpr bool operator!=(DescIdx rhs) const noexcept { return value != rhs.value; }
     constexpr bool operator<(DescIdx rhs) const noexcept { return value < rhs.value; }

libs/libGamedata/gameData/DescriptionContainer.h

@@ -1,10 +1,12 @@
-// Copyright (C) 2005 - 2021 Settlers Freaks (sf-team at siedler25.org)
+// Copyright (C) 2005 - 2024 Settlers Freaks (sf-team at siedler25.org)
 //
 // SPDX-License-Identifier: GPL-2.0-or-later
 
 #pragma once
 
 #include "DescIdx.h"
+#include "DescriptionVector.h"
+#include "helpers/containerUtils.h"
 #include <map>
 #include <stdexcept>
 #include <string>
@@ -28,9 +30,18 @@ struct DescriptionContainer
     const T& get(DescIdx<T> idx) const;
     /// Return a mutable reference to the given item
     T& getMutable(DescIdx<T> idx);
+    /// Return the first item matching the predicate
+    template<class Predicate>
+    constexpr DescIdx<T> find(Predicate&& predicate) const;
+    /// Return all items matching the predicate
+    template<class Predicate>
+    std::vector<DescIdx<T>> findAll(Predicate&& predicate) const;
+
+    auto begin() const { return items.begin(); }
+    auto end() const { return items.end(); }
 
 private:
-    std::vector<T> items;
+    DescriptionVector<T, T> items;
     std::map<std::string, unsigned> name2Idx;
 };
 
@@ -63,7 +74,7 @@ inline const T* DescriptionContainer<T>::tryGet(const DescIdx<T> idx) const
 {
     if(!idx || idx.value >= size())
         return nullptr;
-    return &items[idx.value];
+    return &items[idx];
 }
 
 template<typename T>
@@ -75,11 +86,32 @@ inline const T* DescriptionContainer<T>::tryGet(const std::string& name) const
 template<typename T>
 inline const T& DescriptionContainer<T>::get(const DescIdx<T> idx) const
 {
-    return items[idx.value];
+    return items[idx];
 }
 
 template<typename T>
 inline T& DescriptionContainer<T>::getMutable(const DescIdx<T> idx)
 {
-    return items[idx.value];
+    return items[idx];
+}
+
+template<typename T>
+template<class Predicate>
+constexpr DescIdx<T> DescriptionContainer<T>::find(Predicate&& predicate) const
+{
+    const auto idx = helpers::indexOf_if(items, std::forward<Predicate>(predicate));
+    return idx >= 0 ? DescIdx<T>(static_cast<typename DescIdx<T>::value_type>(idx)) : DescIdx<T>();
 }
+
+template<typename T>
+template<class Predicate>
+std::vector<DescIdx<T>> DescriptionContainer<T>::findAll(Predicate&& predicate) const
+{
+    std::vector<DescIdx<T>> result;
+    for(const auto i : items.indices())
+    {
+        if(predicate(items[i]))
+            result.push_back(i);
+    }
+    return result;
+}

libs/libGamedata/gameData/DescriptionVector.h

@@ -0,0 +1,75 @@
+// Copyright (C) 2005 - 2024 Settlers Freaks (sf-team at siedler25.org)
+//
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#pragma once
+
+#include "DescIdx.h"
+#include <boost/config.hpp>
+#include <utility>
+#include <vector>
+
+template<typename T>
+class IndexRange;
+
+/// Adapter that provides type-safe access to elements by their description index
+template<typename T_Element, typename T_Index>
+class DescriptionVector
+{
+    std::vector<T_Element> storage;
+
+public:
+    using index_type = DescIdx<T_Index>;
+    using index_range_type = IndexRange<T_Index>;
+
+    DescriptionVector() = default;
+    DescriptionVector(size_t count) : storage(count) {}
+
+    auto size() const { return storage.size(); }
+    auto begin() { return storage.begin(); }
+    auto begin() const { return storage.begin(); }
+    auto end() { return storage.end(); }
+    auto end() const { return storage.end(); }
+    T_Element& operator[](const index_type idx) { return storage[idx.value]; }
+    const T_Element& operator[](const index_type idx) const { return storage[idx.value]; }
+
+    void resize(const size_t new_size) { storage.resize(new_size); }
+    void clear() { storage.clear(); }
+    void push_back(const T_Element& value) { storage.push_back(value); }
+    void push_back(T_Element&& value) { storage.push_back(std::move(value)); }
+
+    /// Return an iterable yielding all indices in order
+    index_range_type indices() const
+    {
+        RTTR_Assert(size() < index_type::INVALID);
+        return index_range_type(index_type(size()));
+    }
+};
+
+/// (Iterable) Range yielding DescIdx<T> instances
+template<typename T>
+class IndexRange
+{
+public:
+    using index_type = DescIdx<T>;
+
+    explicit constexpr IndexRange(index_type endValue) : startValue_(0), endValue_(endValue) {}
+
+    class iterator
+    {
+        index_type value;
+
+    public:
+        explicit BOOST_FORCEINLINE constexpr iterator(index_type value) : value(value) {}
+        BOOST_FORCEINLINE constexpr index_type operator*() const noexcept { return value; }
+        BOOST_FORCEINLINE void operator++() noexcept { ++value.value; }
+        BOOST_FORCEINLINE constexpr bool operator!=(iterator rhs) const noexcept { return value != rhs.value; }
+    };
+
+    BOOST_FORCEINLINE constexpr iterator begin() const noexcept { return iterator(startValue_); }
+    BOOST_FORCEINLINE constexpr iterator end() const noexcept { return iterator(endValue_); }
+
+private:
+    const index_type startValue_;
+    const index_type endValue_;
+};

libs/s25client/s25client.cpp

@@ -570,12 +570,12 @@ int main(int argc, char** argv)
         result = e.code;
     } catch(const std::exception& e)
     {
-        bnw::cerr << "An exception occurred: " << e.what() << "\n\n";
+        bnw::cerr << "An exception occurred: " << e.what() << std::endl;
         handleException(nullptr);
         result = 1;
     } catch(...)
     {
-        bnw::cerr << "An unknown exception occurred\n";
+        bnw::cerr << "An unknown exception occurred" << std::endl;
         handleException(nullptr);
         result = 1;
     }

libs/s25main/GameObject.h

@@ -1,4 +1,5 @@
-// Copyright (C) 2005 - 2021 Settlers Freaks (sf-team at siedler25.org)
+// Copyright (C) 2005 - 2024 Settlers Freaks (sf-team at siedler25.org)
+//
 //
 // SPDX-License-Identifier: GPL-2.0-or-later
 
@@ -14,28 +15,31 @@ class GameWorld;
 class EventManager;
 class PostMsg;
 
-/// Basisklasse für alle Spielobjekte
+/// Base class for all game objects
 class GameObject
 {
 public:
     GameObject();
     GameObject(SerializedGameData& sgd, unsigned obj_id);
-    GameObject(const GameObject& go);
     virtual ~GameObject();
+
+protected:
+    // We store GameObject references by address, so they must not be copied
+    explicit GameObject(const GameObject&);
+
+public:
     GameObject& operator=(const GameObject&) = delete;
 
-    /// zerstört das Objekt.
+    /// Handle destruction before deleting the instance
     virtual void Destroy() = 0;
 
-    /// Benachrichtigen, wenn neuer GF erreicht wurde.
+    /// Called for each expiring event (i.e. target GF reached) that this instance has registered
     virtual void HandleEvent(unsigned /*id*/) {}
 
     /// Return the unique ID of an object. Always non-zero!
     unsigned GetObjId() const { return objId; }
 
-    /// Serialisierungsfunktion.
     virtual void Serialize(SerializedGameData& sgd) const = 0;
-    /// Liefert den GOT (siehe oben)
     virtual GO_Type GetGOT() const = 0;
 
     virtual std::string ToString() const;
@@ -57,7 +61,7 @@ class GameObject
     static void DetachWorld(GameWorld* gameWorld);
     /// Return the number of objects alive
     static unsigned GetNumObjs() { return objCounter_; }
-    /// Gibt Obj-ID-Counter zurück
+    /// Return ID counter, i.e. the last object ID used
     static unsigned GetObjIDCounter() { return objIdCounter_; }
     /// Reset the object counter and the object ID counter to 0
     static void ResetCounters()
@@ -73,12 +77,12 @@ class GameObject
     }
 
 protected:
-    /// Zugriff auf übrige Spielwelt
+    /// Access to the currently active game world
     static GameWorld* world;
 
 private:
-    static unsigned objIdCounter_; /// Objekt-ID-Counter (number of objects created)
-    static unsigned objCounter_;   /// Objekt-Counter (number of objects alive)
+    static unsigned objIdCounter_; /// Object-ID-Counter (number of objects created)
+    static unsigned objCounter_;   /// Object-Counter (number of objects alive)
 };
 
 /// Calls destroy on a GameObject and then deletes it setting the ptr to nullptr

libs/s25main/PreviewMinimap.cpp

@@ -44,15 +44,9 @@ void PreviewMinimap::SetMap(const libsiedler2::ArchivItem_Map& s2map)
         LOG.write(_("Failed to load game data!"));
     else
     {
-        DescIdx<LandscapeDesc> lt(0);
-        for(DescIdx<LandscapeDesc> i(0); i.value < worldDesc.landscapes.size(); i.value++)
+        const auto lt = worldDesc.landscapes.find([gfxSet](const LandscapeDesc& l) { return l.s2Id == gfxSet; });
+        for(const TerrainDesc& ter : worldDesc.terrain)
         {
-            if(worldDesc.get(i).s2Id == gfxSet)
-                lt = i;
-        }
-        for(DescIdx<TerrainDesc> i(0); i.value < worldDesc.terrain.size(); i.value++)
-        {
-            const TerrainDesc& ter = worldDesc.get(i);
             if(ter.landscape == lt)
                 terrain2Clr[ter.s2Id] = ter.minimapColor;
         }