* Change defines to constexpr int in player.h
* Add const or constexpr to player.h/cpp where applicable
* Update tests with changed names of player constatns
* remove unecessary variable
* AnimationInfo class correct case-style for private members
clang-tidy warnings:
readability-identifier-naming invalid case style for private member
* AnimationInfo class correct case-style for members
clang-tidy warnings:
readability-identifier-naming invalid case style for member
* AnimationInfo class correct case-style for methods
clang-tidy warnings:
readability-identifier-naming invalid case style for method
* AnimationInfo class use nodiscard
clang-tidy warnings:
modernize-use-nodiscard function should be marked [[nodiscard]]
1. Do not modify the map after loading. Instead, return string views
(guaranteed to be null-terminated) from look up functions and return
the key directly if not found.
2. Use an `unorded_map` instead of `map` where available (C++20).
Saves a bit of RAM (~50 KiB) and improves lookup performance.
Previously this was falling back to FunctionCase, leading to inconsistent casing of class methods throughout the codebase. Applied to Item as an example.
Viewing a stash page results in an entry being created in the stashGrids map. If the player flicks through the entire stash without actually storing an item they end up with ~20kb of unnecessary data in the stash file when they next save.
Most of this was centred around stash pages being saved/loaded as unsigned values but stored as signed values. Consistently used unsigned since it matches the intended usage.
Makes `CelSprite` unowned and adds a new `OwnedCelSprite` class for
owned sprites.
This clarifies ownership and makes the code cleaner in a number of
places.
Additionally, because the `CelSprite` class is now tiny (1 less
pointer), we can pass it by-value instead of by-reference, removing a
pointer indirection in the rendering functions.
This was a cleanup function to handle converting saves between Hellfire <-> Diablo, mainly when a hellfire specific item was dropped and should be removed from a Diablo game. This had an off by one with the way it iterated backwards over the active items list which meant it always called DeleteItem at least once (since it uses an invalid item past the end of the active items range). DeleteItem then always decremented the ActiveItems count. When saving the game after converting the items we then end up with a dropped items array containing an index to an item that is no longer considered active.
However the code in LoadDroppedItems already checks that the conversion was successful before considering it active, so this function doesn't need to be called. Instead we can just save if we converted game mode.
This doesn't handle failed allocations (e.g. if the platform runs out of memory) but makes it easier to use a fixed size container on limited memory devices.
This highlights how all (except one) call sites duplicate the i argument. With the exception of the use in msg.cpp this can be treated as a offset into activeItems and doesn't care about the real index.
Mikołaj Piróg