From 2f7bac0b97dfab42cbc88ce152fb41ca0d884799 Mon Sep 17 00:00:00 2001
From: morfidon <57798071+morfidon@users.noreply.github.com>
Date: Sat, 7 Mar 2026 10:07:15 +0100
Subject: [PATCH] Refactor autosave: centralize decision logic and improve
 readability

- Add RequestAutoSave() wrapper function with centralized filter logic
- Add HasPendingAutoSave() helper for better code readability
- Replace direct QueueAutoSave() calls with RequestAutoSave()
- Consolidate multiplayer and enabled checks in one place
- Improve code maintainability and separation of concerns
---
 Source/diablo.cpp   | 22 +++++++++++++++++++---
 Source/diablo.h     |  2 ++
 Source/gamemenu.cpp |  2 +-
 Source/inv.cpp      | 28 ++++++++++++++--------------
 Source/monster.cpp  |  8 ++++----
 5 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/Source/diablo.cpp b/Source/diablo.cpp
index cfca7ea35..b1b4c921f 100644
--- a/Source/diablo.cpp
+++ b/Source/diablo.cpp
@@ -1624,7 +1624,7 @@ void GameLogic()
 		pendingAutoSaveReason = AutoSaveReason::None;
 	}
 
-	if (pendingAutoSaveReason != AutoSaveReason::None && IsAutoSaveSafe()) {
+	if (HasPendingAutoSave() && IsAutoSaveSafe()) {
 		if (AttemptAutoSave(pendingAutoSaveReason)) {
 			pendingAutoSaveReason = AutoSaveReason::None;
 			autoSaveNextTimerDueAt = SDL_GetTicks() + GetAutoSaveIntervalMilliseconds();
@@ -1959,11 +1959,27 @@ int GetSecondsUntilNextAutoSave()
 	return static_cast<int>((remainingMilliseconds + 999) / 1000);
 }
 
-bool IsAutoSavePending()
+bool HasPendingAutoSave()
 {
 	return pendingAutoSaveReason != AutoSaveReason::None;
 }
 
+void RequestAutoSave(AutoSaveReason reason)
+{
+	if (!*GetOptions().Gameplay.autoSaveEnabled)
+		return;
+
+	if (gbIsMultiplayer)
+		return;
+
+	QueueAutoSave(reason);
+}
+
+bool IsAutoSavePending()
+{
+	return HasPendingAutoSave();
+}
+
 void QueueAutoSave(AutoSaveReason reason)
 {
 	if (gbIsMultiplayer)
@@ -3629,7 +3645,7 @@ tl::expected<void, std::string> LoadGameLevel(bool firstflag, lvl_entry lvldir)
 
 	LoadGameLevelCalculateCursor();
 	if (leveltype == DTYPE_TOWN && lvldir != ENTRY_LOAD && !firstflag)
-		::devilution::QueueAutoSave(AutoSaveReason::TownEntry);
+		::devilution::RequestAutoSave(AutoSaveReason::TownEntry);
 	return {};
 }
 
diff --git a/Source/diablo.h b/Source/diablo.h
index 539211a61..5b47bc9c9 100644
--- a/Source/diablo.h
+++ b/Source/diablo.h
@@ -112,7 +112,9 @@ bool IsDiabloAlive(bool playSFX);
 void MarkCombatActivity();
 bool IsAutoSaveSafe();
 int GetSecondsUntilNextAutoSave();
+bool HasPendingAutoSave();
 bool IsAutoSavePending();
+void RequestAutoSave(AutoSaveReason reason);
 void QueueAutoSave(AutoSaveReason reason);
 bool AttemptAutoSave(AutoSaveReason reason);
 void PrintScreen(SDL_Keycode vkey);
diff --git a/Source/gamemenu.cpp b/Source/gamemenu.cpp
index 64ba95eaf..ce993a238 100644
--- a/Source/gamemenu.cpp
+++ b/Source/gamemenu.cpp
@@ -106,7 +106,7 @@ void GamemenuUpdateSingle()
 
 const char *GetSaveGameMenuLabel()
 {
-	if (IsAutoSavePending()) {
+	if (HasPendingAutoSave()) {
 		saveGameMenuLabel = fmt::format(fmt::runtime(_("Save Game ({:s})")), _("ready"));
 		return saveGameMenuLabel.c_str();
 	}
diff --git a/Source/inv.cpp b/Source/inv.cpp
index 528f670db..202ecd6e7 100644
--- a/Source/inv.cpp
+++ b/Source/inv.cpp
@@ -1692,14 +1692,14 @@ void InvGetItem(Player &player, int ii)
 		NewCursor(player.HoldItem);
 	}
 
-	const bool pickedUniqueItem = &player == MyPlayer && item._iMagical == ITEM_QUALITY_UNIQUE;
-
-	// This potentially moves items in memory so must be done after we've made a copy
-	CleanupItems(ii);
-	if (pickedUniqueItem)
-		QueueAutoSave(AutoSaveReason::UniquePickup);
-	pcursitem = -1;
-}
+	const bool pickedUniqueItem = &player == MyPlayer && item._iMagical == ITEM_QUALITY_UNIQUE;
+
+	// This potentially moves items in memory so must be done after we've made a copy
+	CleanupItems(ii);
+	if (pickedUniqueItem)
+		RequestAutoSave(AutoSaveReason::UniquePickup);
+	pcursitem = -1;
+}
 
 std::optional<Point> FindAdjacentPositionForItem(Point origin, Direction facing)
 {
@@ -1776,12 +1776,12 @@ void AutoGetItem(Player &player, Item *itemPointer, int ii)
 			PlaySFX(SfxID::GrabItem);
 		}
 
-		const bool pickedUniqueItem = &player == MyPlayer && item._iMagical == ITEM_QUALITY_UNIQUE;
-		CleanupItems(ii);
-		if (pickedUniqueItem)
-			QueueAutoSave(AutoSaveReason::UniquePickup);
-		return;
-	}
+		const bool pickedUniqueItem = &player == MyPlayer && item._iMagical == ITEM_QUALITY_UNIQUE;
+		CleanupItems(ii);
+		if (pickedUniqueItem)
+			RequestAutoSave(AutoSaveReason::UniquePickup);
+		return;
+	}
 
 	if (&player == MyPlayer) {
 		player.Say(HeroSpeech::ICantCarryAnymore);
diff --git a/Source/monster.cpp b/Source/monster.cpp
index 0463e6581..5a935cda0 100644
--- a/Source/monster.cpp
+++ b/Source/monster.cpp
@@ -4024,10 +4024,10 @@ void MonsterDeath(Monster &monster, Direction md, bool sendmsg)
 	monster.position.future = monster.position.old;
 	M_ClearSquares(monster);
 	monster.occupyTile(monster.position.tile, false);
-	CheckQuestKill(monster, sendmsg);
-	if (!gbIsMultiplayer && IsAnyOf(monster.type().type, MT_CLEAVER, MT_SKING, MT_DIABLO, MT_DEFILER, MT_NAKRUL))
-		QueueAutoSave(AutoSaveReason::BossKill);
-	M_FallenFear(monster.position.tile);
+	CheckQuestKill(monster, sendmsg);
+	if (!gbIsMultiplayer && IsAnyOf(monster.type().type, MT_CLEAVER, MT_SKING, MT_DIABLO, MT_DEFILER, MT_NAKRUL))
+		RequestAutoSave(AutoSaveReason::BossKill);
+	M_FallenFear(monster.position.tile);
 	if (IsAnyOf(monster.type().type, MT_NACID, MT_RACID, MT_BACID, MT_XACID, MT_SPIDLORD))
 		AddMissile(monster.position.tile, { 0, 0 }, Direction::South, MissileID::AcidPuddle, TARGET_PLAYERS, monster, monster.intelligence + 1, 0);
 }