diff --git a/common/SettingsWrapper.cpp b/common/SettingsWrapper.cpp index 335f700031..d816e64576 100644 --- a/common/SettingsWrapper.cpp +++ b/common/SettingsWrapper.cpp @@ -167,6 +167,104 @@ void SettingsSaveWrapper::_EnumEntry(const char* section, const char* var, int& m_si.SetStringValue(section, var, enumArray[index]); } +SettingsSaveDeviationsWrapper::SettingsSaveDeviationsWrapper( + SettingsInterface& si, std::vector references) + : SettingsWrapper(si) + , m_references(std::move(references)) +{ +} + +SettingsSaveDeviationsWrapper::~SettingsSaveDeviationsWrapper() = default; + +bool SettingsSaveDeviationsWrapper::IsLoading() const +{ + return false; +} + +bool SettingsSaveDeviationsWrapper::IsSaving() const +{ + return true; +} + +template +void SettingsSaveDeviationsWrapper::Keep(const char* section, const char* var, const WriteFn& write) +{ + write(m_staging); + + std::string candidate; + if (!m_staging.GetStringValue(section, var, &candidate)) + { + // Nothing to compare against, so nothing can be said to be redundant. + write(m_si); + return; + } + + m_staging.DeleteValue(section, var); + + for (const MemorySettingsInterface* reference : m_references) + { + std::string existing; + if (reference->GetStringValue(section, var, &existing) && existing == candidate) + { + m_si.DeleteValue(section, var); + return; + } + } + + write(m_si); +} + +void SettingsSaveDeviationsWrapper::Entry(const char* section, const char* var, int& value, const int defvalue /*= 0*/) +{ + Keep(section, var, [&](SettingsInterface& si) { si.SetIntValue(section, var, value); }); +} + +void SettingsSaveDeviationsWrapper::Entry(const char* section, const char* var, uint& value, const uint defvalue /*= 0*/) +{ + Keep(section, var, [&](SettingsInterface& si) { si.SetUIntValue(section, var, value); }); +} + +void SettingsSaveDeviationsWrapper::Entry(const char* section, const char* var, bool& value, const bool defvalue /*= false*/) +{ + Keep(section, var, [&](SettingsInterface& si) { si.SetBoolValue(section, var, value); }); +} + +void SettingsSaveDeviationsWrapper::Entry(const char* section, const char* var, float& value, const float defvalue /*= 0.0*/) +{ + Keep(section, var, [&](SettingsInterface& si) { si.SetFloatValue(section, var, value); }); +} + +void SettingsSaveDeviationsWrapper::Entry( + const char* section, const char* var, std::string& value, const std::string& default_value /*= std::string()*/) +{ + Keep(section, var, [&](SettingsInterface& si) { si.SetStringValue(section, var, value.c_str()); }); +} + +void SettingsSaveDeviationsWrapper::Entry( + const char* section, const char* var, SmallStringBase& value, std::string_view default_value /* = std::string_view() */) +{ + Keep(section, var, [&](SettingsInterface& si) { si.SetStringValue(section, var, value.c_str()); }); +} + +bool SettingsSaveDeviationsWrapper::EntryBitBool(const char* section, const char* var, bool value, const bool defvalue /*= false*/) +{ + Keep(section, var, [&](SettingsInterface& si) { si.SetBoolValue(section, var, value); }); + return value; +} + +int SettingsSaveDeviationsWrapper::EntryBitfield(const char* section, const char* var, int value, const int defvalue /*= 0*/) +{ + Keep(section, var, [&](SettingsInterface& si) { si.SetIntValue(section, var, value); }); + return value; +} + +void SettingsSaveDeviationsWrapper::_EnumEntry(const char* section, const char* var, int& value, const char* const* enumArray, int defvalue) +{ + const int cnt = _calcEnumLength(enumArray); + const int index = (value < 0 || value >= cnt) ? defvalue : value; + Keep(section, var, [&](SettingsInterface& si) { si.SetStringValue(section, var, enumArray[index]); }); +} + SettingsClearWrapper::SettingsClearWrapper(SettingsInterface& si) : SettingsWrapper(si) { diff --git a/common/SettingsWrapper.h b/common/SettingsWrapper.h index 9f621f4932..e8dcd48de9 100644 --- a/common/SettingsWrapper.h +++ b/common/SettingsWrapper.h @@ -3,12 +3,14 @@ #pragma once +#include "MemorySettingsInterface.h" #include "SettingsInterface.h" #include "common/EnumOps.h" #include "common/SmallString.h" #include +#include // Helper class which loads or saves depending on the derived class. class SettingsWrapper @@ -102,6 +104,51 @@ protected: void _EnumEntry(const char* section, const char* var, int& value, const char* const* enumArray, int defvalue) override; }; +/// Saves only the values that differ from every reference, and deletes the rest. +/// +/// A per-game settings file is a record of decisions. Copying a whole configuration +/// into one verbatim writes hundreds of keys the player never chose and never saw, +/// and each of those then reads back as a deliberate choice — which is exactly how +/// a copy of the global settings ends up suppressing every automatic fix a game had. +/// Give this the values that would apply anyway and only the real deviations land. +/// +/// Comparison goes through the string form rather than the typed value, so a float +/// or an enum name compares the way it will actually be stored. That is why the +/// references must be MemorySettingsInterfaces built the same way: same class, same +/// formatting, exact comparison. +class SettingsSaveDeviationsWrapper final : public SettingsWrapper +{ +public: + SettingsSaveDeviationsWrapper(SettingsInterface& si, std::vector references); + ~SettingsSaveDeviationsWrapper(); + + bool IsLoading() const override; + bool IsSaving() const override; + + void Entry(const char* section, const char* var, int& value, const int defvalue = 0) override; + void Entry(const char* section, const char* var, uint& value, const uint defvalue = 0) override; + void Entry(const char* section, const char* var, bool& value, const bool defvalue = false) override; + void Entry(const char* section, const char* var, float& value, const float defvalue = 0.0) override; + void Entry(const char* section, const char* var, std::string& value, const std::string& default_value = std::string()) override; + void Entry(const char* section, const char* var, SmallStringBase& value, std::string_view default_value = std::string_view()) override; + + bool EntryBitBool(const char* section, const char* var, bool value, const bool defvalue = false) override; + int EntryBitfield(const char* section, const char* var, int value, const int defvalue = 0) override; + +protected: + void _EnumEntry(const char* section, const char* var, int& value, const char* const* enumArray, int defvalue) override; + +private: + /// Stages the value with `write`, then keeps it only if no reference already says + /// the same thing. + template + void Keep(const char* section, const char* var, const WriteFn& write); + + std::vector m_references; + /// Holds the candidate long enough to be rendered the same way a reference is. + MemorySettingsInterface m_staging; +}; + class SettingsClearWrapper final : public SettingsWrapper { public: diff --git a/pcsx2-qt/Settings/SettingsWindow.cpp b/pcsx2-qt/Settings/SettingsWindow.cpp index 9adbd0ef63..8ac674e6b8 100644 --- a/pcsx2-qt/Settings/SettingsWindow.cpp +++ b/pcsx2-qt/Settings/SettingsWindow.cpp @@ -294,7 +294,8 @@ void SettingsWindow::onCopyGlobalSettingsClicked() if (QMessageBox::question(this, tr("ARMSX2 Settings"), tr("The configuration for this game will be replaced by the current global settings.\n\nAny current setting values will be " - "overwritten.\n\nDo you want to continue?"), + "overwritten.\n\nOnly the settings that differ from the defaults, or from what the game database sets for this game, are " + "written. Anything else keeps following your global settings.\n\nDo you want to continue?"), QMessageBox::Yes, QMessageBox::No) != QMessageBox::Yes) { return; @@ -302,7 +303,7 @@ void SettingsWindow::onCopyGlobalSettingsClicked() { auto lock = Host::GetSettingsLock(); - Pcsx2Config::CopyConfiguration(m_sif.get(), *Host::Internal::GetBaseSettingsLayer()); + Pcsx2Config::CopyConfiguration(m_sif.get(), *Host::Internal::GetBaseSettingsLayer(), m_serial); Pcsx2Config::ClearInvalidPerGameConfiguration(m_sif.get()); } saveAndReloadGameSettings(); diff --git a/pcsx2/Config.h b/pcsx2/Config.h index 5748953d27..cfed70bff8 100644 --- a/pcsx2/Config.h +++ b/pcsx2/Config.h @@ -1651,7 +1651,12 @@ struct Pcsx2Config void CopyRuntimeConfig(Pcsx2Config& cfg); /// Copies configuration from one file to another. Does not copy controller settings. - static void CopyConfiguration(SettingsInterface* dest_si, SettingsInterface& src_si); + /// Copies a configuration into a per-game settings file, writing only the values + /// that deviate from what the game would run with anyway — stock defaults, and the + /// game database's own opinion for `game_serial`. Everything else is left absent, + /// because in a per-game file a key that is present is a key the player is taken to + /// have claimed, and the database then stands aside for it. + static void CopyConfiguration(SettingsInterface* dest_si, SettingsInterface& src_si, std::string_view game_serial); /// Clears all core keys from the specified interface. static void ClearConfiguration(SettingsInterface* dest_si); diff --git a/pcsx2/GameDatabase.cpp b/pcsx2/GameDatabase.cpp index 05c0c19b34..cc599e0781 100644 --- a/pcsx2/GameDatabase.cpp +++ b/pcsx2/GameDatabase.cpp @@ -539,12 +539,14 @@ static std::optional UserHackOverrideForHWFix(GameDatabaseSc } } -void GameDatabaseSchema::GameEntry::applyGameFixes(Pcsx2Config& config, bool applyAuto, const PerGameOverrides& overrides) const +void GameDatabaseSchema::GameEntry::applyGameFixes( + Pcsx2Config& config, bool applyAuto, const PerGameOverrides& overrides, ApplyMode mode) const { std::string suppressed_fixes; + const bool quiet = (mode == ApplyMode::Hypothetical); // Only apply core game fixes if the user has enabled them. - if (!applyAuto) + if (!applyAuto && !quiet) Console.Warning("GameDB: Game Fixes are disabled"); // Decides whether one database value lands, and says why when it does not. Two @@ -555,6 +557,9 @@ void GameDatabaseSchema::GameEntry::applyGameFixes(Pcsx2Config& config, bool app // Names are the database's own YAML spelling so a log line and a GameIndex entry // read together. const auto wanted = [&](bool claimed, const char* name, std::optional value = std::nullopt) { + if (quiet) + return applyAuto && !claimed; + const std::string label = value.has_value() ? fmt::format("{} = {}", name, value.value()) : std::string(name); if (claimed) @@ -576,29 +581,35 @@ void GameDatabaseSchema::GameEntry::applyGameFixes(Pcsx2Config& config, bool app if (eeRoundMode < FPRoundMode::MaxCount && wanted(overrides.Has(CoreGameDBKnob::EERoundMode), "eeRoundMode", static_cast(eeRoundMode))) { - Console.WriteLn("GameDB: Changing EE/FPU roundmode to %d [%s]", eeRoundMode, s_round_modes[static_cast(eeRoundMode)]); + if (!quiet) + Console.WriteLn("GameDB: Changing EE/FPU roundmode to %d [%s]", eeRoundMode, s_round_modes[static_cast(eeRoundMode)]); config.Cpu.FPUFPCR.SetRoundMode(eeRoundMode); } if (eeDivRoundMode < FPRoundMode::MaxCount && wanted(overrides.Has(CoreGameDBKnob::EEDivRoundMode), "eeDivRoundMode", static_cast(eeDivRoundMode))) { - Console.WriteLn("GameDB: Changing EE/FPU divison roundmode to %d [%s]", eeDivRoundMode, - s_round_modes[static_cast(eeDivRoundMode)]); + if (!quiet) + { + Console.WriteLn("GameDB: Changing EE/FPU divison roundmode to %d [%s]", eeDivRoundMode, + s_round_modes[static_cast(eeDivRoundMode)]); + } config.Cpu.FPUDivFPCR.SetRoundMode(eeDivRoundMode); } if (vu0RoundMode < FPRoundMode::MaxCount && wanted(overrides.Has(CoreGameDBKnob::VU0RoundMode), "vu0RoundMode", static_cast(vu0RoundMode))) { - Console.WriteLn("GameDB: Changing VU0 roundmode to %d [%s]", vu0RoundMode, s_round_modes[static_cast(vu0RoundMode)]); + if (!quiet) + Console.WriteLn("GameDB: Changing VU0 roundmode to %d [%s]", vu0RoundMode, s_round_modes[static_cast(vu0RoundMode)]); config.Cpu.VU0FPCR.SetRoundMode(vu0RoundMode); } if (vu1RoundMode < FPRoundMode::MaxCount && wanted(overrides.Has(CoreGameDBKnob::VU1RoundMode), "vu1RoundMode", static_cast(vu1RoundMode))) { - Console.WriteLn("GameDB: Changing VU1 roundmode to %d [%s]", vu1RoundMode, s_round_modes[static_cast(vu1RoundMode)]); + if (!quiet) + Console.WriteLn("GameDB: Changing VU1 roundmode to %d [%s]", vu1RoundMode, s_round_modes[static_cast(vu1RoundMode)]); config.Cpu.VU1FPCR.SetRoundMode(vu1RoundMode); } @@ -607,7 +618,8 @@ void GameDatabaseSchema::GameEntry::applyGameFixes(Pcsx2Config& config, bool app const int clampMode = enum_cast(eeClampMode); if (wanted(overrides.Has(CoreGameDBKnob::EEClampMode), "eeClampMode", clampMode)) { - Console.WriteLn("GameDB: Changing EE/FPU clamp mode [mode=%d]", clampMode); + if (!quiet) + Console.WriteLn("GameDB: Changing EE/FPU clamp mode [mode=%d]", clampMode); config.Cpu.Recompiler.fpuOverflow = (clampMode >= 1); config.Cpu.Recompiler.fpuExtraOverflow = (clampMode >= 2); config.Cpu.Recompiler.fpuFullMode = (clampMode >= 3); @@ -620,7 +632,8 @@ void GameDatabaseSchema::GameEntry::applyGameFixes(Pcsx2Config& config, bool app const int clampMode = enum_cast(vu0ClampMode); if (wanted(overrides.Has(CoreGameDBKnob::VU0ClampMode), "vu0ClampMode", clampMode)) { - Console.WriteLn("GameDB: Changing VU0 clamp mode [mode=%d]", clampMode); + if (!quiet) + Console.WriteLn("GameDB: Changing VU0 clamp mode [mode=%d]", clampMode); config.Cpu.Recompiler.vu0Overflow = (clampMode >= 1); config.Cpu.Recompiler.vu0ExtraOverflow = (clampMode >= 2); config.Cpu.Recompiler.vu0SignOverflow = (clampMode >= 3); @@ -632,7 +645,8 @@ void GameDatabaseSchema::GameEntry::applyGameFixes(Pcsx2Config& config, bool app const int clampMode = enum_cast(vu1ClampMode); if (wanted(overrides.Has(CoreGameDBKnob::VU1ClampMode), "vu1ClampMode", clampMode)) { - Console.WriteLn("GameDB: Changing VU1 clamp mode [mode=%d]", clampMode); + if (!quiet) + Console.WriteLn("GameDB: Changing VU1 clamp mode [mode=%d]", clampMode); config.Cpu.Recompiler.vu1Overflow = (clampMode >= 1); config.Cpu.Recompiler.vu1ExtraOverflow = (clampMode >= 2); config.Cpu.Recompiler.vu1SignOverflow = (clampMode >= 3); @@ -648,8 +662,11 @@ void GameDatabaseSchema::GameEntry::applyGameFixes(Pcsx2Config& config, bool app // Legacy note - speedhacks are setup in the GameDB as integer values, but // are effectively booleans like the gamefixes config.Speedhacks.Set(it.first, it.second); - Console.WriteLn("GameDB: Setting Speedhack '%s' to [mode=%d]", - Pcsx2Config::SpeedhackOptions::GetSpeedHackName(it.first), it.second); + if (!quiet) + { + Console.WriteLn("GameDB: Setting Speedhack '%s' to [mode=%d]", + Pcsx2Config::SpeedhackOptions::GetSpeedHackName(it.first), it.second); + } } // TODO - config - this could be simplified with maps instead of bitfields and enums @@ -663,15 +680,20 @@ void GameDatabaseSchema::GameEntry::applyGameFixes(Pcsx2Config& config, bool app // if the fix is present, it is said to be enabled config.Gamefixes.Set(id, true); - Console.WriteLn("GameDB: Enabled Gamefix: %s", Pcsx2Config::GamefixOptions::GetGameFixName(id)); + if (!quiet) + Console.WriteLn("GameDB: Enabled Gamefix: %s", Pcsx2Config::GamefixOptions::GetGameFixName(id)); - // The LUT is only used for 1 game so we allocate it only when the gamefix is enabled (save 4MB) - if (id == Fix_GoemonTlbMiss && true) + // The LUT is only used for 1 game so we allocate it only when the gamefix is enabled (save 4MB). + // A hypothetical apply is not going to run anything, so it does not get the 4MB. + if (id == Fix_GoemonTlbMiss && !quiet) vtlb_Alloc_Ppmap(); } // Keyed so the next settings reload replaces or clears it, matching how the // graphics half reports the same thing. + if (quiet) + return; + if (!suppressed_fixes.empty()) { Host::AddKeyedOSDMessage("CoreFixesWarning", @@ -816,14 +838,16 @@ bool GameDatabaseSchema::GameEntry::configMatchesHWFix(const Pcsx2Config::GSOpti } } -void GameDatabaseSchema::GameEntry::applyGSHardwareFixes(Pcsx2Config::GSOptions& config, const PerGameOverrides& overrides) const +void GameDatabaseSchema::GameEntry::applyGSHardwareFixes( + Pcsx2Config::GSOptions& config, const PerGameOverrides& overrides, ApplyMode mode) const { std::string disabled_fixes; + const bool quiet = (mode == ApplyMode::Hypothetical); // Only apply GS HW fixes if the user hasn't manually enabled HW fixes. const bool apply_auto_fixes = !config.ManualUserHacks; const bool is_sw_renderer = EmuConfig.GS.Renderer == GSRendererType::SW; - if (!apply_auto_fixes) + if (!apply_auto_fixes && !quiet) Console.Warning("GameDB: Manual GS hardware renderer fixes are enabled, not using automatic hardware renderer fixes from GameDB."); for (const auto& [id, value] : gsHWFixes) @@ -845,7 +869,7 @@ void GameDatabaseSchema::GameEntry::applyGSHardwareFixes(Pcsx2Config::GSOptions& // to be claimed, which is most of what a player actually changes. if (pinned || (isUserHackHWFix(id) && !apply_auto_fixes)) { - if (configMatchesHWFix(config, id, value)) + if (configMatchesHWFix(config, id, value) || quiet) continue; Console.Warning("GameDB: Skipping GS Hardware Fix: %s to [mode=%d]", getHWFixName(id), value); @@ -973,6 +997,7 @@ void GameDatabaseSchema::GameEntry::applyGSHardwareFixes(Pcsx2Config::GSOptions& if (config.TriFilter == TriFiltering::Automatic) config.TriFilter = static_cast(value); else if (config.TriFilter > TriFiltering::Off) + if (!quiet) Console.Warning("GameDB: Game requires trilinear filtering to be disabled."); } } @@ -1015,6 +1040,7 @@ void GameDatabaseSchema::GameEntry::applyGSHardwareFixes(Pcsx2Config::GSOptions& if (config.InterlaceMode == GSInterlaceMode::Automatic) config.InterlaceMode = static_cast(value); else + if (!quiet) Console.Warning("GameDB: Game requires different deinterlace mode but it has been overridden by user setting."); } } @@ -1065,6 +1091,9 @@ void GameDatabaseSchema::GameEntry::applyGSHardwareFixes(Pcsx2Config::GSOptions& case GSHWFixId::RecommendedBlendingLevel: { + if (quiet) + break; + if (!is_sw_renderer && value >= 0 && value <= static_cast(AccBlendLevel::Maximum) && static_cast(EmuConfig.GS.AccurateBlendingUnit) < value) { static constexpr std::array(AccBlendLevel::MaxCount)> s_blending_option_names = {{ @@ -1096,6 +1125,9 @@ void GameDatabaseSchema::GameEntry::applyGSHardwareFixes(Pcsx2Config::GSOptions& case GSHWFixId::RecommendedAccurateAlphaTest: { + if (quiet) + break; + if (!is_sw_renderer && value >= 0 && value <= 1 && static_cast(config.HWAccurateAlphaTest) < value) { @@ -1117,6 +1149,9 @@ void GameDatabaseSchema::GameEntry::applyGSHardwareFixes(Pcsx2Config::GSOptions& case GSHWFixId::RecommendedHWAA1: { + if (quiet) + break; + if (!is_sw_renderer && value >= 0 && value <= 1 && static_cast(config.HWAA1) < value) { @@ -1152,12 +1187,16 @@ void GameDatabaseSchema::GameEntry::applyGSHardwareFixes(Pcsx2Config::GSOptions& break; } - Console.WriteLn("GameDB: Enabled GS Hardware Fix: %s to [mode=%d]", getHWFixName(id), value); + if (!quiet) + Console.WriteLn("GameDB: Enabled GS Hardware Fix: %s to [mode=%d]", getHWFixName(id), value); } // fixup skipdraw range just in case the db has a bad range (but the linter should catch this) config.SkipDrawEnd = std::max(config.SkipDrawStart, config.SkipDrawEnd); + if (quiet) + return; + if (!is_sw_renderer && !disabled_fixes.empty()) { // Pinning a single hack lands here too, and then blaming manual mode would be a lie. diff --git a/pcsx2/GameDatabase.h b/pcsx2/GameDatabase.h index a86f77738a..95bf8ed6b7 100644 --- a/pcsx2/GameDatabase.h +++ b/pcsx2/GameDatabase.h @@ -94,6 +94,16 @@ namespace GameDatabaseSchema Count }; + /// Whether the database's values are actually being adopted, or only being worked + /// out so something can be compared against them. A hypothetical apply says nothing + /// to the log or the screen and allocates nothing, because nothing is going to run + /// with the result. + enum class ApplyMode : u8 + { + Live, + Hypothetical, + }; + struct GameEntry { std::string name; @@ -123,10 +133,12 @@ namespace GameDatabaseSchema /// Applies Core game fixes to an existing config. Anything the player set for /// this game specifically is left alone — `overrides` says which, and defaults /// to nothing claimed, which is the old behaviour. - void applyGameFixes(Pcsx2Config& config, bool applyAuto, const PerGameOverrides& overrides = {}) const; + void applyGameFixes(Pcsx2Config& config, bool applyAuto, const PerGameOverrides& overrides = {}, + ApplyMode mode = ApplyMode::Live) const; /// Applies GS hardware fixes to an existing config, on the same terms. - void applyGSHardwareFixes(Pcsx2Config::GSOptions& config, const PerGameOverrides& overrides = {}) const; + void applyGSHardwareFixes(Pcsx2Config::GSOptions& config, const PerGameOverrides& overrides = {}, + ApplyMode mode = ApplyMode::Live) const; /// Returns true if the current config value for the specified hw fix id matches the value. static bool configMatchesHWFix(const Pcsx2Config::GSOptions& config, GSHWFixId id, int value); diff --git a/pcsx2/ImGui/FullscreenUI_Internal.h b/pcsx2/ImGui/FullscreenUI_Internal.h index 99d83a8239..2b3cb73b65 100644 --- a/pcsx2/ImGui/FullscreenUI_Internal.h +++ b/pcsx2/ImGui/FullscreenUI_Internal.h @@ -513,6 +513,9 @@ namespace FullscreenUI inline SettingsPage s_settings_page = SettingsPage::Interface; inline std::unique_ptr s_game_settings_interface; inline std::unique_ptr s_game_settings_entry; + // Whose settings are being edited. Kept beside the interface rather than read off + // the entry, which is null when an ELF is what opened the page. Empty for those. + inline std::string s_game_settings_serial; inline std::vector> s_game_list_directories_cache; inline std::vector s_graphics_adapter_list_cache; inline std::vector s_game_patch_list; diff --git a/pcsx2/ImGui/FullscreenUI_Settings.cpp b/pcsx2/ImGui/FullscreenUI_Settings.cpp index 9993ceef3d..5a8b0d6711 100644 --- a/pcsx2/ImGui/FullscreenUI_Settings.cpp +++ b/pcsx2/ImGui/FullscreenUI_Settings.cpp @@ -1649,6 +1649,7 @@ void FullscreenUI::SwitchToSettings() { s_game_settings_entry.reset(); s_game_settings_interface.reset(); + s_game_settings_serial.clear(); s_game_patch_list = {}; s_enabled_game_patch_cache = {}; s_game_cheats_list = {}; @@ -1662,6 +1663,7 @@ void FullscreenUI::SwitchToSettings() void FullscreenUI::SwitchToGameSettings(const std::string_view serial, u32 crc) { s_game_settings_entry.reset(); + s_game_settings_serial = serial; s_game_settings_interface = std::make_unique(VMManager::GetGameSettingsPath(serial, crc)); s_game_settings_interface->Load(); PopulatePatchesAndCheatsList(serial, crc); @@ -1737,7 +1739,7 @@ void FullscreenUI::DoCopyGameSettings() if (!s_game_settings_interface) return; - Pcsx2Config::CopyConfiguration(s_game_settings_interface.get(), *GetEditingSettingsInterface(false)); + Pcsx2Config::CopyConfiguration(s_game_settings_interface.get(), *GetEditingSettingsInterface(false), s_game_settings_serial); Pcsx2Config::ClearInvalidPerGameConfiguration(s_game_settings_interface.get()); SetSettingsChanged(s_game_settings_interface.get()); diff --git a/pcsx2/Pcsx2Config.cpp b/pcsx2/Pcsx2Config.cpp index 2a415298fc..5dbebccaf0 100644 --- a/pcsx2/Pcsx2Config.cpp +++ b/pcsx2/Pcsx2Config.cpp @@ -3,6 +3,7 @@ #include "common/CocoaTools.h" #include "common/FileSystem.h" +#include "common/MemorySettingsInterface.h" #include "common/Path.h" #include "common/SettingsInterface.h" #include "common/SettingsWrapper.h" @@ -11,6 +12,7 @@ #include "Config.h" #include "GS.h" #include "CDVD/CDVDcommon.h" +#include "GameDatabase.h" #include "Host.h" #include "Host/AudioStream.h" #include "SIO/Memcard/MemoryCardFile.h" @@ -2315,7 +2317,7 @@ void Pcsx2Config::CopyRuntimeConfig(Pcsx2Config& cfg) } } -void Pcsx2Config::CopyConfiguration(SettingsInterface* dest_si, SettingsInterface& src_si) +void Pcsx2Config::CopyConfiguration(SettingsInterface* dest_si, SettingsInterface& src_si, const std::string_view game_serial) { FPControlRegisterBackup fpcr_backup(FPControlRegister::GetDefault()); @@ -2324,10 +2326,45 @@ void Pcsx2Config::CopyConfiguration(SettingsInterface* dest_si, SettingsInterfac SettingsLoadWrapper wrapper(src_si); temp.LoadSaveCore(wrapper); } + + // Two things a value can agree with, and agreeing with either means writing it + // down would decide nothing. + // + // Stock defaults: the value is simply untouched, and the file would carry it only + // as noise. Copying the whole configuration verbatim is what buried the handful of + // real choices under hundreds of these, and in a per-game file a key that is + // present is a key the player is taken to have claimed — which is how one press of + // this button used to disable every automatic fix a game had. + // + // What the database would set: it is going to set it regardless, so writing it can + // only turn into a claim that suppresses the very fix it agrees with. + // + // Note the reference is a default configuration with the database applied, not this + // one — the question is what the database wants, not what it would leave the source + // at. That matters for the few fixes that clamp rather than assign; for those this + // errs towards writing the player's value, never towards dropping a fix. + MemorySettingsInterface defaults_si; { - SettingsSaveWrapper wrapper(*dest_si); - temp.LoadSaveCore(wrapper); + Pcsx2Config defaults; + SettingsSaveWrapper wrapper(defaults_si); + defaults.LoadSaveCore(wrapper); } + + MemorySettingsInterface database_si; + { + Pcsx2Config database; + if (const GameDatabaseSchema::GameEntry* game = GameDatabase::findGame(game_serial)) + { + game->applyGameFixes(database, true, {}, GameDatabaseSchema::ApplyMode::Hypothetical); + game->applyGSHardwareFixes(database.GS, {}, GameDatabaseSchema::ApplyMode::Hypothetical); + } + + SettingsSaveWrapper wrapper(database_si); + database.LoadSaveCore(wrapper); + } + + SettingsSaveDeviationsWrapper wrapper(*dest_si, {&defaults_si, &database_si}); + temp.LoadSaveCore(wrapper); } void Pcsx2Config::ClearConfiguration(SettingsInterface* dest_si) diff --git a/tests/ctest/core/CMakeLists.txt b/tests/ctest/core/CMakeLists.txt index 57569a0f64..480060e797 100644 --- a/tests/ctest/core/CMakeLists.txt +++ b/tests/ctest/core/CMakeLists.txt @@ -4,6 +4,7 @@ add_pcsx2_test(core_test ee_fpu_reloc_tests.cpp patch_tests.cpp savestate_legacy_tests.cpp + settings_precedence_tests.cpp MockMemoryInterface.h StubHost.cpp ) diff --git a/tests/ctest/core/settings_precedence_tests.cpp b/tests/ctest/core/settings_precedence_tests.cpp new file mode 100644 index 0000000000..51a6f603b1 --- /dev/null +++ b/tests/ctest/core/settings_precedence_tests.cpp @@ -0,0 +1,425 @@ +// SPDX-FileCopyrightText: 2026 ARMSX2 Contributors +// SPDX-License-Identifier: GPL-3.0+ + +// Global settings < GameDB < per-game settings. +// +// The middle of that used to be the end of it: the database wrote into EmuConfig +// after the whole settings read, so a value the player set for one game was quietly +// replaced. What decides it now is whether the per-game file holds the key, since +// every settings screen deletes the key rather than writing a value for "use the +// global setting". +// +// That rule only holds while the key tables stay complete. A knob nobody mapped is a +// setting that silently goes back to being overridden, and it looks like nothing — +// no warning, no failure, just the old bug for that one setting. Hence the drift +// guards below, which are the real reason this file exists. + +#include + +#include "Config.h" +#include "GameDatabase.h" +#include "PerGameOverrides.h" + +#include "common/MemorySettingsInterface.h" +#include "common/SettingsWrapper.h" + +namespace +{ + +// Stands in for a per-game INI. Same class the reference configurations are built +// with, so values compare the way they will be stored. +class GameFile +{ +public: + GameFile& Set(const char* section, const char* key, int value) + { + m_si.SetIntValue(section, key, value); + return *this; + } + + GameFile& Set(const char* section, const char* key, bool value) + { + m_si.SetBoolValue(section, key, value); + return *this; + } + + const MemorySettingsInterface& Get() const { return m_si; } + PerGameOverrides Overrides() const { return ComputePerGameOverrides(m_si); } + +private: + MemorySettingsInterface m_si; +}; + +} // namespace + +// ---------------------------------------------------------------- provenance + +TEST(SettingsPrecedence, AnEmptyFileClaimsNothing) +{ + const PerGameOverrides ov = GameFile().Overrides(); + EXPECT_FALSE(ov.Any()); + EXPECT_EQ(ov.gs_fixes, 0u); + EXPECT_EQ(ov.gs_hacks, 0u); + EXPECT_EQ(ov.core, 0u); + EXPECT_EQ(ov.speedhacks, 0u); + EXPECT_TRUE(ov.gamefixes.none()); +} + +TEST(SettingsPrecedence, EveryGamefixKeyClaimsItsOwnFixAndNoOther) +{ + for (u32 i = GamefixId_FIRST; i < GamefixId_COUNT; i++) + { + const GamefixId id = static_cast(i); + const char* key = PerGameOverrideKeys::ForGamefix(id); + ASSERT_NE(key, nullptr) << "gamefix " << i; + + GameFile file; + file.Set("EmuCore/Gamefixes", key, false); + const PerGameOverrides ov = file.Overrides(); + + EXPECT_EQ(ov.gamefixes.count(), 1u) << key; + EXPECT_TRUE(ov.Has(id)) << key; + EXPECT_EQ(ov.core, 0u) << key; + EXPECT_EQ(ov.speedhacks, 0u) << key; + } +} + +TEST(SettingsPrecedence, EverySpeedhackKeyClaimsItsOwnHackAndNoOther) +{ + for (u32 i = 0; i < static_cast(SpeedHack::MaxCount); i++) + { + const SpeedHack hack = static_cast(i); + const char* key = PerGameOverrideKeys::ForSpeedHack(hack); + ASSERT_NE(key, nullptr) << "speedhack " << i; + + GameFile file; + file.Set("EmuCore/Speedhacks", key, 1); + const PerGameOverrides ov = file.Overrides(); + + EXPECT_EQ(ov.speedhacks, 1u << i) << key; + EXPECT_TRUE(ov.gamefixes.none()) << key; + EXPECT_EQ(ov.core, 0u) << key; + } +} + +// A clamp mode is one picker over three or four keys. The screens write and delete +// them together, so any one of them present has to claim the whole mode — otherwise +// the database rewrites the other bits and the player gets a mode they never chose. +TEST(SettingsPrecedence, AnyOneKeyOfAClampModeClaimsTheWholeMode) +{ + for (u32 i = 0; i < static_cast(CoreGameDBKnob::MaxCount); i++) + { + const CoreGameDBKnob knob = static_cast(i); + const PerGameOverrideKeys::CoreKnobKeys keys = PerGameOverrideKeys::ForCoreKnob(knob); + ASSERT_NE(keys.section, nullptr) << "knob " << i; + ASSERT_GT(keys.count, 0u) << "knob " << i; + + for (u32 k = 0; k < keys.count; k++) + { + GameFile file; + file.Set(keys.section, keys.keys[k], true); + const PerGameOverrides ov = file.Overrides(); + + EXPECT_EQ(ov.core, 1u << i) << keys.keys[k]; + EXPECT_TRUE(ov.gamefixes.none()) << keys.keys[k]; + EXPECT_EQ(ov.speedhacks, 0u) << keys.keys[k]; + } + } +} + +TEST(SettingsPrecedence, AGraphicsKeyClaimsItsFixAndItsLegacyHackBit) +{ + GameFile file; + file.Set("EmuCore/GS", "UserHacks_align_sprite_X", true); + const PerGameOverrides ov = file.Overrides(); + + EXPECT_TRUE(ov.Has(GameDatabaseSchema::GSHWFixId::AlignSprite)); + // The narrower mask matters too: MaskUserHacks() strips an unclaimed hack before + // the database is ever consulted, so without this bit the value is already gone. + EXPECT_EQ(ov.gs_hacks, 1u << static_cast(GSUserHackOverride::AlignSprite)); +} + +// Mipmapping was never a "user hack", so it has no bit in the legacy mask. It still +// has to be claimable, because it is one of the settings people actually change. +TEST(SettingsPrecedence, ASettingThatWasNeverAUserHackIsStillClaimable) +{ + GameFile file; + file.Set("EmuCore/GS", "hw_mipmap", false); + const PerGameOverrides ov = file.Overrides(); + + EXPECT_TRUE(ov.Has(GameDatabaseSchema::GSHWFixId::Mipmap)); + EXPECT_EQ(ov.gs_hacks, 0u); +} + +// One control, two database fixes. The database clamps the blend level from both +// ends, so claiming the setting has to silence both or the player still gets moved. +TEST(SettingsPrecedence, TheBlendingSettingClaimsBothOfItsClamps) +{ + GameFile file; + file.Set("EmuCore/GS", "accurate_blending_unit", 2); + const PerGameOverrides ov = file.Overrides(); + + EXPECT_TRUE(ov.Has(GameDatabaseSchema::GSHWFixId::MinimumBlendingLevel)); + EXPECT_TRUE(ov.Has(GameDatabaseSchema::GSHWFixId::MaximumBlendingLevel)); +} + +// ---------------------------------------------------------------- drift guards + +TEST(SettingsPrecedenceDrift, EveryGamefixHasASettingsKey) +{ + for (u32 i = GamefixId_FIRST; i < GamefixId_COUNT; i++) + { + const char* key = PerGameOverrideKeys::ForGamefix(static_cast(i)); + ASSERT_NE(key, nullptr) << "gamefix " << i << " has no settings key, so it can never be claimed"; + EXPECT_STRNE(key, "") << "gamefix " << i; + } +} + +TEST(SettingsPrecedenceDrift, EverySpeedhackHasASettingsKey) +{ + for (u32 i = 0; i < static_cast(SpeedHack::MaxCount); i++) + { + const char* key = PerGameOverrideKeys::ForSpeedHack(static_cast(i)); + ASSERT_NE(key, nullptr) << "speedhack " << i << " has no settings key, so it can never be claimed"; + } +} + +// The six exceptions are deliberate and are the whole list: three renderer routine +// selectors with no setting and no UI, and three that only raise a recommendation and +// write no config field. Anything else reaching this list is a setting that has gone +// back to being silently overridden. +TEST(SettingsPrecedenceDrift, OnlyTheSixUnsettableGraphicsFixesLackAKey) +{ + using GSHWFixId = GameDatabaseSchema::GSHWFixId; + static constexpr GSHWFixId kExpected[] = { + GSHWFixId::RecommendedBlendingLevel, + GSHWFixId::RecommendedAccurateAlphaTest, + GSHWFixId::RecommendedHWAA1, + GSHWFixId::GetSkipCount, + GSHWFixId::BeforeDraw, + GSHWFixId::MoveHandler, + }; + + for (u32 i = 0; i < static_cast(GSHWFixId::Count); i++) + { + const GSHWFixId id = static_cast(i); + if (PerGameOverrideKeys::ForGSHWFix(id) != nullptr) + continue; + + bool expected = false; + for (const GSHWFixId allowed : kExpected) + expected |= (allowed == id); + + EXPECT_TRUE(expected) << "GS hardware fix " << i << " has no settings key, so the player cannot claim it"; + } +} + +TEST(SettingsPrecedenceDrift, EveryMappedKeyIsRecognisedAsAClaim) +{ + for (u32 i = GamefixId_FIRST; i < GamefixId_COUNT; i++) + { + EXPECT_TRUE(PerGameOverrideKeys::ClaimsAGameDBSetting( + "EmuCore/Gamefixes", PerGameOverrideKeys::ForGamefix(static_cast(i)))) + << "gamefix " << i; + } + + for (u32 i = 0; i < static_cast(SpeedHack::MaxCount); i++) + { + EXPECT_TRUE(PerGameOverrideKeys::ClaimsAGameDBSetting( + "EmuCore/Speedhacks", PerGameOverrideKeys::ForSpeedHack(static_cast(i)))) + << "speedhack " << i; + } + + for (u32 i = 0; i < static_cast(CoreGameDBKnob::MaxCount); i++) + { + const PerGameOverrideKeys::CoreKnobKeys keys = PerGameOverrideKeys::ForCoreKnob(static_cast(i)); + for (u32 k = 0; k < keys.count; k++) + EXPECT_TRUE(PerGameOverrideKeys::ClaimsAGameDBSetting(keys.section, keys.keys[k])) << keys.keys[k]; + } + + EXPECT_FALSE(PerGameOverrideKeys::ClaimsAGameDBSetting("EmuCore/GS", "Renderer")); + EXPECT_FALSE(PerGameOverrideKeys::ClaimsAGameDBSetting("EmuCore", "EnableCheats")); +} + +// ---------------------------------------------------------------- the apply side + +TEST(SettingsPrecedenceApply, AnUnclaimedCoreKnobStillTakesTheDatabaseValue) +{ + GameDatabaseSchema::GameEntry entry; + entry.eeClampMode = static_cast(3); + entry.gameFixes.push_back(Fix_EETiming); + entry.speedHacks.emplace_back(SpeedHack::MTVU, 1); + + Pcsx2Config config; + config.Cpu.Recompiler.SetEEClampMode(1); + config.Gamefixes.Set(Fix_EETiming, false); + config.Speedhacks.vuThread = false; + + entry.applyGameFixes(config, true, {}, GameDatabaseSchema::ApplyMode::Hypothetical); + + EXPECT_EQ(config.Cpu.Recompiler.GetEEClampMode(), 3u); + EXPECT_TRUE(config.Gamefixes.Get(Fix_EETiming)); + EXPECT_TRUE(config.Speedhacks.vuThread); +} + +TEST(SettingsPrecedenceApply, AClaimedCoreKnobKeepsThePlayersValue) +{ + GameDatabaseSchema::GameEntry entry; + entry.eeClampMode = static_cast(3); + entry.gameFixes.push_back(Fix_EETiming); + entry.speedHacks.emplace_back(SpeedHack::MTVU, 1); + + const PerGameOverrides ov = GameFile() + .Set("EmuCore/CPU/Recompiler", "fpuFullMode", false) + .Set("EmuCore/Gamefixes", "EETimingHack", false) + .Set("EmuCore/Speedhacks", "vuThread", false) + .Overrides(); + + Pcsx2Config config; + config.Cpu.Recompiler.SetEEClampMode(1); + config.Gamefixes.Set(Fix_EETiming, false); + config.Speedhacks.vuThread = false; + + entry.applyGameFixes(config, true, ov, GameDatabaseSchema::ApplyMode::Hypothetical); + + EXPECT_EQ(config.Cpu.Recompiler.GetEEClampMode(), 1u); + EXPECT_FALSE(config.Gamefixes.Get(Fix_EETiming)); + EXPECT_FALSE(config.Speedhacks.vuThread); +} + +// Claiming one setting must not cost the game every other fix it had. That was the +// whole failing of the two all-or-nothing switches this replaces. +TEST(SettingsPrecedenceApply, ClaimingOneKnobLeavesTheRestOfTheEntryAlone) +{ + GameDatabaseSchema::GameEntry entry; + entry.eeClampMode = static_cast(3); + entry.vu1ClampMode = static_cast(2); + entry.gameFixes.push_back(Fix_EETiming); + + const PerGameOverrides ov = GameFile().Set("EmuCore/CPU/Recompiler", "fpuOverflow", true).Overrides(); + + Pcsx2Config config; + config.Cpu.Recompiler.SetEEClampMode(1); + config.Cpu.Recompiler.vu1Overflow = false; + config.Cpu.Recompiler.vu1ExtraOverflow = false; + config.Cpu.Recompiler.vu1SignOverflow = false; + config.Gamefixes.Set(Fix_EETiming, false); + + entry.applyGameFixes(config, true, ov, GameDatabaseSchema::ApplyMode::Hypothetical); + + EXPECT_EQ(config.Cpu.Recompiler.GetEEClampMode(), 1u) << "the claimed one"; + EXPECT_TRUE(config.Cpu.Recompiler.vu1Overflow) << "an unclaimed one"; + EXPECT_TRUE(config.Cpu.Recompiler.vu1ExtraOverflow) << "an unclaimed one"; + EXPECT_TRUE(config.Gamefixes.Get(Fix_EETiming)) << "an unclaimed one"; +} + +TEST(SettingsPrecedenceApply, AClaimedGraphicsFixKeepsThePlayersValue) +{ + GameDatabaseSchema::GameEntry entry; + entry.gsHWFixes.emplace_back(GameDatabaseSchema::GSHWFixId::Mipmap, 1); + entry.gsHWFixes.emplace_back(GameDatabaseSchema::GSHWFixId::TextureInsideRT, 1); + + Pcsx2Config::GSOptions gs; + gs.HWMipmap = false; + gs.UserHacks_TextureInsideRt = GSTextureInRtMode::Disabled; + + const PerGameOverrides ov = GameFile().Set("EmuCore/GS", "hw_mipmap", false).Overrides(); + entry.applyGSHardwareFixes(gs, ov, GameDatabaseSchema::ApplyMode::Hypothetical); + + EXPECT_FALSE(gs.HWMipmap) << "claimed, so the database stands aside"; + EXPECT_EQ(gs.UserHacks_TextureInsideRt, GSTextureInRtMode::InsideTargets) << "unclaimed, so it still applies"; +} + +// ---------------------------------------------------------------- the copy filter + +namespace +{ + +// Everything CopyConfiguration would write, as key counts per section. +size_t CountKeys(const MemorySettingsInterface& si, const char* section) +{ + return si.GetKeyValueList(section).size(); +} + +} // namespace + +TEST(SettingsCopyFilter, CopyingAnUntouchedConfigurationWritesNothing) +{ + MemorySettingsInterface source; + { + Pcsx2Config defaults; + SettingsSaveWrapper wrapper(source); + defaults.LoadSaveCore(wrapper); + } + + MemorySettingsInterface dest; + Pcsx2Config::CopyConfiguration(&dest, source, std::string_view()); + + EXPECT_EQ(CountKeys(dest, "EmuCore/GS"), 0u); + EXPECT_EQ(CountKeys(dest, "EmuCore/Gamefixes"), 0u); + EXPECT_EQ(CountKeys(dest, "EmuCore/Speedhacks"), 0u); + EXPECT_EQ(CountKeys(dest, "EmuCore/CPU/Recompiler"), 0u); + EXPECT_TRUE(dest.IsEmpty()) << "a copy of stock settings is not a set of decisions"; +} + +// The other half of the filter, and the one that keeps the copy button from +// suppressing fixes: a value the database is going to set anyway is not a decision, +// so writing it down would only create a claim against the fix it agrees with. +// +// Driven through the wrapper rather than CopyConfiguration because building the real +// second reference means loading the game database off disk, which a unit test has no +// business doing. The reference here is what a database entry would have produced. +TEST(SettingsCopyFilter, AValueTheDatabaseWouldSetAnywayIsNotWritten) +{ + MemorySettingsInterface defaults_si; + { + Pcsx2Config defaults; + SettingsSaveWrapper wrapper(defaults_si); + defaults.LoadSaveCore(wrapper); + } + + MemorySettingsInterface database_si; + { + // Stands in for a game whose entry carries `gameFixes: [EETiming]`. + Pcsx2Config database; + database.Gamefixes.Set(Fix_EETiming, true); + SettingsSaveWrapper wrapper(database_si); + database.LoadSaveCore(wrapper); + } + + // The player turned the same fix on globally, and separately turned another one on + // that the database says nothing about. + Pcsx2Config source; + source.Gamefixes.Set(Fix_EETiming, true); + source.Gamefixes.Set(Fix_XGKick, true); + + MemorySettingsInterface dest; + { + SettingsSaveDeviationsWrapper wrapper(dest, {&defaults_si, &database_si}); + source.LoadSaveCore(wrapper); + } + + EXPECT_FALSE(dest.ContainsValue("EmuCore/Gamefixes", "EETimingHack")) + << "the database sets this one, so claiming it would suppress the fix it agrees with"; + EXPECT_TRUE(dest.ContainsValue("EmuCore/Gamefixes", "XgKickHack")) + << "the database says nothing about this one, so it is a real decision"; + EXPECT_EQ(CountKeys(dest, "EmuCore/Gamefixes"), 1u); +} + +TEST(SettingsCopyFilter, OnlyTheChangedValueIsWritten) +{ + MemorySettingsInterface source; + { + Pcsx2Config config; + config.Gamefixes.Set(Fix_EETiming, true); + SettingsSaveWrapper wrapper(source); + config.LoadSaveCore(wrapper); + } + + MemorySettingsInterface dest; + Pcsx2Config::CopyConfiguration(&dest, source, std::string_view()); + + EXPECT_EQ(CountKeys(dest, "EmuCore/Gamefixes"), 1u); + EXPECT_TRUE(dest.ContainsValue("EmuCore/Gamefixes", "EETimingHack")); + EXPECT_EQ(CountKeys(dest, "EmuCore/GS"), 0u); +}