Refactoring

Author: yuripourre

Source/controls/local_coop/local_coop.cpp

@@ -122,9 +122,6 @@ struct LocalCoopState {
 	/// Player 1 (index 0) is always marked as active when local coop is enabled
 	std::array<LocalCoopPlayer, MaxLocalPlayers> players;
 
-	/// D-pad repeat delay in milliseconds
-	static constexpr uint32_t DpadRepeatDelay = 300;
-
 	/// Panel ownership: which game player ID controls the currently open panels
 	/// 0 = player 1 (default), 1-3 = local coop players 2-4
 	/// Use -1 to indicate no explicit owner (falls back to player 1)
@@ -159,14 +156,6 @@ struct LocalCoopState {
 	int64_t cameraSmoothScreenX = 0;
 	int64_t cameraSmoothScreenY = 0;
 
-	/// Dead zone radius in screen pixels - camera won't move if average player position
-	/// is within this distance from the current camera target
-	static constexpr int CameraDeadZone = 32;
-
-	/// Camera smoothing factor (0.0 = no smoothing, 1.0 = instant)
-	/// Higher values = faster camera response but potentially jerky movement
-	static constexpr float CameraSmoothFactor = 0.25f;
-
 	/// Check if a local coop player owns the panels (not player 1)
 	[[nodiscard]] bool HasPanelOwner() const { return panelOwnerPlayerId > 0; }
 
@@ -209,11 +198,6 @@ struct LocalCoopState {
 	[[nodiscard]] LocalCoopPlayer *GetPlayer(uint8_t playerId);
 	[[nodiscard]] const LocalCoopPlayer *GetPlayer(uint8_t playerId) const;
 
-	/// Get the LocalCoopPlayer for a coop player (players 2-4) - deprecated, use GetPlayer instead
-	/// @param playerId Game player ID (1-3 for coop players)
-	/// @return Pointer to LocalCoopPlayer, or nullptr if invalid or player 1
-	[[nodiscard]] LocalCoopPlayer *GetCoopPlayer(uint8_t playerId);
-	[[nodiscard]] const LocalCoopPlayer *GetCoopPlayer(uint8_t playerId) const;
 };
 
 extern LocalCoopState g_LocalCoop;
@@ -531,9 +515,9 @@ void AssignLocalCoopSpellToSlot(uint8_t playerId, int slotIndex);
  * @brief Load available heroes for local co-op player selection.
  *
  * Excludes heroes already selected by other local players.
- * @param playerId Game player ID (0 = player 1, 1-3 = coop players)
+ * @param playerIdx Game player index (same as Players[] / g_LocalCoop.players[]: 0–3).
  */
-void LoadAvailableHeroesForLocalPlayer(uint8_t playerId);
+void LoadAvailableHeroesForCoopSlot(uint8_t playerIdx);
 
 /**
  * @brief Confirm character selection and spawn a local co-op player.
@@ -863,10 +847,9 @@ uint8_t GetLocalCoopStoreOwnerPlayerId();
 /**
  * @brief Close the active store and clear local co-op store ownership.
  *
- * This is a convenience function that combines setting ActiveStore to None
- * and clearing local co-op store ownership. Use this instead of manually
- * setting ActiveStore = TalkID::None followed by ClearLocalCoopStoreOwner().
+ * Sets ActiveStore to None and clears local co-op store ownership (e.g. stick input
+ * for the prior store owner). Prefer this over assigning ActiveStore directly.
  */
-void CloseStore();
+void CloseLocalCoopStore();
 
 } // namespace devilution

Source/controls/local_coop/local_coop.hpp

@@ -4,6 +4,7 @@
  * Implementation of button mapping utilities for local co-op.
  */
 #include "controls/local_coop/local_coop_button_mapper.hpp"
+#include "controls/local_coop/local_coop_constants.hpp"
 
 #ifndef USE_SDL1
 #ifdef USE_SDL3
@@ -19,13 +20,13 @@ int LocalCoopButtonMapper::GetSkillSlot(ControllerButton button)
 {
 	switch (button) {
 	case ControllerButton_BUTTON_A:
-		return ButtonToSkillSlot[0]; // A -> slot 2
+		return LocalCoopInput::ButtonToSkillSlot[0]; // A -> slot 2
 	case ControllerButton_BUTTON_B:
-		return ButtonToSkillSlot[1]; // B -> slot 3
+		return LocalCoopInput::ButtonToSkillSlot[1]; // B -> slot 3
 	case ControllerButton_BUTTON_X:
-		return ButtonToSkillSlot[2]; // X -> slot 0
+		return LocalCoopInput::ButtonToSkillSlot[2]; // X -> slot 0
 	case ControllerButton_BUTTON_Y:
-		return ButtonToSkillSlot[3]; // Y -> slot 1
+		return LocalCoopInput::ButtonToSkillSlot[3]; // Y -> slot 1
 	default:
 		return -1;
 	}
@@ -37,27 +38,27 @@ int LocalCoopButtonMapper::GetSkillSlot(uint8_t sdlButton)
 #ifdef USE_SDL3
 	switch (sdlButton) {
 	case SDL_GAMEPAD_BUTTON_SOUTH:
-		return ButtonToSkillSlot[0]; // A (South) -> slot 2
+		return LocalCoopInput::ButtonToSkillSlot[0]; // A (South) -> slot 2
 	case SDL_GAMEPAD_BUTTON_EAST:
-		return ButtonToSkillSlot[1]; // B (East) -> slot 3
+		return LocalCoopInput::ButtonToSkillSlot[1]; // B (East) -> slot 3
 	case SDL_GAMEPAD_BUTTON_WEST:
-		return ButtonToSkillSlot[2]; // X (West) -> slot 0
+		return LocalCoopInput::ButtonToSkillSlot[2]; // X (West) -> slot 0
 	case SDL_GAMEPAD_BUTTON_NORTH:
-		return ButtonToSkillSlot[3]; // Y (North) -> slot 1
+		return LocalCoopInput::ButtonToSkillSlot[3]; // Y (North) -> slot 1
 	default:
 		return -1;
 	}
 #else
 	// SDL2 uses SDL_GameControllerButton enum
 	switch (sdlButton) {
 	case SDL_CONTROLLER_BUTTON_A:
-		return ButtonToSkillSlot[0]; // A -> slot 2
+		return LocalCoopInput::ButtonToSkillSlot[0]; // A -> slot 2
 	case SDL_CONTROLLER_BUTTON_B:
-		return ButtonToSkillSlot[1]; // B -> slot 3
+		return LocalCoopInput::ButtonToSkillSlot[1]; // B -> slot 3
 	case SDL_CONTROLLER_BUTTON_X:
-		return ButtonToSkillSlot[2]; // X -> slot 0
+		return LocalCoopInput::ButtonToSkillSlot[2]; // X -> slot 0
 	case SDL_CONTROLLER_BUTTON_Y:
-		return ButtonToSkillSlot[3]; // Y -> slot 1
+		return LocalCoopInput::ButtonToSkillSlot[3]; // Y -> slot 1
 	default:
 		return -1;
 	}
@@ -76,7 +77,7 @@ int LocalCoopButtonMapper::GetBeltSlot(int buttonIndex, bool leftShoulderHeld, b
 		return -1;
 
 	const int baseSlot = leftShoulderHeld ? 0 : 4;
-	return baseSlot + ButtonToBeltOffset[buttonIndex];
+	return baseSlot + LocalCoopInput::ButtonToBeltOffset[static_cast<size_t>(buttonIndex)];
 }
 
 int LocalCoopButtonMapper::GetButtonIndex(uint8_t sdlButton)

Source/controls/local_coop/local_coop_button_mapper.cpp

@@ -80,12 +80,6 @@ class LocalCoopButtonMapper {
 	static std::string_view GetBeltButtonLabel(int buttonIndex);
 
 private:
-	// Button to skill slot mapping: A=2, B=3, X=0, Y=1
-	static constexpr int ButtonToSkillSlot[4] = { 2, 3, 0, 1 };
-
-	// Button to belt offset mapping: A=0, B=1, X=2, Y=3
-	static constexpr int ButtonToBeltOffset[4] = { 0, 1, 2, 3 };
-
 	// Skill slot button labels: slot 0=X, slot 1=Y, slot 2=A, slot 3=B
 	static constexpr std::string_view SkillSlotLabels[4] = { "X", "Y", "A", "B" };
 

Source/controls/local_coop/local_coop_button_mapper.hpp

@@ -100,6 +100,17 @@ constexpr int BaseContentWidth = MinBeltWidth - 7;
 constexpr int BaseWidth = LeftBorderPadding + BaseContentWidth + RightBorderPadding;
 } // namespace PanelWidth
 
+/// Shared-screen bounds for local co-op (IsTilePositionOnScreen)
+namespace SharedScreen {
+constexpr int TileToScreenXMultiplier = 32;
+constexpr int TileToScreenYMultiplier = 16;
+constexpr int MaxDistanceFromRightEdge = 100;
+constexpr int MaxDistanceFromViewportHalfHeight = 80;
+constexpr int NormalizationScale = 1000;
+constexpr int64_t EllipseOutsideThresholdSq = 1000000;
+constexpr uint8_t ExcludeNoPlayer = 255;
+} // namespace SharedScreen
+
 } // namespace LocalCoopLayout
 
 namespace LocalCoopInput {

Source/controls/local_coop/local_coop_constants.hpp

@@ -6,6 +6,7 @@
 #include "pfile.h"
 
 #include <cstdint>
+#include <functional>
 #include <string>
 #include <string_view>
 
@@ -668,43 +669,60 @@ void sfile_write_stash()
 	Stash.dirty = false;
 }
 
-bool pfile_ui_set_hero_infos(bool (*uiAddHeroInfo)(_uiheroinfo *))
+namespace {
+
+template <typename Fn>
+bool pfile_ui_set_hero_infos_impl(Fn &&fn)
 {
 	memset(hero_names, 0, sizeof(hero_names));
 
 	uint32_t originalSaveNumber = gSaveNumber;
 	for (uint32_t i = 0; i < MAX_CHARACTERS; i++) {
 		std::optional<SaveReader> archive = OpenSaveArchive(i);
-		if (archive) {
-			PlayerPack pkplr;
-			if (ReadHero(*archive, &pkplr)) {
-				_uiheroinfo uihero;
-				uihero.saveNumber = i;
-				strcpy(hero_names[i], pkplr.pName);
-				const bool hasSaveGame = ArchiveContainsGame(*archive);
-				if (hasSaveGame)
-					pkplr.bIsHellfire = gbIsHellfireSaveGame ? 1 : 0;
-
-				Player &player = Players[0];
-
-				UnPackPlayer(pkplr, player);
-				// Temporarily set gSaveNumber to the save we're reading from
-				// so that LoadHeroItems reads items from the correct save file
-				gSaveNumber = i;
-				LoadHeroItems(player);
-				RemoveAllInvalidItems(player);
-				CalcPlrInv(player, false);
-
-				Game2UiPlayer(player, &uihero, hasSaveGame);
-				uiAddHeroInfo(&uihero);
-			}
-		}
+		if (!archive)
+			continue;
+		PlayerPack pkplr;
+		if (!ReadHero(*archive, &pkplr))
+			continue;
+		_uiheroinfo uihero;
+		uihero.saveNumber = i;
+		strcpy(hero_names[i], pkplr.pName);
+		const bool hasSaveGame = ArchiveContainsGame(*archive);
+		if (hasSaveGame)
+			pkplr.bIsHellfire = gbIsHellfireSaveGame ? 1 : 0;
+
+		// Must unpack into Players[0]: InitPlayer/UnPackPlayer use Player::getId() (distance
+		// from &Players[0]) for vision/lighting; a stack Player yields an invalid index.
+		Player &player = Players[0];
+
+		UnPackPlayer(pkplr, player);
+		// Temporarily set gSaveNumber to the save we're reading from
+		// so that LoadHeroItems reads items from the correct save file
+		gSaveNumber = i;
+		LoadHeroItems(player);
+		RemoveAllInvalidItems(player);
+		CalcPlrInv(player, false);
+
+		Game2UiPlayer(player, &uihero, hasSaveGame);
+		fn(&uihero);
 	}
 	gSaveNumber = originalSaveNumber;
 
 	return true;
 }
 
+} // namespace
+
+bool pfile_ui_set_hero_infos(bool (*uiAddHeroInfo)(_uiheroinfo *))
+{
+	return pfile_ui_set_hero_infos_impl([uiAddHeroInfo](_uiheroinfo *h) { return uiAddHeroInfo(h); });
+}
+
+bool pfile_ui_set_hero_infos(std::function<bool(_uiheroinfo *)> uiAddHeroInfo)
+{
+	return pfile_ui_set_hero_infos_impl(std::move(uiAddHeroInfo));
+}
+
 void pfile_ui_set_class_stats(HeroClass playerClass, _uidefaultstats *classStats)
 {
 	const ClassAttributes &classAttributes = GetClassAttributes(playerClass);

Source/pfile.cpp

@@ -6,6 +6,7 @@
 #pragma once
 
 #include <cstdint>
+#include <functional>
 #include <string>
 
 #include <expected.hpp>
@@ -119,6 +120,7 @@ HeroCompareResult pfile_compare_hero_demo(int demo, bool logDetails);
 
 void sfile_write_stash();
 bool pfile_ui_set_hero_infos(bool (*uiAddHeroInfo)(_uiheroinfo *));
+bool pfile_ui_set_hero_infos(std::function<bool(_uiheroinfo *)> uiAddHeroInfo);
 void pfile_ui_set_class_stats(HeroClass playerClass, _uidefaultstats *classStats);
 uint32_t pfile_ui_get_first_unused_save_num();
 bool pfile_ui_save_create(_uiheroinfo *heroinfo);

Source/pfile.h

@@ -1326,7 +1326,7 @@ void SmithEnter()
 		StartStore(TalkID::SmithRepair);
 		break;
 	case 20:
-		CloseStore();
+		CloseLocalCoopStore();
 		break;
 	}
 }
@@ -1560,7 +1560,7 @@ void WitchEnter()
 		StartStore(TalkID::WitchRecharge);
 		break;
 	case 20:
-		CloseStore();
+		CloseLocalCoopStore();
 		break;
 	}
 }
@@ -1703,7 +1703,7 @@ void BoyEnter()
 	}
 
 	if ((CurrentTextLine != 8 && !BoyItem.isEmpty()) || (CurrentTextLine != 12 && BoyItem.isEmpty())) {
-		CloseStore();
+		CloseLocalCoopStore();
 		return;
 	}
 
@@ -1757,7 +1757,7 @@ void HealerBuyItem(Item &item)
 void BoyBuyEnter()
 {
 	if (CurrentTextLine != 10) {
-		CloseStore();
+		CloseLocalCoopStore();
 		return;
 	}
 
@@ -1882,7 +1882,7 @@ void HealerEnter()
 		}
 		break;
 	case 18:
-		CloseStore();
+		CloseLocalCoopStore();
 		break;
 	}
 }
@@ -1928,7 +1928,7 @@ void StorytellerEnter()
 		StartStore(TalkID::StorytellerIdentify);
 		break;
 	case 18:
-		CloseStore();
+		CloseLocalCoopStore();
 		break;
 	}
 }
@@ -2004,7 +2004,7 @@ void TavernEnter()
 		StartStore(TalkID::Gossip);
 		break;
 	case 18:
-		CloseStore();
+		CloseLocalCoopStore();
 		break;
 	}
 }
@@ -2019,7 +2019,7 @@ void BarmaidEnter()
 		StartStore(TalkID::Gossip);
 		break;
 	case 14:
-		CloseStore();
+		CloseLocalCoopStore();
 		IsStashOpen = true;
 		Stash.RefreshItemStatFlags();
 		invflag = true;
@@ -2030,7 +2030,7 @@ void BarmaidEnter()
 		}
 		break;
 	case 18:
-		CloseStore();
+		CloseLocalCoopStore();
 		break;
 	}
 }
@@ -2045,7 +2045,7 @@ void DrunkEnter()
 		StartStore(TalkID::Gossip);
 		break;
 	case 18:
-		CloseStore();
+		CloseLocalCoopStore();
 		break;
 	}
 }
@@ -2145,7 +2145,7 @@ void AddStoreHoldRepair(Item *itm, int8_t i)
 void InitStores()
 {
 	ClearSText(0, NumStoreLines);
-	CloseStore();
+	CloseLocalCoopStore();
 	IsTextFullSize = false;
 	HasScrollbar = false;
 	PremiumItemCount = 0;
@@ -2186,7 +2186,7 @@ void FreeStoreMem()
 	if (*GetOptions().Gameplay.showItemGraphicsInStores) {
 		FreeHalfSizeItemSprites();
 	}
-	CloseStore();
+	CloseLocalCoopStore();
 	for (STextStruct &entry : TextLine) {
 		entry.text.clear();
 		entry.text.shrink_to_fit();
@@ -2498,7 +2498,7 @@ void StoreESC()
 	case TalkID::Tavern:
 	case TalkID::Drunk:
 	case TalkID::Barmaid:
-		CloseStore();
+		CloseLocalCoopStore();
 		break;
 	case TalkID::Gossip:
 		StartStore(OldActiveStore);