mirror of
https://github.com/ARMSX2/ARMSX2.git
synced 2026-08-24 16:50:16 -07:00
PINE: report what a setting is actually running as, not what the INI says
A settings query answered the wrong question. GameDB hardware fixes are applied to the live config after the settings load and are never written back to the file, so on any game carrying them the persisted value and the running value disagree — and the query only ever knew about the first. On Rogue Galaxy it reported autoflush off and preload off while the renderer was running autoflush at 2 and preload on. That cost real time during the Rogue Galaxy work. The confusion is the smaller half. The real damage is to measurement: a settings A/B that writes a key to some value measures the GameDB value in BOTH arms, because GameDB re-applies it after every settings load, while the two arms report two different settings. That is a wrong answer with no symptom, on exactly the titles worth investigating. So add an opcode that reports both values side by side. Effective values come from serialising the live config back out through the same wrapper that writes the INI, which means they land under the identical section/key names a caller already uses and every setting is covered without a key map — a hand-written map would need extending by every future setting, and the one that got missed would be the one somebody trusted. It also fixes a smaller lie: keys absent from the INI came back as empty strings, reading as "unset" rather than as their default. The reply says the two strings differ; it does not say why, because from here that is not knowable. A GameDB fix, safe-mode masking and a settings layer this query does not read are indistinguishable at the point of comparison, and naming one of them would be inventing the reason. The existing read is left alone, so anything speaking the old opcode keeps working. gsctl's `get` now reports the running value, prints the discrepancy to stderr where a human cannot miss it and a pipeline does not have to care, and keeps the on-disk value available behind --persisted.
This commit is contained in:
@@ -15,7 +15,10 @@
|
||||
#include "VMManager.h"
|
||||
#include "vtlb.h"
|
||||
#include "common/Error.h"
|
||||
#include "common/FPControl.h"
|
||||
#include "common/MemorySettingsInterface.h"
|
||||
#include "common/SettingsInterface.h"
|
||||
#include "common/SettingsWrapper.h"
|
||||
#include "common/Threading.h"
|
||||
|
||||
#include <atomic>
|
||||
@@ -173,6 +176,7 @@ namespace PINEServer
|
||||
MsgSetSetting = 0x12, /**< Writes a setting by section/key and applies it. */
|
||||
MsgFrameAdvance = 0x13, /**< Advances a paused VM by one frame. */
|
||||
MsgGSDump = 0x14, /**< Records a GS dump of the next N frames. */
|
||||
MsgGetEffectiveSetting = 0x15, /**< Reads what a setting is actually running as. */
|
||||
|
||||
MsgUnimplemented = 0xFF /**< Unimplemented IPC message. */
|
||||
};
|
||||
@@ -417,6 +421,65 @@ namespace PINEServer
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* Reports what a setting is actually running as, which on any game carrying GameDB fixes is
|
||||
* NOT what the INI says: applyGSHardwareFixes runs after the settings load and rewrites
|
||||
* EmuConfig in memory without ever touching the file. Reading the persisted value alone gets
|
||||
* you told that autoflush is off while the renderer is running it at 2.
|
||||
*
|
||||
* The measurement hazard is worse than the confusion. A settings A/B that writes a key to
|
||||
* "off" measures the GameDB value in BOTH arms -- because GameDB re-applies it after every
|
||||
* settings load -- while the two arms report different settings. That is a wrong answer with
|
||||
* no symptom, on exactly the titles worth investigating.
|
||||
*
|
||||
* Effective values come from serialising the live EmuConfig back out through the same wrapper
|
||||
* that writes the INI, so they land under the identical section/key names the caller already
|
||||
* uses, and every setting is covered for free. A hand-written key map would need extending by
|
||||
* every future setting, and the one that got missed would be the one somebody trusted.
|
||||
*
|
||||
* `known` false means the key is not part of Pcsx2Config at all -- EnableFastBoot, the UI
|
||||
* section, anything host-side. That is not an error: it says the persisted value is the whole
|
||||
* truth for that key, which is worth telling apart from "both agree".
|
||||
*
|
||||
* `differs` is deliberately a claim about the two STRINGS and not about the cause. Something
|
||||
* mutated the config after it was loaded -- a GameDB fix, safe-mode masking, or a settings
|
||||
* layer this query does not read -- and which one it was is not knowable from here. Naming it
|
||||
* "overridden" would be inventing the reason.
|
||||
*/
|
||||
static std::string BuildEffectiveSettingJson(const std::string& section, const std::string& key)
|
||||
{
|
||||
MemorySettingsInterface effective_si;
|
||||
{
|
||||
// Serialise a copy, so nothing the wrapper does can reach the live config. The FPCR
|
||||
// backup matches every other Pcsx2Config round-trip in the tree: the struct carries FP
|
||||
// control state, and this runs on the PINE thread, whose FPCR is its own to restore.
|
||||
FPControlRegisterBackup fpcr_backup(FPControlRegister::GetDefault());
|
||||
Pcsx2Config snapshot = EmuConfig;
|
||||
SettingsSaveWrapper wrapper(effective_si);
|
||||
snapshot.LoadSave(wrapper);
|
||||
}
|
||||
|
||||
std::string effective;
|
||||
const bool known = effective_si.GetStringValue(section.c_str(), key.c_str(), &effective);
|
||||
|
||||
std::string persisted;
|
||||
{
|
||||
auto lock = Host::GetSettingsLock();
|
||||
persisted = Host::GetSettingsInterface()->GetStringValue(section.c_str(), key.c_str(), "");
|
||||
}
|
||||
|
||||
// An unpersisted key reads back empty, which means "never written", not "set to empty".
|
||||
// Counting that as a difference would flag most of the config the moment anyone asked,
|
||||
// since the INI only stores what has been changed away from its default.
|
||||
const bool differs = known && !persisted.empty() && persisted != effective;
|
||||
|
||||
return fmt::format(
|
||||
"{{\"section\":\"{}\",\"key\":\"{}\",\"effective\":\"{}\",\"persisted\":\"{}\","
|
||||
"\"known\":{},\"differs\":{}}}",
|
||||
JsonEscape(section), JsonEscape(key), JsonEscape(effective), JsonEscape(persisted),
|
||||
known ? "true" : "false", differs ? "true" : "false");
|
||||
}
|
||||
|
||||
/**
|
||||
* Queues a GS dump of the next `frames` frames, or stops a dump already recording when
|
||||
* `frames` is zero -- the same pair of actions the GSDumpMultiFrame hotkey binds to press
|
||||
@@ -1045,6 +1108,23 @@ PINEServer::IPCBuffer PINEServer::ParseCommand(std::span<u8> buf, std::vector<u8
|
||||
ret_cnt += size;
|
||||
break;
|
||||
}
|
||||
case MsgGetEffectiveSetting:
|
||||
{
|
||||
std::string section, key;
|
||||
if (!ReadLengthPrefixedString(buf, buf_cnt, buf_size, §ion) ||
|
||||
!ReadLengthPrefixedString(buf, buf_cnt, buf_size, &key)) [[unlikely]]
|
||||
goto error;
|
||||
|
||||
const std::string reply = BuildEffectiveSettingJson(section, key);
|
||||
const u32 size = reply.size() + 1;
|
||||
if (!SafetyChecks(buf_cnt, 0, ret_cnt, size + 4, buf_size)) [[unlikely]]
|
||||
goto error;
|
||||
ToResultVector(ret_buffer, size, ret_cnt);
|
||||
ret_cnt += 4;
|
||||
memcpy(&ret_buffer[ret_cnt], reply.c_str(), size);
|
||||
ret_cnt += size;
|
||||
break;
|
||||
}
|
||||
case MsgSetSetting:
|
||||
{
|
||||
std::string section, key, value;
|
||||
|
||||
+53
-2
@@ -10,10 +10,18 @@ sitting on a savestate.
|
||||
|
||||
Requires `EmuCore/EnablePINE = true` in the INI (or the Big Picture UI toggle).
|
||||
|
||||
`get` reports what the emulator is ACTUALLY running, not what the INI says. On any
|
||||
game carrying GameDB hardware fixes those differ, because the fixes are applied to
|
||||
the live config after the settings load and never written to the file. That gap is
|
||||
also why a settings A/B can measure the same thing in both arms while reporting two
|
||||
different settings — `get` now says so on stderr when it sees one. Use --persisted
|
||||
for the on-disk value.
|
||||
|
||||
Examples:
|
||||
gsctl.py stats
|
||||
gsctl.py stats --watch 1.0
|
||||
gsctl.py get EmuCore/GS accurate_blending_unit
|
||||
gsctl.py get EmuCore/GS UserHacks_AutoFlushLevel --json
|
||||
gsctl.py set EmuCore/GS accurate_blending_unit 3
|
||||
gsctl.py loadstate 2
|
||||
gsctl.py frameadvance
|
||||
@@ -40,6 +48,7 @@ MSG_GET_STATS = 0x10
|
||||
MSG_GET_SETTING = 0x11
|
||||
MSG_SET_SETTING = 0x12
|
||||
MSG_FRAME_ADVANCE = 0x13
|
||||
MSG_GET_EFFECTIVE_SETTING = 0x15
|
||||
|
||||
IPC_OK = 0
|
||||
STATUS_NAMES = {0: "running", 1: "paused", 2: "shutdown"}
|
||||
@@ -133,10 +142,29 @@ class Pine:
|
||||
return STATUS_NAMES.get(raw, "unknown(%d)" % raw)
|
||||
|
||||
def get_setting(self, section, key):
|
||||
"""The PERSISTED value, straight from the INI layer stack.
|
||||
|
||||
This is not necessarily what the emulator is running -- see
|
||||
get_effective_setting. Kept because "what is on disk" is a real question,
|
||||
just rarely the one being asked.
|
||||
"""
|
||||
return self._read_string(
|
||||
self.request(MSG_GET_SETTING, lp_string(section) + lp_string(key))
|
||||
)
|
||||
|
||||
def get_effective_setting(self, section, key):
|
||||
"""What the setting is actually running as, plus the persisted value.
|
||||
|
||||
Returns section, key, effective, persisted, known, differs. `known` is
|
||||
False for keys outside Pcsx2Config, where the persisted value is all there
|
||||
is. `differs` compares the two strings and makes no claim about the cause.
|
||||
"""
|
||||
return json.loads(
|
||||
self._read_string(
|
||||
self.request(MSG_GET_EFFECTIVE_SETTING, lp_string(section) + lp_string(key))
|
||||
)
|
||||
)
|
||||
|
||||
def set_setting(self, section, key, value):
|
||||
payload = lp_string(section) + lp_string(key) + lp_string(str(value))
|
||||
return json.loads(self._read_string(self.request(MSG_SET_SETTING, payload)))
|
||||
@@ -172,9 +200,13 @@ def main():
|
||||
sub.add_parser("title", help="current game title")
|
||||
sub.add_parser("frameadvance", help="advance a paused VM by one frame")
|
||||
|
||||
p = sub.add_parser("get", help="read a setting")
|
||||
p = sub.add_parser("get", help="read what a setting is actually running as")
|
||||
p.add_argument("section")
|
||||
p.add_argument("key", nargs="?")
|
||||
p.add_argument("--persisted", action="store_true",
|
||||
help="read the INI instead of the live config (what 'get' used to do)")
|
||||
p.add_argument("--json", action="store_true",
|
||||
help="print the full record: effective, persisted, known, differs")
|
||||
|
||||
p = sub.add_parser("set", help="write a setting and apply it")
|
||||
p.add_argument("section")
|
||||
@@ -207,7 +239,26 @@ def main():
|
||||
elif args.cmd == "get":
|
||||
# Accept both 'get EmuCore/GS Key' and 'get EmuCore/GS/Key'.
|
||||
section, key = (args.section, args.key) if args.key else split_section_key(args.section)
|
||||
print(pine.get_setting(section, key))
|
||||
if args.persisted:
|
||||
print(pine.get_setting(section, key))
|
||||
else:
|
||||
record = pine.get_effective_setting(section, key)
|
||||
if args.json:
|
||||
print(json.dumps(record))
|
||||
else:
|
||||
# stdout stays a bare value so this still composes in a pipeline;
|
||||
# the discrepancy goes to stderr, where a human cannot miss it and
|
||||
# a script does not have to care.
|
||||
print(record["effective"] if record["known"] else record["persisted"])
|
||||
if record["differs"]:
|
||||
print(
|
||||
"note: %s/%s is running as '%s' but the INI says '%s' — something "
|
||||
"changed it after load (GameDB fix, safe-mode masking, or a settings "
|
||||
"layer this query does not read). An A/B that writes this key will "
|
||||
"measure the running value in BOTH arms."
|
||||
% (section, key, record["effective"], record["persisted"]),
|
||||
file=sys.stderr,
|
||||
)
|
||||
elif args.cmd == "set":
|
||||
if args.value is None:
|
||||
section, key = split_section_key(args.section)
|
||||
|
||||
Reference in New Issue
Block a user