Check both SPL_NULL and SPL_INVALID to determine spell validity

Source/control.cpp

@@ -737,7 +737,7 @@ void CheckPanelInfo()
 		AddPanelString(_("Hotkey: 's'"));
 		Player &myPlayer = *MyPlayer;
 		const spell_id spellId = myPlayer._pRSpell;
-		if (spellId != SPL_INVALID && spellId != SPL_NULL) {
+		if (IsValidSpell(spellId)) {
 			switch (myPlayer._pRSplType) {
 			case RSPLTYPE_SKILL:
 				AddPanelString(fmt::format(fmt::runtime(_("{:s} Skill")), pgettext("spell", spelldata[spellId].sSkillText)));

Source/controls/modifier_hints.cpp

@@ -131,7 +131,7 @@ void DrawSpellsCircleMenuHint(const Surface &out, const Point &origin)
 	for (int slot = 0; slot < 4; ++slot) {
 		splId = myPlayer._pSplHotKey[slot];
 
-		if (splId != SPL_INVALID && splId != SPL_NULL && (spells & GetSpellBitmask(splId)) != 0)
+		if (IsValidSpell(splId) && (spells & GetSpellBitmask(splId)) != 0)
 			splType = (leveltype == DTYPE_TOWN && !spelldata[splId].sTownSpell) ? RSPLTYPE_INVALID : myPlayer._pSplTHotKey[slot];
 		else {
 			splType = RSPLTYPE_INVALID;

Source/items.cpp

@@ -1868,7 +1868,7 @@ bool SmithItemOk(int i)
 		return false;
 	if (AllItemsList[i].itype == ItemType::Gold)
 		return false;
-	if (AllItemsList[i].itype == ItemType::Staff && (!gbIsHellfire || AllItemsList[i].iSpell != SPL_NULL))
+	if (AllItemsList[i].itype == ItemType::Staff && (!gbIsHellfire || IsValidSpell(AllItemsList[i].iSpell)))
 		return false;
 	if (AllItemsList[i].itype == ItemType::Ring)
 		return false;
@@ -2387,7 +2387,7 @@ void CalcPlrItemVals(Player &player, bool loadgfx)
 			maxd += item._iMaxDam;
 			tac += item._iAC;
 
-			if (item._iSpell != SPL_NULL) {
+			if (IsValidSpell(item._iSpell)) {
 				spl |= GetSpellBitmask(item._iSpell);
 			}
 
@@ -4648,7 +4648,7 @@ void RepairItem(Item &item, int lvl)
 
 void RechargeItem(Item &item, Player &player)
 {
-	if (item._itype != ItemType::Staff || item._iSpell == SPL_NULL)
+	if (item._itype != ItemType::Staff || !IsValidSpell(item._iSpell))
 		return;
 
 	if (item._iCharges == item._iMaxCharges)

Source/panels/spell_book.cpp

@@ -136,7 +136,7 @@ void DrawSpellBook(const Surface &out)
 	const int textPaddingTop = 7;
 	for (int i = 1; i < 8; i++) {
 		spell_id sn = SpellPages[sbooktab][i - 1];
-		if (sn != SPL_INVALID && (spl & GetSpellBitmask(sn)) != 0) {
+		if (IsValidSpell(sn) && (spl & GetSpellBitmask(sn)) != 0) {
 			spell_type st = GetSBookTrans(sn, true);
 			SetSpellTrans(st);
 			const Point spellCellPosition = GetPanelPosition(UiPanels::Spell, { 11, yp + SpellBookDescription.height });
@@ -198,7 +198,7 @@ void CheckSBook()
 		spell_id sn = SpellPages[sbooktab][(MousePosition.y - iconArea.position.y) / SpellBookDescription.height];
 		Player &player = *MyPlayer;
 		uint64_t spl = player._pMemSpells | player._pISpells | player._pAblSpells;
-		if (sn != SPL_INVALID && (spl & GetSpellBitmask(sn)) != 0) {
+		if (IsValidSpell(sn) && (spl & GetSpellBitmask(sn)) != 0) {
 			spell_type st = RSPLTYPE_SPELL;
 			if ((player._pISpells & GetSpellBitmask(sn)) != 0) {
 				st = RSPLTYPE_CHARGES;

Source/panels/spell_list.cpp

@@ -102,18 +102,24 @@ void DrawSpell(const Surface &out)
 	spell_id spl = myPlayer._pRSpell;
 	spell_type st = myPlayer._pRSplType;
 
-	// BUGFIX: Move the next line into the if statement to avoid OOB (SPL_INVALID is -1) (fixed)
-	if (st == RSPLTYPE_SPELL && spl != SPL_INVALID) {
+	if (!IsValidSpell(spl)) {
+		st = RSPLTYPE_INVALID;
+		spl = SPL_NULL;
+	}
+
+	if (st == RSPLTYPE_SPELL) {
 		int tlvl = myPlayer.GetSpellLevel(spl);
 		if (CheckSpell(MyPlayerId, spl, st, true) != SpellCheckResult::Success)
 			st = RSPLTYPE_INVALID;
 		if (tlvl <= 0)
 			st = RSPLTYPE_INVALID;
 	}
+
 	if (leveltype == DTYPE_TOWN && st != RSPLTYPE_INVALID && !spelldata[spl].sTownSpell)
 		st = RSPLTYPE_INVALID;
+
 	SetSpellTrans(st);
-	const int nCel = (spl != SPL_INVALID) ? SpellITbl[spl] : 26;
+	const int nCel = SpellITbl[spl];
 	const Point position = GetMainPanel().position + Displacement { 565, 119 };
 	DrawSpellCel(out, position, nCel);
 
@@ -301,7 +307,8 @@ void ToggleSpell(size_t slot)
 
 	Player &myPlayer = *MyPlayer;
 
-	if (myPlayer._pSplHotKey[slot] == SPL_INVALID) {
+	const spell_id spellId = myPlayer._pSplHotKey[slot];
+	if (!IsValidSpell(spellId)) {
 		return;
 	}
 
@@ -322,8 +329,7 @@ void ToggleSpell(size_t slot)
 		return;
 	}
 
-	const spell_id spellId = myPlayer._pSplHotKey[slot];
-	if (spellId != SPL_INVALID && spellId != SPL_NULL && (spells & GetSpellBitmask(spellId)) != 0) {
+	if ((spells & GetSpellBitmask(spellId)) != 0) {
 		myPlayer._pRSpell = spellId;
 		myPlayer._pRSplType = myPlayer._pSplTHotKey[slot];
 		force_redraw = 255;
@@ -341,7 +347,7 @@ void DoSpeedBook()
 
 	Player &myPlayer = *MyPlayer;
 
-	if (myPlayer._pRSpell != SPL_INVALID) {
+	if (IsValidSpell(myPlayer._pRSpell)) {
 		for (int i = RSPLTYPE_SKILL; i <= RSPLTYPE_CHARGES; i++) {
 			uint64_t spells;
 			switch (i) {

Source/player.cpp

@@ -2046,6 +2046,16 @@ void Player::RestorePartialMana()
 	}
 }
 
+void Player::ReadySpellFromEquipment(inv_body_loc bodyLocation)
+{
+	auto &item = InvBody[bodyLocation];
+	if (item._itype == ItemType::Staff && IsValidSpell(item._iSpell) && item._iCharges > 0) {
+		_pRSpell = item._iSpell;
+		_pRSplType = RSPLTYPE_CHARGES;
+		force_redraw = 255;
+	}
+}
+
 void Player::UpdatePreviewCelSprite(_cmd_id cmdId, Point point, uint16_t wParam1, uint16_t wParam2)
 {
 	// if game is not running don't show a preview
@@ -3475,7 +3485,7 @@ void CheckPlrSpell(bool isShiftHeld, spell_id spellID, spell_type spellType)
 	}
 	Player &myPlayer = *MyPlayer;
 
-	if (spellID == SPL_INVALID) {
+	if (!IsValidSpell(spellID)) {
 		myPlayer.Say(HeroSpeech::IDontHaveASpellReady);
 		return;
 	}

Source/player.h

@@ -668,15 +668,7 @@ struct Player {
 	 * @brief Sets the readied spell to the spell in the specified equipment slot. Does nothing if the item does not have a valid spell.
 	 * @param bodyLocation - the body location whose item will be checked for the spell.
 	 */
-	void ReadySpellFromEquipment(inv_body_loc bodyLocation)
-	{
-		auto &item = InvBody[bodyLocation];
-		if (item._itype == ItemType::Staff && item._iSpell != SPL_NULL && item._iCharges > 0) {
-			_pRSpell = item._iSpell;
-			_pRSplType = RSPLTYPE_CHARGES;
-			force_redraw = 255;
-		}
-	}
+	void ReadySpellFromEquipment(inv_body_loc bodyLocation);
 
 	/**
 	 * @brief Does the player currently have a ranged weapon equipped?

Source/spells.cpp

@@ -107,6 +107,13 @@ void PlacePlayer(int pnum)
 
 } // namespace
 
+bool IsValidSpell(spell_id spl)
+{
+	return spl > SPL_NULL
+	    && spl <= SPL_LAST
+	    && (spl <= SPL_LASTDIABLO || gbIsHellfire);
+}
+
 bool IsWallSpell(spell_id spl)
 {
 	return spl == SPL_FIREWALL || spl == SPL_LIGHTWALL;

Source/spells.h

@@ -16,6 +16,7 @@ enum class SpellCheckResult : uint8_t {
 	Fail_Busy,
 };
 
+bool IsValidSpell(spell_id spl);
 bool IsWallSpell(spell_id spl);
 bool TargetsMonster(spell_id id);
 int GetManaAmount(const Player &player, spell_id sn);

Source/stores.cpp

@@ -501,7 +501,7 @@ bool SmithSellOk(int i)
 		return false;
 	if (pI->_itype == ItemType::Gold)
 		return false;
-	if (pI->_itype == ItemType::Staff && (!gbIsHellfire || pI->_iSpell != SPL_NULL))
+	if (pI->_itype == ItemType::Staff && (!gbIsHellfire || IsValidSpell(pI->_iSpell)))
 		return false;
 	if (pI->_iClass == ICLASS_QUEST)
 		return false;
@@ -798,7 +798,7 @@ bool WitchSellOk(int i)
 		rv = false;
 	if (pI->_iClass == ICLASS_QUEST)
 		rv = false;
-	if (pI->_itype == ItemType::Staff && (!gbIsHellfire || pI->_iSpell != SPL_NULL))
+	if (pI->_itype == ItemType::Staff && (!gbIsHellfire || IsValidSpell(pI->_iSpell)))
 		rv = true;
 	if (pI->IDidx >= IDI_FIRSTQUEST && pI->IDidx <= IDI_LASTQUEST)
 		rv = false;