armsx/ARMSX2
0
0
Fork 0
mirror of https://github.com/ARMSX2/ARMSX2.git synced 2026-08-24 16:50:16 -07:00
Code Issues Packages Projects Releases Wiki Activity

iOS: three defects the branch review found, two of them silent

Browse Source 
Seven lenses over the branch, each finding then handed to a skeptic told to
refute it rather than confirm it. Thirty-one were raised. These three
survived and matter, and two of them fail without saying anything, which is
why none of them turned up in a device pass over a green suite.

Per-game write() set the enabled key straight from the picker, before the
guard that needs the preset to resolve. Choose On, delete the pack the
preset came from, then save any unrelated row on that game: the file keeps
the chain enabled and loses both preset keys, and an absent key in the game
layer falls through to the base layer. That game then renders the GLOBAL
preset. The type's own first comment says this never happens and boot-time
repair has always got it right; write did not. It is the worst kind of wrong
because it is invisible -- one CRT shader looks like another, so the player
sees a filter and assumes it is theirs.

A per-game preset never received its saved parameter values at all.
SettingsStore pushes the global tier's overrides at launch and on every
change, but a per-game preset is chosen in a file SettingsStore never reads,
so the game rendered the shader author's defaults and every value saved
against that preset was ignored. The boot repair already resolves that token
before bootISO, which is the one place that knows both the token and the
timing; pushStored is nonisolated now so it can be called from there without
hopping actors and losing the ordering.

Save as New Preset could destroy the preset it was saving from. The
reference it writes is relative to My Presets and the sheet pre-fills the
base's own name, so selecting a saved preset, nudging a value and accepting
the default replaced that file with one whose only reference is its own
filename. Nothing resolves that, and the values it held are gone. It refuses
now, in the write path, which is the only place that can see both the target
and the base.
This commit is contained in:
J1coding
2026-08-18 23:41:53 +02:00
committed by Jeen
parent 30d9816eda
commit af30d18304
2 changed files with 28 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
platforms/ios/app/src/main/swift/Models/PerGameShaderSelection.swift
+11 -8
View File
@@ -8,15 +8,13 @@ import Foundation
enum PerGameShaderSelection {
static let section = "EmuCore/GS"
/// Written together and cleared together. The token is the durable identity and the absolute
/// is a cache of it, because both preset roots sit under a container UUID that moves.
/// Written and cleared together: the token is the identity, the absolute is its cache.
static let keys = (
enabled: "ShaderChainEnabled",
presetRef: "ShaderChainPresetRef",
presetPath: "ShaderChainPreset")
/// Re-roots a game's token against the container this launch got. Writes nothing for a game
/// that chose no shader, and nothing again when the absolute already agrees.
/// Re-roots a game's token against this launch's container, and pushes what it resolves to.
static func repair(forISO isoName: String) {
let token = string(keys.presetRef, useCurrent: false, iso: isoName)
guard !token.isEmpty else { return }
@@ -26,11 +24,13 @@ enum PerGameShaderSelection {
setBool(keys.enabled, false, useCurrent: false, iso: isoName)
return
}
// Nothing else pushes this one: the settings store never reads the per-game file.
ShaderParams.pushStored(token: token)
guard string(keys.presetPath, useCurrent: false, iso: isoName) != url.path else { return }
setString(keys.presetPath, url.path, useCurrent: false, iso: isoName)
}
/// -1 use global, 0 off, 1 on, on the sentinel every other per-game control already uses.
/// -1 use global, 0 off, 1 on -- the sentinel every other per-game control uses.
static func loadedChain(useCurrent: Bool, iso: String) -> Int {
guard has(keys.enabled, useCurrent: useCurrent, iso: iso) else { return -1 }
return bool(keys.enabled, useCurrent: useCurrent, iso: iso) ? 1 : 0
@@ -42,9 +42,12 @@ enum PerGameShaderSelection {
/// Off keeps the enabled key and drops the preset; absence is the answer that inherits.
static func write(chain: Int, presetRef: String, useCurrent: Bool, iso: String) {
setBool(keys.enabled, chain == 1, useCurrent: useCurrent, iso: iso)
guard chain == 1, !presetRef.isEmpty,
let url = ShaderPresetLibrary.resolve(presetRef) else {
// Enabled true with no preset key reads the GLOBAL preset out of the base layer.
let resolved = chain == 1 && !presetRef.isEmpty
? ShaderPresetLibrary.resolve(presetRef)
: nil
setBool(keys.enabled, resolved != nil, useCurrent: useCurrent, iso: iso)
guard let url = resolved else {
delete(keys.presetRef, useCurrent: useCurrent, iso: iso)
delete(keys.presetPath, useCurrent: useCurrent, iso: iso)
return
platforms/ios/app/src/main/swift/Models/ShaderParams.swift
+17 -9
View File
@@ -6,6 +6,7 @@ import Foundation
enum ShaderParamsError: LocalizedError {
case noName
case noSavedRoot
case wouldOverwriteBase
var errorDescription: String? {
switch self {
@@ -13,6 +14,8 @@ enum ShaderParamsError: LocalizedError {
return "That name has nothing in it that can become a filename."
case .noSavedRoot:
return "The Documents shader folder could not be opened."
case .wouldOverwriteBase:
return "That is the preset this one is built from. Give it a different name."
}
}
}
@@ -151,7 +154,7 @@ final class ShaderParams: ObservableObject {
let text = Self.presetText(base: base, params: params, overrides: overrides)
do {
let url = try await Task.detached(priority: .userInitiated) {
try Self.write(text, named: safe)
try Self.write(text, named: safe, base: base)
}.value
savedName = url.deletingPathExtension().lastPathComponent
} catch {
@@ -159,9 +162,8 @@ final class ShaderParams: ObservableObject {
}
}
/// Sends the effective value of EVERY parameter, not just the changed ones. librashader
/// has no unset call, so a name dropped from the map leaves the chain on whatever was
/// pushed last, and a reset would never take.
/// Sends EVERY parameter's effective value. librashader has no unset call, so a name
/// dropped from the map would leave the chain on whatever was pushed last.
private func pushEffective() {
guard !params.isEmpty, let url = ShaderPresetLibrary.resolve(token) else { return }
var effective: [String: NSNumber] = [:]
@@ -177,7 +179,8 @@ final class ShaderParams: ObservableObject {
/// the overrides go down, because a chain built from the preset file already holds that
/// preset's number for every name it is not told about, and reading the file here to say
/// so again would put a librashader parse on the launch path.
static func pushStored(token: String) {
/// nonisolated so the per-game boot path can push before bootISO, without hopping actors.
nonisolated static func pushStored(token: String) {
guard let url = ShaderPresetLibrary.resolve(token) else { return }
var values: [String: NSNumber] = [:]
for (name, value) in stored()[token] ?? [:] where value.isFinite {
@@ -196,7 +199,7 @@ final class ShaderParams: ObservableObject {
ARMSX2Bridge.setINIString(Self.section, key: Self.key, value: json)
}
private static func stored() -> [String: [String: Float]] {
private nonisolated static func stored() -> [String: [String: Float]] {
let json = ARMSX2Bridge.getINIString(section, key: key, defaultValue: "")
guard let data = json.data(using: .utf8),
let decoded = try? JSONDecoder()
@@ -230,8 +233,7 @@ final class ShaderParams: ObservableObject {
}
/// Relative while the base sits in the same Documents root, so the pair survives the
/// container UUID changing. A bundled base has no such route and gets a path that a
/// reinstall breaks, which is why the save sheet says the reference is a path.
/// container UUID moving. A bundled base gets a path instead, which a reinstall breaks.
private static func reference(to base: URL) -> String {
let target = base.standardizedFileURL
guard let root = ShaderPresetLibrary.userRoot?.standardizedFileURL,
@@ -245,7 +247,8 @@ final class ShaderParams: ObservableObject {
return (up + to[shared...]).joined(separator: "/")
}
private nonisolated static func write(_ text: String, named name: String) throws -> URL {
private nonisolated static func write(_ text: String, named name: String,
base: URL) throws -> URL {
guard ShaderPresetLibrary.prepareUserRoots() != nil,
let root = ShaderPresetLibrary.savedPresetRoot?.standardizedFileURL else {
throw ShaderParamsError.noSavedRoot
@@ -255,6 +258,11 @@ final class ShaderParams: ObservableObject {
guard url.deletingLastPathComponent().path == root.path else {
throw ShaderParamsError.noName
}
// Saving onto the base leaves a preset that references itself, and the sheet
// pre-fills the base's name, so the default is what walks into it.
guard url.path != base.standardizedFileURL.path else {
throw ShaderParamsError.wouldOverwriteBase
}
try text.write(to: url, atomically: true, encoding: .utf8)
return url
}