Support `const` inventory iterators

Previously, inventory iterators could only be used with non-const
objects, leading to suboptimal const correctness.

Adds support for `const` objects to inventory iterators via
C++17 CTAD.

Source/control.cpp

@@ -1028,7 +1028,7 @@ void CheckPanelInfo()
 		InfoColor = UiFlags::ColorWhite;
 		panelflag = true;
 		AddPanelString(_("Hotkey: 's'"));
-		Player &myPlayer = *MyPlayer;
+		const Player &myPlayer = *MyPlayer;
 		const SpellID spellId = myPlayer._pRSpell;
 		if (IsValidSpell(spellId)) {
 			switch (myPlayer._pRSplType) {

Source/inv.h

@@ -245,7 +245,7 @@ Size GetInventorySize(const Item &item);
  * @brief Checks whether the player has an inventory item matching the predicate.
  */
 template <typename Predicate>
-bool HasInventoryItem(Player &player, Predicate &&predicate)
+bool HasInventoryItem(const Player &player, Predicate &&predicate)
 {
 	const InventoryPlayerItemsRange items { player };
 	return c_find_if(items, std::forward<Predicate>(predicate)) != items.end();
@@ -255,7 +255,7 @@ bool HasInventoryItem(Player &player, Predicate &&predicate)
  * @brief Checks whether the player has a belt item matching the predicate.
  */
 template <typename Predicate>
-bool HasBeltItem(Player &player, Predicate &&predicate)
+bool HasBeltItem(const Player &player, Predicate &&predicate)
 {
 	const BeltPlayerItemsRange items { player };
 	return c_find_if(items, std::forward<Predicate>(predicate)) != items.end();
@@ -265,7 +265,7 @@ bool HasBeltItem(Player &player, Predicate &&predicate)
  * @brief Checks whether the player has an inventory or a belt item matching the predicate.
  */
 template <typename Predicate>
-bool HasInventoryOrBeltItem(Player &player, Predicate &&predicate)
+bool HasInventoryOrBeltItem(const Player &player, Predicate &&predicate)
 {
 	return HasInventoryItem(player, predicate) || HasBeltItem(player, predicate);
 }
@@ -273,7 +273,7 @@ bool HasInventoryOrBeltItem(Player &player, Predicate &&predicate)
 /**
  * @brief Checks whether the player has an inventory item with the given ID (IDidx).
  */
-inline bool HasInventoryItemWithId(Player &player, _item_indexes id)
+inline bool HasInventoryItemWithId(const Player &player, _item_indexes id)
 {
 	return HasInventoryItem(player, [id](const Item &item) {
 		return item.IDidx == id;
@@ -283,7 +283,7 @@ inline bool HasInventoryItemWithId(Player &player, _item_indexes id)
 /**
  * @brief Checks whether the player has a belt item with the given ID (IDidx).
  */
-inline bool HasBeltItemWithId(Player &player, _item_indexes id)
+inline bool HasBeltItemWithId(const Player &player, _item_indexes id)
 {
 	return HasBeltItem(player, [id](const Item &item) {
 		return item.IDidx == id;
@@ -293,7 +293,7 @@ inline bool HasBeltItemWithId(Player &player, _item_indexes id)
 /**
  * @brief Checks whether the player has an inventory or a belt item with the given ID (IDidx).
  */
-inline bool HasInventoryOrBeltItemWithId(Player &player, _item_indexes id)
+inline bool HasInventoryOrBeltItemWithId(const Player &player, _item_indexes id)
 {
 	return HasInventoryItemWithId(player, id) || HasBeltItemWithId(player, id);
 }

Source/inv_iterators.hpp

@@ -2,6 +2,7 @@
 
 #include <cstddef>
 #include <iterator>
+#include <type_traits>
 #include <utility>
 #include <vector>
 
@@ -13,19 +14,23 @@ namespace devilution {
 /**
  * @brief A range over non-empty items in a container.
  */
+template <typename ItemT>
 class ItemsContainerRange {
+	static_assert(std::is_same_v<ItemT, Item> || std::is_same_v<ItemT, const Item>,
+	    "The template argument must be `Item` or `const Item`");
+
 public:
 	class Iterator {
 	public:
 		using iterator_category = std::forward_iterator_tag;
 		using difference_type = int;
-		using value_type = Item;
+		using value_type = ItemT;
 		using pointer = value_type *;
 		using reference = value_type &;
 
 		Iterator() = default;
 
-		Iterator(Item *items, std::size_t count, std::size_t index)
+		Iterator(ItemT *items, std::size_t count, std::size_t index)
 		    : items_(items)
 		    , count_(count)
 		    , index_(index)
@@ -85,12 +90,12 @@ public:
 			}
 		}
 
-		Item *items_ = nullptr;
+		ItemT *items_ = nullptr;
 		std::size_t count_ = 0;
 		std::size_t index_ = 0;
 	};
 
-	ItemsContainerRange(Item *items, std::size_t count)
+	ItemsContainerRange(ItemT *items, std::size_t count)
 	    : items_(items)
 	    , count_(count)
 	{
@@ -107,26 +112,30 @@ public:
 	}
 
 private:
-	Item *items_;
+	ItemT *items_;
 	std::size_t count_;
 };
 
 /**
  * @brief A range over non-empty items in a list of containers.
  */
+template <typename ItemT>
 class ItemsContainerListRange {
+	static_assert(std::is_same_v<ItemT, Item> || std::is_same_v<ItemT, const Item>,
+	    "The template argument must be `Item` or `const Item`");
+
 public:
 	class Iterator {
 	public:
 		using iterator_category = std::forward_iterator_tag;
 		using difference_type = int;
-		using value_type = Item;
+		using value_type = ItemT;
 		using pointer = value_type *;
 		using reference = value_type &;
 
 		Iterator() = default;
 
-		explicit Iterator(std::vector<ItemsContainerRange::Iterator> iterators)
+		explicit Iterator(std::vector<typename ItemsContainerRange<ItemT>::Iterator> iterators)
 		    : iterators_(std::move(iterators))
 		{
 			advancePastEmpty();
@@ -173,7 +182,7 @@ public:
 			}
 		}
 
-		std::vector<ItemsContainerRange::Iterator> iterators_;
+		std::vector<typename ItemsContainerRange<ItemT>::Iterator> iterators_;
 		std::size_t current_ = 0;
 	};
 };
@@ -181,21 +190,27 @@ public:
 /**
  * @brief A range over equipped player items.
  */
+template <typename PlayerT>
 class EquippedPlayerItemsRange {
+	static_assert(std::is_same_v<PlayerT, Player> || std::is_same_v<PlayerT, const Player>,
+	    "The template argument must be `Player` or `const Player`");
+	using ItemT = std::conditional_t<std::is_const_v<PlayerT>, const Item, Item>;
+	using Iterator = typename ItemsContainerRange<ItemT>::Iterator;
+
 public:
-	explicit EquippedPlayerItemsRange(Player &player)
+	explicit EquippedPlayerItemsRange(PlayerT &player)
 	    : player_(&player)
 	{
 	}
 
-	[[nodiscard]] ItemsContainerRange::Iterator begin() const
+	[[nodiscard]] Iterator begin() const
 	{
-		return ItemsContainerRange::Iterator { &player_->InvBody[0], containerSize(), 0 };
+		return Iterator { &player_->InvBody[0], containerSize(), 0 };
 	}
 
-	[[nodiscard]] ItemsContainerRange::Iterator end() const
+	[[nodiscard]] Iterator end() const
 	{
-		return ItemsContainerRange::Iterator { nullptr, containerSize(), containerSize() };
+		return Iterator { nullptr, containerSize(), containerSize() };
 	}
 
 private:
@@ -204,27 +219,33 @@ private:
 		return sizeof(player_->InvBody) / sizeof(player_->InvBody[0]);
 	}
 
-	Player *player_;
+	PlayerT *player_;
 };
 
 /**
  * @brief A range over non-equipped inventory player items.
  */
+template <typename PlayerT>
 class InventoryPlayerItemsRange {
+	static_assert(std::is_same_v<PlayerT, Player> || std::is_same_v<PlayerT, const Player>,
+	    "The template argument must be `Player` or `const Player`");
+	using ItemT = std::conditional_t<std::is_const_v<PlayerT>, const Item, Item>;
+	using Iterator = typename ItemsContainerRange<ItemT>::Iterator;
+
 public:
-	explicit InventoryPlayerItemsRange(Player &player)
+	explicit InventoryPlayerItemsRange(PlayerT &player)
 	    : player_(&player)
 	{
 	}
 
-	[[nodiscard]] ItemsContainerRange::Iterator begin() const
+	[[nodiscard]] Iterator begin() const
 	{
-		return ItemsContainerRange::Iterator { &player_->InvList[0], containerSize(), 0 };
+		return Iterator { &player_->InvList[0], containerSize(), 0 };
 	}
 
-	[[nodiscard]] ItemsContainerRange::Iterator end() const
+	[[nodiscard]] Iterator end() const
 	{
-		return ItemsContainerRange::Iterator { nullptr, containerSize(), containerSize() };
+		return Iterator { nullptr, containerSize(), containerSize() };
 	}
 
 private:
@@ -233,27 +254,33 @@ private:
 		return static_cast<std::size_t>(player_->_pNumInv);
 	}
 
-	Player *player_;
+	PlayerT *player_;
 };
 
 /**
  * @brief A range over belt player items.
  */
+template <typename PlayerT>
 class BeltPlayerItemsRange {
+	static_assert(std::is_same_v<PlayerT, Player> || std::is_same_v<PlayerT, const Player>,
+	    "The template argument must be `Player` or `const Player`");
+	using ItemT = std::conditional_t<std::is_const_v<PlayerT>, const Item, Item>;
+	using Iterator = typename ItemsContainerRange<ItemT>::Iterator;
+
 public:
-	explicit BeltPlayerItemsRange(Player &player)
+	explicit BeltPlayerItemsRange(PlayerT &player)
 	    : player_(&player)
 	{
 	}
 
-	[[nodiscard]] ItemsContainerRange::Iterator begin() const
+	[[nodiscard]] Iterator begin() const
 	{
-		return ItemsContainerRange::Iterator { &player_->SpdList[0], containerSize(), 0 };
+		return Iterator { &player_->SpdList[0], containerSize(), 0 };
 	}
 
-	[[nodiscard]] ItemsContainerRange::Iterator end() const
+	[[nodiscard]] Iterator end() const
 	{
-		return ItemsContainerRange::Iterator { nullptr, containerSize(), containerSize() };
+		return Iterator { nullptr, containerSize(), containerSize() };
 	}
 
 private:
@@ -262,61 +289,73 @@ private:
 		return sizeof(player_->SpdList) / sizeof(player_->SpdList[0]);
 	}
 
-	Player *player_;
+	PlayerT *player_;
 };
 
 /**
  * @brief A range over non-equipped player items in the following order: Inventory, Belt.
  */
+template <typename PlayerT>
 class InventoryAndBeltPlayerItemsRange {
+	static_assert(std::is_same_v<PlayerT, Player> || std::is_same_v<PlayerT, const Player>,
+	    "The template argument must be `Player` or `const Player`");
+	using ItemT = std::conditional_t<std::is_const_v<PlayerT>, const Item, Item>;
+	using Iterator = typename ItemsContainerListRange<ItemT>::Iterator;
+
 public:
-	explicit InventoryAndBeltPlayerItemsRange(Player &player)
+	explicit InventoryAndBeltPlayerItemsRange(PlayerT &player)
 	    : player_(&player)
 	{
 	}
 
-	[[nodiscard]] ItemsContainerListRange::Iterator begin() const
+	[[nodiscard]] Iterator begin() const
 	{
-		return ItemsContainerListRange::Iterator({
+		return Iterator({
 		    InventoryPlayerItemsRange(*player_).begin(),
 		    BeltPlayerItemsRange(*player_).begin(),
 		});
 	}
 
-	[[nodiscard]] ItemsContainerListRange::Iterator end() const
+	[[nodiscard]] Iterator end() const
 	{
-		return ItemsContainerListRange::Iterator({
+		return Iterator({
 		    InventoryPlayerItemsRange(*player_).end(),
 		    BeltPlayerItemsRange(*player_).end(),
 		});
 	}
 
 private:
-	Player *player_;
+	PlayerT *player_;
 };
 
 /**
  * @brief A range over non-empty player items in the following order: Equipped, Inventory, Belt.
  */
+template <typename PlayerT>
 class PlayerItemsRange {
+	static_assert(std::is_same_v<PlayerT, Player> || std::is_same_v<PlayerT, const Player>,
+	    "The template argument must be `Player` or `const Player`");
+	using ItemT = std::conditional_t<std::is_const_v<PlayerT>, const Item, Item>;
+	using Iterator = typename ItemsContainerListRange<ItemT>::Iterator;
+
 public:
-	explicit PlayerItemsRange(Player &player)
+	explicit PlayerItemsRange(PlayerT &player)
 	    : player_(&player)
 	{
 	}
 
-	[[nodiscard]] ItemsContainerListRange::Iterator begin() const
+	[[nodiscard]] Iterator begin() const
 	{
-		return ItemsContainerListRange::Iterator({
+		return Iterator({
 		    EquippedPlayerItemsRange(*player_).begin(),
 		    InventoryPlayerItemsRange(*player_).begin(),
 		    BeltPlayerItemsRange(*player_).begin(),
 		});
 	}
 
-	[[nodiscard]] ItemsContainerListRange::Iterator end() const
+	[[nodiscard]] Iterator end() const
 	{
-		return ItemsContainerListRange::Iterator({
+		return Iterator({
 		    EquippedPlayerItemsRange(*player_).end(),
 		    InventoryPlayerItemsRange(*player_).end(),
 		    BeltPlayerItemsRange(*player_).end(),
@@ -324,7 +363,7 @@ public:
 	}
 
 private:
-	Player *player_;
+	PlayerT *player_;
 };
 
 } // namespace devilution

Source/panels/spell_list.cpp

@@ -116,7 +116,7 @@ void DrawSpellList(const Surface &out)
 {
 	InfoString = StringOrView {};
 
-	Player &myPlayer = *MyPlayer;
+	const Player &myPlayer = *MyPlayer;
 
 	for (auto &spellListItem : GetSpellListItems()) {
 		const SpellID spellId = spellListItem.id;