explode() and implode() both have comments saying they expect the buffers to be zero-init, however new[] default initialises arrays which for char[] leaves them in an implementation defined state. On MSVC the memory is unitialised. To be safe use std::make_unique<T[]>(size_t) as this value-initialises each element of the array.
I don't think this ever comes up because no spells have an sManaCost of 255, however _pMaxManaBase is stored as fixed point, so it should be shifted before use.
But to be honest, I'm not even sure what it is trying to do here. Was sManaCost == 255 supposed to indicate that it was going to use all the player's mana? Why was it cast to a BYTE before using it? Did they intend to limit the manage usage to 255 and just screwed up? I originally saw this logic in Devilution, and found it because the Middle Earth mod had a slight change here, where it used pMana (maybe because they thought it was intended to be a spell that used all the player's mana?) Also, I saw that in 1.07, Ghidra saw `ma` as a byte, not an int as written in Devilution - I wonder if that's where the cast came from? In the ME Mod version, it sees `ma` as an int.
Also use mod for testing if item dimensions are even. Lets assume the release build can optimise this into a bitmask, probably wont need to do this anyway after a refactor.
This matches the behaviour of inventory cell hit logic. Previously it was possible to click exactly on a border and be unable to put an item in the stash.
This OOB happened when rendering a sprite so that it is exactly
off-screen (touching the border but not visible) on top/bottom
while also being only partly off-screen on the left or right.
ephphatha
staphen
Gleb Mazovetskiy
DakkJaniels
obligaron
Anders Jenbo
Vladimir Olteanu