Browse Source

Refine spell load validation

Expose ValidatePlayer directly instead of routing load-time validation through a one-off wrapper.
Add a persisted-state regression test that rewrites a Hellfire-origin save under Diablo and verifies invalid spell selections stay cleared after saving and reloading hotkeys.
pull/8507/head
morfidon 3 weeks ago
parent
commit
2e24433d30
  1. 4
      Source/loadsave.cpp
  2. 19
      Source/player.cpp
  3. 1
      Source/player.h
  4. 80
      test/writehero_test.cpp

4
Source/loadsave.cpp

@ -50,8 +50,6 @@ namespace devilution {
bool gbIsHellfireSaveGame;
uint8_t giNumberOfLevels;
void ValidatePlayerForLoad();
namespace {
constexpr size_t MaxMissilesForSaveGame = 125;
@ -2521,7 +2519,7 @@ tl::expected<void, std::string> LoadGame(bool firstflag)
Player &myPlayer = *MyPlayer;
LoadPlayer(file, myPlayer);
ValidatePlayerForLoad();
ValidatePlayer();
CalcPlrInv(myPlayer, false);
SanitizePlayerSpellSelections(myPlayer);

19
Source/player.cpp

@ -1410,6 +1410,8 @@ bool PlrDeathModeOK(Player &player)
return false;
}
} // namespace
void ValidatePlayer()
{
assert(MyPlayer != nullptr);
@ -1467,6 +1469,8 @@ void ValidatePlayer()
myPlayer._pInfraFlag = false;
}
namespace {
HeroClass GetPlayerSpriteClass(HeroClass cls)
{
if (cls == HeroClass::Bard && !HaveBardAssets())
@ -1528,16 +1532,11 @@ void GetPlayerGraphicsPath(std::string_view path, std::string_view prefix, std::
*BufCopy(out, "plrgfx\\", path, "\\", prefix, "\\", prefix, type) = '\0';
}
} // namespace
void ValidatePlayerForLoad()
{
ValidatePlayer();
}
void Player::CalcScrolls()
{
_pScrlSpells = 0;
} // namespace
void Player::CalcScrolls()
{
_pScrlSpells = 0;
for (const Item &item : InventoryAndBeltPlayerItemsRange { *this }) {
if (item.isScroll() && item._iStatFlag) {
_pScrlSpells |= GetSpellBitmask(item._iSpell);

1
Source/player.h

@ -985,6 +985,7 @@ void CheckPlrSpell(bool isShiftHeld, SpellID spellID = MyPlayer->_pRSpell, Spell
void SyncPlrAnim(Player &player);
void SyncInitPlrPos(Player &player);
void SyncInitPlr(Player &player);
void ValidatePlayer();
void CheckStats(Player &player);
void ModifyPlrStr(Player &player, int l);
void ModifyPlrMag(Player &player, int l);

80
test/writehero_test.cpp

@ -12,6 +12,7 @@
#include "game_mode.hpp"
#include "init.hpp"
#include "loadsave.h"
#include "menu.h"
#include "pack.h"
#include "pfile.h"
#include "spells.h"
@ -21,6 +22,9 @@
#include "utils/paths.h"
namespace devilution {
uint32_t gSaveNumber = 0;
namespace {
constexpr int SpellDatVanilla[] = {
@ -536,5 +540,81 @@ TEST(Writehero, pfile_read_player_from_save_preserves_valid_spell_selections)
EXPECT_EQ(player._pRSplType, SpellType::Spell);
}
TEST(Writehero, DiabloRewritePersistsSanitizedSpellSelectionsFromHellfireSave)
{
LoadCoreArchives();
LoadGameArchives();
if (!HaveMainData()) {
GTEST_SKIP() << "MPQ assets (spawn.mpq or DIABDAT.MPQ) not found - skipping test";
}
const std::string savePath = paths::BasePath() + "multi_0.sv";
const std::string hellfireSavePath = paths::BasePath() + "multi_0.hsv";
paths::SetPrefPath(paths::BasePath());
RemoveFile(savePath.c_str());
RemoveFile(hellfireSavePath.c_str());
gbVanilla = false;
gbIsSpawn = false;
gbIsMultiplayer = true;
leveltype = DTYPE_TOWN;
currlevel = 0;
ViewPosition = {};
giNumberOfLevels = 25;
gbIsHellfire = true;
gbIsHellfireSaveGame = true;
Players.resize(1);
MyPlayerId = 0;
MyPlayer = &Players[MyPlayerId];
LoadSpellData();
LoadPlayerDataFiles();
LoadMonsterData();
LoadItemData();
_uiheroinfo info {};
info.heroclass = HeroClass::Rogue;
pfile_ui_save_create(&info);
gSaveNumber = info.saveNumber;
Player &player = *MyPlayer;
player._pMemSpells = GetSpellBitmask(SpellID::Healing) | GetSpellBitmask(SpellID::Apocalypse);
player._pSplLvl[static_cast<size_t>(SpellID::Healing)] = 1;
player._pSplLvl[static_cast<size_t>(SpellID::Apocalypse)] = 1;
player._pSplHotKey[0] = SpellID::Apocalypse;
player._pSplTHotKey[0] = SpellType::Spell;
player._pSplHotKey[1] = SpellID::Healing;
player._pSplTHotKey[1] = SpellType::Spell;
player._pRSpell = SpellID::Apocalypse;
player._pRSplType = SpellType::Spell;
pfile_write_hero(/*writeGameData=*/true);
RenameFile(hellfireSavePath.c_str(), savePath.c_str());
gbIsHellfire = false;
gbIsHellfireSaveGame = false;
giNumberOfLevels = 17;
pfile_read_player_from_save(info.saveNumber, player);
pfile_write_hero();
player._pSplHotKey[0] = SpellID::Apocalypse;
player._pSplTHotKey[0] = SpellType::Spell;
player._pSplHotKey[1] = SpellID::Invalid;
player._pSplTHotKey[1] = SpellType::Invalid;
player._pRSpell = SpellID::Apocalypse;
player._pRSplType = SpellType::Spell;
LoadHotkeys();
EXPECT_EQ(player._pSplHotKey[0], SpellID::Invalid);
EXPECT_EQ(player._pSplTHotKey[0], SpellType::Invalid);
EXPECT_EQ(player._pSplHotKey[1], SpellID::Healing);
EXPECT_EQ(player._pSplTHotKey[1], SpellType::Spell);
EXPECT_EQ(player._pRSpell, SpellID::Invalid);
EXPECT_EQ(player._pRSplType, SpellType::Invalid);
}
} // namespace
} // namespace devilution

Loading…
Cancel
Save