Release: 0.9.3.1, and make upload signing opt-in

The ISO reader revert is confirmed working by the reporter, so this ships it.

Upload signing is now requested explicitly with -Parmsx3.uploadSigning rather
than used whenever keystore.properties happens to exist. Once that file was
created, every release build silently started signing with the upload key, and
an APK signed differently from the one already installed cannot be installed
over it -- so a sideload build becomes something testers cannot install, and
Android's error does not mention signatures.

It was being worked around by hiding keystore.properties by hand before each
build, which is the kind of step that gets forgotten exactly once and then
wastes a tester's evening. build-play-aab.sh passes the flag; nothing else
does, and asking for it without the keystore present is now an error rather
than a silent fallback.

android/armsx3-ui/app/build.gradle.kts

@@ -1,3 +1,5 @@
+import java.util.Properties
+
 plugins {
     alias(libs.plugins.android.application)
     alias(libs.plugins.compose.compiler)
@@ -32,19 +34,21 @@ android {
         // agree -- an APK that installs below its core's target is a dlopen failure at boot.
         minSdk = (project.findProperty("armsx3.minSdk") as String?)?.toInt() ?: 33
         targetSdk = 37
-        versionCode = 18
-        versionName = "0.9.3"
+        versionCode = 19
+        versionName = "0.9.3.1"
 
         // ARMSX2's UI reads these. STORAGE_ALL_FILES gates the all-files storage path in
         // onboarding; IN_APP_UPDATER gates the in-app GitHub-release updater.
         //
-        // On because ARMSX3 ships as a sideloaded APK from its own GitHub releases, which is
-        // exactly the case an in-app updater is for. It must go back off, and the code and the
-        // REQUEST_INSTALL_PACKAGES permission must move into a github-only flavor, before any
-        // Play build exists: Play forbids self-updating apps, and it is the PERMISSION in the
-        // bundle that gets rejected, which this runtime flag does nothing about.
+        // These are the github values; the play flavor overrides all three below.
+        //
+        // The warning that used to live here was right and is now acted on: a runtime boolean
+        // does nothing about the PERMISSION in the bundle, which is what Play rejects. The
+        // permissions have moved into the github flavor's manifest, so the play bundle does not
+        // declare them at all.
         buildConfigField("boolean", "STORAGE_ALL_FILES", "true")
         buildConfigField("boolean", "IN_APP_UPDATER", "true")
+        buildConfigField("boolean", "FRAME_GENERATION", "true")
 
         ndk {
             // The core is arm64-only.
@@ -68,6 +72,40 @@ android {
         }
     }
 
+    // Two distributions, and they are not interchangeable.
+    //
+    // github is the sideloaded build: it updates itself from GitHub releases, can be pointed at
+    // an arbitrary data folder, and ships frame generation.
+    //
+    // play is what Google Play will accept. Self-updating is forbidden outright, all-files
+    // storage is a policy review it does not need, and frame generation is left out. The
+    // applicationId differs so the two install side by side instead of over each other.
+    flavorDimensions += "distribution"
+
+    productFlavors {
+        create("github") {
+            dimension = "distribution"
+        }
+
+        create("play") {
+            dimension = "distribution"
+            applicationId = "com.armsx3.play"
+
+            buildConfigField("boolean", "STORAGE_ALL_FILES", "false")
+            buildConfigField("boolean", "IN_APP_UPDATER", "false")
+            buildConfigField("boolean", "FRAME_GENERATION", "false")
+
+            // Frame generation is excluded by SOURCE SET, not by a packaging filter: a
+            // packaging block inside a flavor is not honoured and silently applied to both,
+            // which dropped the library from the github build too. libarmsx3_lsfg.so lives in
+            // src/github/jniLibs, so only that flavor bundles it.
+            //
+            // Excluding the file is the whole exclusion. The shim is dlopen'd by name, and the
+            // core already reports frame generation unavailable when the library is absent,
+            // which is the same path a device that cannot run it takes.
+        }
+    }
+
     externalNativeBuild {
         cmake {
             path = file("src/main/cpp/CMakeLists.txt")
@@ -75,17 +113,73 @@ android {
         }
     }
 
+    // Reads android/armsx3-ui/keystore.properties when it exists:
+    //
+    //     storeFile=/absolute/path/to/upload.jks
+    //     storePassword=...
+    //     keyAlias=upload
+    //     keyPassword=...
+    //
+    // Absent, only the debug key exists and release builds stay sideload-only. The file is
+    // gitignored and nothing here echoes its contents.
+    signingConfigs {
+        val props = rootProject.file("keystore.properties")
+
+        if (props.exists()) {
+            val k = Properties().apply { props.inputStream().use { load(it) } }
+
+            create("upload") {
+                storeFile = file(k.getProperty("storeFile"))
+                storePassword = k.getProperty("storePassword")
+                keyAlias = k.getProperty("keyAlias")
+                keyPassword = k.getProperty("keyPassword")
+            }
+        }
+    }
+
     buildTypes {
         release {
-            isMinifyEnabled = true
-            isShrinkResources = true
+            // Off for the Play bundle, on for GitHub APKs.
+            //
+            // Not a preference: AGP 9.2.1's R8 writes its mapping as mapping.prt, a compressed
+            // per-class archive, while packageBundle still demands a plain mapping.txt, so an
+            // AAB cannot be built with R8 enabled at all. Set by build-play-aab.sh.
+            //
+            // The cost is small and there is precedent: ARMSX2 ships its Play build with minify
+            // off entirely, and here a 94 MB native core dominates a 76 MB APK, so shrinking the
+            // Kotlin saves comparatively little.
+            //
+            // A gradle property rather than the variant API, matching how armsx3.minSdk is
+            // already threaded through by build-variants.sh.
+            val noMinify = project.hasProperty("armsx3.noMinify")
+            isMinifyEnabled = !noMinify
+            isShrinkResources = !noMinify
             proguardFiles(
                 getDefaultProguardFile("proguard-android-optimize.txt"),
                 "proguard-rules.pro"
             )
-            // Debug-signed so alpha release builds are sideloadable without the
-            // upload key. Swap this for the real config before any public build.
-            signingConfig = signingConfigs.getByName("debug")
+            // The upload key when one is configured, the debug key otherwise.
+            //
+            // GitHub APKs are deliberately debug-signed so an alpha stays sideloadable without
+            // the upload key present. Play rejects a debug-signed bundle outright, so
+            // build-play-aab.sh refuses to run without keystore.properties.
+            //
+            // The file is gitignored (*.jks, keystore.properties) and read at build time, so no
+            // credential is ever in the repo or on a command line.
+            // The upload key ONLY when explicitly asked for, which build-play-aab.sh does.
+            //
+            // Opt-in rather than "use it if it exists": once the keystore was created, every
+            // release build silently started using it, and a differently-signed APK cannot be
+            // installed over an existing one. That turns a sideload build into something testers
+            // cannot install, and the error Android shows says nothing about signatures. It was
+            // being worked around by hiding keystore.properties by hand before each build, which
+            // is exactly the kind of step that gets forgotten once.
+            signingConfig = if (project.hasProperty("armsx3.uploadSigning")) {
+                signingConfigs.findByName("upload")
+                    ?: throw GradleException("armsx3.uploadSigning set but keystore.properties is missing")
+            } else {
+                signingConfigs.getByName("debug")
+            }
         }
     }
 

android/armsx3-ui/app/src/github/AndroidManifest.xml

@@ -0,0 +1,35 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+    Everything Google Play will not accept, declared by the github flavor alone so the play
+    bundle cannot inherit it by accident.
+
+    REQUEST_INSTALL_PACKAGES and the FileProvider are what the in-app updater needs: it downloads
+    a GitHub release APK and hands it to the system package installer. Play forbids apps that
+    update themselves outside the store, and it is the DECLARED PERMISSION that gets rejected, so
+    gating the code behind a runtime flag was never enough on its own.
+
+    MANAGE_EXTERNAL_STORAGE backs the all-files onboarding path, which lets the data folder live
+    anywhere on the device. The play build uses the app-specific directories instead (internal or
+    SD card), which are raw-writable under scoped storage with no permission at all.
+-->
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:tools="http://schemas.android.com/tools">
+
+    <uses-permission android:name="android.permission.REQUEST_INSTALL_PACKAGES" />
+
+    <uses-permission android:name="android.permission.MANAGE_EXTERNAL_STORAGE"
+        tools:ignore="ScopedStorage" />
+
+    <application>
+        <!-- Hands the downloaded update APK to the system package installer. -->
+        <provider
+            android:name="androidx.core.content.FileProvider"
+            android:authorities="${applicationId}.updateprovider"
+            android:exported="false"
+            android:grantUriPermissions="true">
+            <meta-data
+                android:name="android.support.FILE_PROVIDER_PATHS"
+                android:resource="@xml/update_paths" />
+        </provider>
+    </application>
+</manifest>

android/armsx3-ui/app/src/main/AndroidManifest.xml

@@ -23,16 +23,10 @@
          Sideload/GitHub builds only. The Play flavour must NOT ship this (the
          policy needs a declared exemption); that is what the STORAGE_ALL_FILES
          buildConfig flag gates in code. -->
-    <!-- In-app updater: install the downloaded APK. SIDELOAD ONLY.
-         A self-updating app is a hard Play-policy violation, and it is this permission in the
-         bundle that gets rejected, not the runtime flag. ARMSX2 keeps it out of its Play build
-         with a github-only flavor and a build script that fails closed if it ever appears;
-         ARMSX3 has no Play build, so it lives here. Adding a Play target means moving this and
-         the provider below into a github flavor FIRST. -->
-    <uses-permission android:name="android.permission.REQUEST_INSTALL_PACKAGES" />
-
-    <uses-permission android:name="android.permission.MANAGE_EXTERNAL_STORAGE"
-        tools:ignore="ScopedStorage" />
+    <!-- REQUEST_INSTALL_PACKAGES, MANAGE_EXTERNAL_STORAGE and the updater's FileProvider are
+         declared by the GITHUB flavor only, in src/github/AndroidManifest.xml. Play rejects the
+         permission present in the bundle, not the code path behind a runtime flag, so none of
+         them may sit here where both flavors inherit them. -->
 
     <!-- Optional motion controls (Pad settings). Not required, so the Play install
          isn't gated for devices without a gyroscope. -->
@@ -204,18 +198,6 @@
             android:theme="@android:style/Theme.Translucent.NoTitleBar"
             android:excludeFromRecents="true"
             android:exported="false" />
-    
-        <!-- Hands the downloaded update APK to the system package installer. Paired with
-             REQUEST_INSTALL_PACKAGES above; see the note there before shipping to Play. -->
-        <provider
-            android:name="androidx.core.content.FileProvider"
-            android:authorities="${applicationId}.updateprovider"
-            android:exported="false"
-            android:grantUriPermissions="true">
-            <meta-data
-                android:name="android.support.FILE_PROVIDER_PATHS"
-                android:resource="@xml/update_paths" />
-        </provider>
 
 </application>
 

android/armsx3-ui/app/src/main/java/com/armsx2/ui/emulation/EmulationMenuScreen.kt

@@ -1040,18 +1040,21 @@ private fun PerformancePane(state: EmulationMenuUiState, viewModel: EmulationMen
     // recompiler toggles for silicon that does not exist here.
     // Frame generation first: it is the one setting here that changes the framerate rather than
     // how fast the emulator runs, so it is what someone opening this menu mid-game is looking for.
-    SectionCard(str("perf.framegen.title")) {
-        HorizontalOptions(
-            title = str("perf.framegen.label"),
-            options = listOf(
-                str("perf.framegen.off"), str("perf.framegen.x2"),
-                str("perf.framegen.x3"), str("perf.framegen.x4"),
-            ).mapIndexed { index, label -> index to label },
-            selected = settings.ps3.frameGeneration,
-            onSelect = { v -> viewModel.updateSettings { it.copy(ps3 = it.ps3.copy(frameGeneration = v)) } },
-        )
+    // Not in the play build: libarmsx3_lsfg.so is not bundled there, so this would be inert.
+    if (com.armsx2.BuildConfig.FRAME_GENERATION) {
+        SectionCard(str("perf.framegen.title")) {
+            HorizontalOptions(
+                title = str("perf.framegen.label"),
+                options = listOf(
+                    str("perf.framegen.off"), str("perf.framegen.x2"),
+                    str("perf.framegen.x3"), str("perf.framegen.x4"),
+                ).mapIndexed { index, label -> index to label },
+                selected = settings.ps3.frameGeneration,
+                onSelect = { v -> viewModel.updateSettings { it.copy(ps3 = it.ps3.copy(frameGeneration = v)) } },
+            )
+        }
+        Spacer(Modifier.height(10.dp))
     }
-    Spacer(Modifier.height(10.dp))
     SectionCard(str("perf.ps3cpu.title")) {
         HorizontalOptions(
             title = str("perf.ppuDecoder.label"),

android/armsx3-ui/app/src/main/java/com/armsx2/ui/settings/PerformanceTab.kt

@@ -407,6 +407,10 @@ fun PerformanceTab(state: MutableState<Settings>) {
         // Frame generation. Its own section rather than folded into the GPU one because it is
         // not a rendering option -- it inserts frames that the game never drew, and the choice
         // to do that is a different kind of decision from how the real ones are drawn.
+        // Absent from the play build, where libarmsx3_lsfg.so is not in the bundle. The core
+        // reports frame generation unavailable without it and every control here would be inert,
+        // so showing them would only offer something that cannot work.
+        if (com.armsx2.BuildConfig.FRAME_GENERATION)
         CollapsibleSection(str("perf.framegen.title")) {
             SegmentedGridRow(
                 label = str("perf.framegen.label"),

android/armsx3-ui/app/src/play/java/com/armsx2/update/UpdaterStubs.kt

@@ -0,0 +1,21 @@
+package com.armsx2.update
+
+import androidx.compose.runtime.Composable
+
+/**
+ * No-op replacements for the in-app updater, for the Play build.
+ *
+ * Google Play forbids an app that updates itself from outside the store, and the rejection is on
+ * the REQUEST_INSTALL_PACKAGES permission being present in the bundle -- not on whether the code
+ * behind it ever runs. So the real implementation lives in src/github and this stands in its
+ * place here, which keeps the permission, the FileProvider and the download-and-install code out
+ * of the play bundle entirely rather than merely unreachable.
+ *
+ * Both call sites are already guarded by BuildConfig.IN_APP_UPDATER, which is false for this
+ * flavor, so neither of these is reached. They exist so the play source set still compiles.
+ */
+@Composable
+fun UpdaterEntry() = Unit
+
+@Composable
+fun AutoUpdateGate() = Unit

android/build-play-aab.sh

@@ -0,0 +1,180 @@
+#!/usr/bin/env bash
+#
+# Build the Google Play bundle, and refuse to produce one that Play would reject.
+#
+# The legacy variant is what goes to Play: armv8.1-a and minSdk 30 is the widest floor ARMSX3
+# has, and Play serves one bundle to every device, so the lowest floor reaches the most people.
+# targetSdk is 37 for every variant, so the API-level requirement is met regardless.
+#
+# The checks below are the point of this script. A flavor split is only as good as the thing
+# that notices when it stops working, and every one of these has a specific failure behind it:
+#
+#   REQUEST_INSTALL_PACKAGES  a self-updating app is a hard policy violation, and it is the
+#                             declared permission that gets rejected, not the code path
+#   MANAGE_EXTERNAL_STORAGE   all-files access needs a declared exemption the app does not need
+#   RECORD_AUDIO              would put "Microphone" on the listing for a capability not shipped
+#   libarmsx3_lsfg.so         frame generation is not distributed through Play
+#   updateprovider            the FileProvider that hands the downloaded APK to the installer
+#
+# Fails closed: if a check cannot run, that is a failure too.
+set -euo pipefail
+
+HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+UI="$HERE/armsx3-ui"
+: "${OUT_DIR:=$HOME/Downloads}"
+: "${ANDROID_HOME:=$HOME/Library/Android/sdk}"
+
+MIN_SDK="${PLAY_MIN_SDK:-30}"
+
+# Gradle needs a JDK and the shell this is run from may not have one on PATH. Android Studio
+# ships one; fall back to it rather than failing several steps later with "Unable to locate a
+# Java Runtime", which does not point at the cause.
+if [ -z "${JAVA_HOME:-}" ]; then
+	for candidate in \
+		"/Applications/Android Studio.app/Contents/jbr/Contents/Home" \
+		"$(/usr/libexec/java_home 2>/dev/null || true)"
+	do
+		[ -x "$candidate/bin/java" ] && { export JAVA_HOME="$candidate"; break; }
+	done
+fi
+[ -n "${JAVA_HOME:-}" ] || { echo "FAIL: no JDK found; set JAVA_HOME" >&2; exit 1; }
+export PATH="$JAVA_HOME/bin:$PATH"
+
+# Refuse to build at all without an upload key. A debug-signed bundle is rejected by Play, and
+# finding that out at upload time after a fifteen-minute build is a poor way to learn it.
+if [ ! -f "$UI/keystore.properties" ] || grep -q "REPLACE_ME\|REPLACE_WITH_ABSOLUTE_PATH" "$UI/keystore.properties"; then
+	cat >&2 <<'MSG'
+FAIL: upload key not configured (file missing, or placeholders not filled in).
+
+Create android/armsx3-ui/keystore.properties with:
+
+    storeFile=/absolute/path/to/upload.jks
+    storePassword=...
+    keyAlias=upload
+    keyPassword=...
+
+and generate the key itself with:
+
+    keytool -genkeypair -v -keystore upload.jks -alias upload \
+        -keyalg RSA -keysize 4096 -validity 10000
+
+Keep upload.jks and its passwords safe and backed up: Play ties the listing to this key
+and losing it means losing the ability to update the app. Both the keystore and the
+properties file are gitignored.
+MSG
+	exit 1
+fi
+
+# armsx3.noMinify because AGP 9.2.1 cannot bundle with R8 on: R8 writes mapping.prt, a
+# compressed per-class archive, and packageBundle demands a plain mapping.txt. ARMSX2 ships
+# its Play build with minify off too, so this is the existing precedent rather than a new
+# compromise.
+# Stage the LEGACY core, and do not trust whatever happens to be in jniLibs.
+#
+# build-variants.sh writes each variant's core to the same path in turn, so the file left there
+# is simply whichever variant ran last. A bundle built on top of that would ship an armv8.2 core
+# with minSdk 30 -- installable on devices that cannot execute it, and failing at dlopen with
+# nothing to explain why. Play serves one bundle to every device, so the ISA floor has to be the
+# lowest one ARMSX3 supports.
+CORE_SRC="$(cd "$HERE/.." && pwd)/build-legacy/android/libarmsx3-core.so"
+JNI="$UI/app/src/main/jniLibs/arm64-v8a"
+
+if [ ! -f "$CORE_SRC" ]; then
+	echo "FAIL: legacy core not built. Run: ninja -C build-legacy android/libarmsx3-core.so" >&2
+	exit 1
+fi
+
+NDK_DIR="$(ls -d "$ANDROID_HOME/ndk/"*/ 2>/dev/null | sort -V | tail -1)"
+STRIP="${NDK_DIR}toolchains/llvm/prebuilt/darwin-x86_64/bin/llvm-strip"
+[ -x "$STRIP" ] || { echo "FAIL: llvm-strip not found under $ANDROID_HOME/ndk" >&2; exit 1; }
+
+# Same reason as build-variants.sh: cmake only regenerates this at configure time.
+bash "$HERE/stamp-git-version.sh"
+
+echo "==> Staging the legacy core"
+mkdir -p "$JNI"
+"$STRIP" --strip-unneeded -o "$JNI/libarmsx3-core.so" "$CORE_SRC"
+
+echo "==> Building Play bundle (minSdk $MIN_SDK, minify off)"
+( cd "$UI" && ./gradlew --quiet :app:bundlePlayRelease "-Parmsx3.minSdk=$MIN_SDK" -Parmsx3.noMinify -Parmsx3.uploadSigning )
+
+AAB="$UI/app/build/outputs/bundle/playRelease/app-play-release.aab"
+[ -f "$AAB" ] || { echo "FAIL: no bundle produced at $AAB" >&2; exit 1; }
+
+echo "==> Verifying the bundle"
+
+# An AAB is a ZIP and its entries are COMPRESSED, so grepping the archive itself finds
+# nothing and reports a clean bundle no matter what is in it. That is not a theoretical
+# risk: the first version of this script did exactly that and passed every check while
+# also failing to find the applicationId it was supposed to find, which is what gave it
+# away. Extract first, then inspect the manifest and the file list.
+WORK="$(mktemp -d)"
+trap 'rm -rf "$WORK"' EXIT
+unzip -q -o "$AAB" -d "$WORK"
+
+MANIFEST="$WORK/base/manifest/AndroidManifest.xml"
+[ -f "$MANIFEST" ] || { echo "FAIL: no manifest in the bundle" >&2; exit 1; }
+
+fail=0
+
+# The manifest is protobuf-encoded, so permission names appear as plain strings inside it
+# but the file contains NUL bytes -- LC_ALL=C and -a keep grep from calling it binary and
+# silently saying nothing, which reads exactly like a pass.
+check_absent_manifest() {
+	local needle="$1" why="$2"
+	if LC_ALL=C grep -aqF "$needle" "$MANIFEST"; then
+		echo "FAIL: '$needle' is declared in the bundle -- $why" >&2
+		fail=1
+	else
+		echo "  ok: $needle absent from the manifest"
+	fi
+}
+
+check_absent_manifest "REQUEST_INSTALL_PACKAGES" "Play forbids self-updating apps"
+check_absent_manifest "MANAGE_EXTERNAL_STORAGE"  "all-files access needs a declared exemption"
+check_absent_manifest "RECORD_AUDIO"             "would add Microphone to the listing"
+check_absent_manifest "updateprovider"           "the updater FileProvider must not ship"
+
+# Native libraries are entries in the archive, so check the listing rather than the bytes.
+#
+# Captured ONCE into a variable rather than piped into each grep. Under `set -o pipefail`,
+# `unzip -l | grep -q x` fails whenever x IS found: grep exits at the first match, closes the
+# pipe, unzip takes SIGPIPE, and the pipeline reports failure. That inverted every positive
+# check -- it reported the core library missing from a bundle that plainly contained it, while
+# the absence checks passed for the wrong reason, because grep read to the end and found
+# nothing.
+LISTING="$(unzip -l "$AAB")"
+
+# Matched with `case`, not with a pipe into grep. Under `set -o pipefail` any `... | grep -q x`
+# FAILS when x is found: grep exits at the first match, closes the pipe, and whatever is feeding
+# it takes SIGPIPE. That inverted every positive check -- the core library was reported missing
+# from a bundle that plainly contained it, while the absence checks passed for the wrong reason,
+# because grep read to the end and found nothing. Piping printf instead of unzip moved the
+# broken pipe rather than removing it; case has no subprocess to signal.
+if [[ "$LISTING" == *libarmsx3_lsfg.so* ]]; then
+	echo "FAIL: libarmsx3_lsfg.so is in the bundle -- frame generation is not shipped through Play" >&2
+	fail=1
+else
+	echo "  ok: libarmsx3_lsfg.so absent"
+fi
+
+# And the things that MUST be there.
+if LC_ALL=C grep -aqF "com.armsx3.play" "$MANIFEST"; then
+	echo "  ok: applicationId is com.armsx3.play"
+else
+	echo "FAIL: applicationId com.armsx3.play not in the manifest -- wrong flavor built?" >&2
+	fail=1
+fi
+
+if [[ "$LISTING" == *libarmsx3-core.so* ]]; then
+	echo "  ok: core library present"
+else
+	echo "FAIL: libarmsx3-core.so missing from the bundle" >&2
+	fail=1
+fi
+
+[ "$fail" -eq 0 ] || { echo "==> REFUSING to ship this bundle" >&2; exit 1; }
+
+OUT="$OUT_DIR/ARMSX3-$(sed -n 's/.*versionName = "\([^"]*\)".*/\1/p' "$UI/app/build.gradle.kts" | head -1)-play.aab"
+cp "$AAB" "$OUT"
+echo "==> $OUT"

android/build-variants.sh

@@ -67,6 +67,10 @@ export ANDROID_HOME JAVA_HOME
 CMAKE_BIN="$ANDROID_HOME/cmake/$CMAKE_VERSION/bin"
 UI="$ROOT/android/armsx3-ui"
 JNI_LIBS="$UI/app/src/main/jniLibs/arm64-v8a"
+# Frame generation ships in the github flavor only, so its library lives in that source set.
+# The play bundle must not contain it: excluding the file IS the exclusion, because the core
+# dlopen's it by name and reports the feature unavailable when it is absent.
+JNI_LIBS_GITHUB="$UI/app/src/github/jniLibs/arm64-v8a"
 
 # variant : ndk : api : march : apk name suffix
 #
@@ -155,14 +159,15 @@ build_variant() {
 	local lsfg_so="$build_dir/3rdparty/lsfg/libarmsx3_lsfg.so"
 
 	if [[ -f "$lsfg_so" ]]; then
-		"$strip" --strip-unneeded -o "$JNI_LIBS/libarmsx3_lsfg.so" "$lsfg_so"
+		mkdir -p "$JNI_LIBS_GITHUB"
+		"$strip" --strip-unneeded -o "$JNI_LIBS_GITHUB/libarmsx3_lsfg.so" "$lsfg_so"
 
 		# The isolation is the whole design, so verify it every build rather than trusting it.
 		# Only the shim's own entry points may be dynamic: a single leaked vk* symbol means the
 		# dynamic linker can bind the renderer's Vulkan calls to framegen's copies.
 		local leaked
 		leaked=$("$ANDROID_HOME/ndk/$ndk/toolchains/llvm/prebuilt/darwin-x86_64/bin/llvm-nm" \
-			-D --defined-only "$JNI_LIBS/libarmsx3_lsfg.so" 2>/dev/null | grep -cE "vk[A-Z]|LSFG" || true)
+			-D --defined-only "$JNI_LIBS_GITHUB/libarmsx3_lsfg.so" 2>/dev/null | grep -cE "vk[A-Z]|LSFG" || true)
 
 		if [[ "$leaked" != "0" ]]; then
 			echo "$name: libarmsx3_lsfg.so exports $leaked Vulkan/LSFG symbols -- isolation broken," \
@@ -171,16 +176,22 @@ build_variant() {
 		fi
 	else
 		echo "$name: libarmsx3_lsfg.so was not built, frame generation will be absent from this APK" >&2
-		rm -f "$JNI_LIBS/libarmsx3_lsfg.so"
+		rm -f "$JNI_LIBS_GITHUB/libarmsx3_lsfg.so"
 	fi
 
-	( cd "$UI" && ./gradlew --quiet :app:assembleRelease "-Parmsx3.minSdk=$api" )
+	# assembleGithubRelease, not assembleRelease: the flavor split means there is no
+	# flavorless release variant any more. The play bundle is built by build-play-aab.sh.
+	( cd "$UI" && ./gradlew --quiet :app:assembleGithubRelease "-Parmsx3.minSdk=$api" )
 
 	local out="$OUT_DIR/ARMSX3-$(version_name)-$suffix.apk"
-	cp "$UI/app/build/outputs/apk/release/app-release.apk" "$out"
+	cp "$UI/app/build/outputs/apk/github/release/app-github-release.apk" "$out"
 	echo "==> $name: $out"
 }
 
+# Stamp the version before anything builds, or every APK reports whichever commit cmake
+# last configured against rather than the one being built.
+bash "$ROOT/android/stamp-git-version.sh"
+
 for v in $VARIANTS; do
 	build_variant "$v"
 done

android/stamp-git-version.sh

@@ -0,0 +1,40 @@
+#!/usr/bin/env bash
+#
+# Refresh rpcs3/git-version.h from the current HEAD.
+#
+# cmake generates this header, but only when it CONFIGURES, and build-variants.sh deliberately
+# skips reconfiguring an already-correct build dir because re-running cmake regenerates LLVM's
+# generated headers and costs a full rebuild. So the version stamp froze at whenever cmake last
+# ran, and every build after that reported an old commit.
+#
+# That is not cosmetic. A tester on 0.9.3 reported a 0.9.1-era commit in their log, which sent
+# an investigation looking for a regression in commits their build did not contain. A build that
+# misreports itself makes every bug report ambiguous.
+#
+# Writing the header directly is enough: ninja sees it change and rebuilds only what includes it.
+set -euo pipefail
+
+ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+OUT="$ROOT/rpcs3/git-version.h"
+
+cd "$ROOT"
+COUNT="$(git rev-list HEAD --count)"
+SHA="$(git rev-parse --short=8 HEAD)"
+BRANCH="$(git rev-parse --abbrev-ref HEAD)"
+
+NEW="// This is a generated file.
+
+#define RPCS3_GIT_VERSION \"${COUNT}-${SHA}\"
+#define RPCS3_GIT_BRANCH \"${BRANCH}\"
+#define RPCS3_GIT_FULL_BRANCH \"local_build\"
+
+// If you don't want this file to update/recompile, change to 1.
+#define RPCS3_GIT_VERSION_NO_UPDATE 0"
+
+# Only write when it differs, so an unchanged HEAD does not force a rebuild.
+if [ ! -f "$OUT" ] || [ "$(cat "$OUT")" != "$NEW" ]; then
+	printf '%s' "$NEW" > "$OUT"
+	echo "==> git-version.h: ${COUNT}-${SHA} (${BRANCH})"
+else
+	echo "==> git-version.h already current: ${COUNT}-${SHA}"
+fi

rpcs3/Emu/System.cpp

@@ -760,7 +760,19 @@ void Emulator::Init()
 				// Finalize interrupted saving
 				if (!fs::rename(pending, save_path + desired, false))
 				{
-					sys_log.fatal("Failed to fix save data: %s (%s)", pending, fs::g_tls_error);
+					// Not fatal, and saying so was actively misleading: the loop continues, the
+					// emulator starts, and only this one save is left half-finished. Reported as a
+					// crash on Android because a fatal line in logcat reads like one.
+					//
+					// It is also permanent when it happens here rather than transient. Android
+					// storage can refuse a directory rename outright -- errno 1, EPERM -- and
+					// nothing about starting the emulator again changes that, so this fires on
+					// every launch and looks like a fault that is getting worse. The message says
+					// what to do instead of repeating an alarm.
+					sys_log.error("Could not finish an interrupted save: %s (%s). The game will "
+						"start; that one save is left as it was. If this repeats every launch, "
+						"delete the .working_ and .backup_ folders for it under "
+						"dev_hdd0/home/%s/savedata/.", pending, fs::g_tls_error, m_usr);
 					continue;
 				}
 
@@ -770,7 +782,11 @@ void Emulator::Init()
 			// Remove pending backup data
 			if (!fs::remove_all(save_path + entry.name))
 			{
-				sys_log.fatal("Failed to remove save data backup: %s%s (%s)", save_path, entry.name, fs::g_tls_error);
+				// Same reasoning as above: a backup that cannot be deleted costs disk space and
+				// nothing else, and the emulator carries on regardless.
+				sys_log.error("Could not remove a leftover save backup: %s%s (%s). Harmless, but "
+					"it will be reported again on every launch until the folder is deleted.",
+					save_path, entry.name, fs::g_tls_error);
 			}
 			else
 			{

rpcs3/Loader/ISO.cpp

@@ -3,7 +3,6 @@
 #include "ISO.h"
 #include "Emu/VFS.h"
 #include "Emu/system_utils.hpp"
-#include "Emu/System.h"
 #include "Crypto/utils.h"
 
 #include <codecvt>
@@ -11,7 +10,6 @@
 #include <cmath>
 #include <filesystem>
 #include <stack>
-#include <span>
 #include <cstdlib>
 
 LOG_CHANNEL(sys_log, "SYS");
@@ -53,7 +51,7 @@ static void* get_aligned_buf()
 		}
 	} s_aligned_buf {};
 
-	return ensure(s_aligned_buf.buf);
+	return s_aligned_buf.buf;
 }
 
 static bool is_iso_file(iso_file& file, u64* size = nullptr)
@@ -65,10 +63,7 @@ static bool is_iso_file(iso_file& file, u64* size = nullptr)
 
 	char magic[5];
 
-	if (!file.read_at(32768ULL + 1, magic, 5) == 5)
-	{
-		return false;
-	}
+	file.read_at(32768ULL + 1, magic, 5);
 
 	const bool ret = magic[0] == 'C' && magic[1] == 'D' && magic[2] == '0' && magic[3] == '0' && magic[4] == '1';
 
@@ -107,6 +102,12 @@ bool is_iso_file(const std::string& path, u64* size, bool* is_raw_device)
 	return is_iso_file(file, size);
 }
 
+// Convert 4 bytes in big-endian format to an unsigned integer
+static u32 char_arr_BE_to_uint(const u8* arr)
+{
+	return arr[0] << 24 | arr[1] << 16 | arr[2] << 8 | arr[3];
+}
+
 // Reset the iv to a particular LBA
 static void reset_iv(std::array<u8, 16>& iv, u32 lba)
 {
@@ -119,7 +120,7 @@ static void reset_iv(std::array<u8, 16>& iv, u32 lba)
 }
 
 // Main function that will decrypt the sector(s)
-static bool decrypt_data(aes_context& aes, u64 offset, const std::span<u8> buffer, const std::span<u8> out_buffer, u64 size)
+static bool decrypt_data(aes_context& aes, u64 offset, const unsigned char* buffer, unsigned char* out_buffer, u64 size)
 {
 	// The following preliminary checks are good to be provided.
 	// Commented out to gain a bit of performance, just because we know the caller is providing values in the expected range
@@ -141,14 +142,15 @@ static bool decrypt_data(aes_context& aes, u64 offset, const std::span<u8> buffe
 
 	std::array<u8, 16> iv;
 	u64 cur_offset;
+	u64 cur_size;
 
 	// If the offset is not at the beginning of a sector, the first 16 bytes in the buffer
 	// represents the IV for decrypting the next data in the buffer.
 	// Otherwise, the IV is based on sector's LBA
 	if (sector_offset != 0)
 	{
-		std::memcpy(iv.data(), buffer.data(), iv.size());
-		cur_offset = iv.size();
+		std::memcpy(iv.data(), buffer, 16);
+		cur_offset = 16;
 	}
 	else
 	{
@@ -156,7 +158,7 @@ static bool decrypt_data(aes_context& aes, u64 offset, const std::span<u8> buffe
 		cur_offset = 0;
 	}
 
-	u64 cur_size = sector_offset + size <= ISO_SECTOR_SIZE ? size : ISO_SECTOR_SIZE - sector_offset;
+	cur_size = sector_offset + size <= ISO_SECTOR_SIZE ? size : ISO_SECTOR_SIZE - sector_offset;
 	cur_size -= cur_offset;
 
 	// Partial (or even full) first sector
@@ -311,7 +313,7 @@ iso_type_status iso_file_decryption::retrieve_key(iso_archive& archive, std::str
 
 	for (auto path_it = entries.begin(); path_it != entries.end(); path_it++)
 	{
-		const fs::dir_entry dir_entry = std::move(*path_it);
+		const auto dir_entry = std::move(*path_it);
 
 		if (dir_entry.name == "." || dir_entry.name == ".." || dir_entry.is_directory)
 		{
@@ -327,7 +329,7 @@ iso_type_status iso_file_decryption::retrieve_key(iso_archive& archive, std::str
 		}
 
 		// If the decryption fails
-		if (!decrypt_data(aes_ctx, iso_file.file_offset(0), enc_sec, dec_sec, ISO_SECTOR_SIZE))
+		if (!decrypt_data(aes_ctx, iso_file.file_offset(0), enc_sec.data(), dec_sec.data(), ISO_SECTOR_SIZE))
 		{
 			continue;
 		}
@@ -420,7 +422,7 @@ bool iso_file_decryption::init(const std::string& path, iso_archive* archive)
 	// Following checks and assigned values are based on PS3 ISO specification.
 	// E.g. all even regions (0, 2, 4 etc.) are always unencrypted while the odd ones are encrypted
 
-	const u32 region_count = read_from_ptr<be_t<u32>>(sec0_sec1);
+	const u32 region_count = char_arr_BE_to_uint(sec0_sec1.data());
 
 	// Ensure the region count is a proper value
 	if (region_count < 1 || region_count > 127) // It's non-PS3ISO
@@ -431,13 +433,13 @@ bool iso_file_decryption::init(const std::string& path, iso_archive* archive)
 
 	m_region_info.resize(region_count * 2 - 1);
 
-	for (usz i = 0; i < m_region_info.size(); i++)
+	for (size_t i = 0; i < m_region_info.size(); i++)
 	{
 		// Store the region information in address format
-		const usz modulo_2 = i % 2;
-		m_region_info[i].encrypted = (modulo_2 == 1);
+		m_region_info[i].encrypted = (i % 2 == 1);
 		m_region_info[i].region_first_addr = (i == 0 ? 0ULL : m_region_info[i - 1].region_last_addr + 1ULL);
-		m_region_info[i].region_last_addr = (static_cast<u64>(read_from_ptr<be_t<u32>>(sec0_sec1, 12 + (i * 4))) - modulo_2) * ISO_SECTOR_SIZE + ISO_SECTOR_SIZE - 1ULL;
+		m_region_info[i].region_last_addr = (static_cast<u64>(char_arr_BE_to_uint(sec0_sec1.data() + 12 + (i * 4)))
+			- (i % 2 == 1 ? 1ULL : 0ULL)) * ISO_SECTOR_SIZE + ISO_SECTOR_SIZE - 1ULL;
 	}
 
 	//
@@ -486,19 +488,21 @@ bool iso_file_decryption::init(const std::string& path, iso_archive* archive)
 	if (m_enc_type == iso_encryption_type::NONE)
 	{
 		// The 3k3y watermarks located at offset 0xF70: (D|E)ncrypted 3K BLD
-		static const u8 k3k3y_enc_watermark[16] = {0x45, 0x6E, 0x63, 0x72, 0x79, 0x70, 0x74, 0x65, 0x64, 0x20, 0x33, 0x4B, 0x20, 0x42, 0x4C, 0x44};
-		static const u8 k3k3y_dec_watermark[16] = {0x44, 0x6E, 0x63, 0x72, 0x79, 0x70, 0x74, 0x65, 0x64, 0x20, 0x33, 0x4B, 0x20, 0x42, 0x4C, 0x44};
+		static const unsigned char k3k3y_enc_watermark[16] =
+			{0x45, 0x6E, 0x63, 0x72, 0x79, 0x70, 0x74, 0x65, 0x64, 0x20, 0x33, 0x4B, 0x20, 0x42, 0x4C, 0x44};
+		static const unsigned char k3k3y_dec_watermark[16] =
+			{0x44, 0x6E, 0x63, 0x72, 0x79, 0x70, 0x74, 0x65, 0x64, 0x20, 0x33, 0x4B, 0x20, 0x42, 0x4C, 0x44};
 
 		if (std::memcmp(&k3k3y_enc_watermark[0], &sec0_sec1[0xF70], sizeof(k3k3y_enc_watermark)) == 0)
 		{
 			// Grab D1 from the 3k3y sector
-			u8 key[16];
+			unsigned char key[16];
 
 			std::memcpy(key, &sec0_sec1[0xF80], 0x10);
 
 			// Convert D1 to KEY and generate the "m_aes_dec" context
-			u8 key_d1[] = {0x38, 11, 0xcf, 11, 0x53, 0x45, 0x5b, 60, 120, 0x17, 0xab, 0x4f, 0xa3, 0xba, 0x90, 0xed};
-			u8 iv_d1[] = {0x69, 0x47, 0x47, 0x72, 0xaf, 0x6f, 0xda, 0xb3, 0x42, 0x74, 0x3a, 0xef, 170, 0x18, 0x62, 0x87};
+			unsigned char key_d1[] = {0x38, 11, 0xcf, 11, 0x53, 0x45, 0x5b, 60, 120, 0x17, 0xab, 0x4f, 0xa3, 0xba, 0x90, 0xed};
+			unsigned char iv_d1[] = {0x69, 0x47, 0x47, 0x72, 0xaf, 0x6f, 0xda, 0xb3, 0x42, 0x74, 0x3a, 0xef, 170, 0x18, 0x62, 0x87};
 
 			aes_context aes_d1;
 
@@ -543,7 +547,7 @@ bool iso_file_decryption::init(const std::string& path, iso_archive* archive)
 	return true;
 }
 
-bool iso_file_decryption::decrypt(u64 offset, const std::span<u8> buffer, const std::string& name)
+bool iso_file_decryption::decrypt(u64 offset, void* buffer, u64 size, const std::string& name)
 {
 	// If it's a non-encrypted type, nothing more to do
 	if (m_enc_type == iso_encryption_type::NONE)
@@ -554,22 +558,13 @@ bool iso_file_decryption::decrypt(u64 offset, const std::span<u8> buffer, const
 	// If it's a 3k3y ISO and data at offset 0xF70 is being requested, we should null it out
 	if (m_enc_type == iso_encryption_type::DEC_3K3Y || m_enc_type == iso_encryption_type::ENC_3K3Y)
 	{
-		constexpr u64 range_start = 0xF70ULL;
-		constexpr u64 range_end = 0x1070ULL;
-		constexpr u64 range = range_end - range_start;
-
-		const u64 buffer_size = buffer.size();
-
-		ensure(offset <= (u64{umax} - buffer_size)); // Check for overflow
-		const u64 buffer_end = offset + buffer_size;
-
-		if (buffer_end > range_start && offset < range_end)
+		if (offset + size >= 0xF70ULL && offset <= 0x1070ULL)
 		{
 			// Zero out the 0xF70 - 0x1070 overlap
-			const u64 buf_overlap_start = offset < range_start ? range_start - offset : 0;
-			const u64 buf_overlap_end = buffer_end < range_end ? buffer_size : range;
+			unsigned char* buf = reinterpret_cast<unsigned char*>(buffer);
+			unsigned char* buf_overlap_start = offset < 0xF70ULL ? buf + 0xF70ULL - offset : buf;
 
-			std::memset(&buffer[buf_overlap_start], 0x00, buf_overlap_end - buf_overlap_start);
+			memset(buf_overlap_start, 0x00, offset + size < 0x1070ULL ? size - (buf_overlap_start - buf) : 0x100ULL - (buf_overlap_start - buf));
 		}
 
 		// If it's a decrypted ISO then return, otherwise go on to the decryption logic
@@ -591,7 +586,7 @@ bool iso_file_decryption::decrypt(u64 offset, const std::span<u8> buffer, const
 			}
 
 			// Decrypt the region before sending it back
-			decrypt_data(m_aes_dec, offset, buffer, buffer, buffer.size());
+			decrypt_data(m_aes_dec, offset, reinterpret_cast<unsigned char*>(buffer), reinterpret_cast<unsigned char*>(buffer), size);
 
 			return true;
 		}
@@ -600,9 +595,9 @@ bool iso_file_decryption::decrypt(u64 offset, const std::span<u8> buffer, const
 	iso_log.error("decrypt: %s: LBA request wasn't in the 'm_region_info' for an encrypted ISO? - RP: 0x%lx, RC: 0x%lx, LR: (0x%016lx - 0x%016lx)",
 		name,
 		offset,
-		static_cast<u32>(m_region_info.size()),
-		static_cast<u32>(!m_region_info.empty() ? m_region_info.back().region_first_addr : 0),
-		static_cast<u32>(!m_region_info.empty() ? m_region_info.back().region_last_addr : 0));
+		static_cast<unsigned long int>(m_region_info.size()),
+		static_cast<unsigned long int>(!m_region_info.empty() ? m_region_info.back().region_first_addr : 0),
+		static_cast<unsigned long int>(!m_region_info.empty() ? m_region_info.back().region_last_addr : 0));
 
 	return true;
 }
@@ -645,14 +640,13 @@ u64 iso_file_encrypted::read_at(u64 offset, void* buffer, u64 size)
 	const u64 total_size = this->size();
 	const u64 archive_first_offset = file_offset(offset);
 	const u64 archive_last_offset = archive_first_offset + max_size - 1;
+	iso_sector first_sec, last_sec;
 	void* aligned_buf = get_aligned_buf(); // thread-safe buffer
 
-	iso_sector first_sec {};
 	first_sec.lba_address = (archive_first_offset / ISO_SECTOR_SIZE) * ISO_SECTOR_SIZE;
 	first_sec.offset = archive_first_offset % ISO_SECTOR_SIZE;
 	first_sec.size = first_sec.offset + max_size <= ISO_SECTOR_SIZE ? max_size : ISO_SECTOR_SIZE - first_sec.offset;
 
-	iso_sector last_sec {};
 	last_sec.lba_address = last_sec.address_aligned = (archive_last_offset / ISO_SECTOR_SIZE) * ISO_SECTOR_SIZE;
 	// last_sec.offset = last_sec.offset_aligned = 0; // Always 0 so no need to set and use those attributes
 	last_sec.size = (archive_last_offset % ISO_SECTOR_SIZE) + 1;
@@ -683,7 +677,7 @@ u64 iso_file_encrypted::read_at(u64 offset, void* buffer, u64 size)
 
 	u64 total_read = m_file.read_at(first_sec.address_aligned, &reinterpret_cast<u8*>(aligned_buf)[first_sec.offset_aligned], first_sec.size_aligned);
 
-	m_dec->decrypt(first_sec.address_aligned, {&reinterpret_cast<u8*>(aligned_buf)[first_sec.offset_aligned], first_sec.size_aligned}, m_meta.name);
+	m_dec->decrypt(first_sec.address_aligned, &reinterpret_cast<u8*>(aligned_buf)[first_sec.offset_aligned], first_sec.size_aligned, m_meta.name);
 	std::memcpy(buffer, &reinterpret_cast<u8*>(aligned_buf)[first_sec.offset], first_sec.size);
 
 	const u64 sector_count = (last_sec.lba_address - first_sec.lba_address) / ISO_SECTOR_SIZE + 1;
@@ -720,7 +714,7 @@ u64 iso_file_encrypted::read_at(u64 offset, void* buffer, u64 size)
 
 			total_read += m_file.read_at(first_sec.lba_address + ISO_SECTOR_SIZE, &reinterpret_cast<u8*>(buffer)[first_sec.size], inner_sector_size);
 
-			m_dec->decrypt(first_sec.lba_address + ISO_SECTOR_SIZE, {&reinterpret_cast<u8*>(buffer)[first_sec.size], inner_sector_size}, m_meta.name);
+			m_dec->decrypt(first_sec.lba_address + ISO_SECTOR_SIZE, &reinterpret_cast<u8*>(buffer)[first_sec.size], inner_sector_size, m_meta.name);
 		}
 		else
 		{
@@ -730,7 +724,7 @@ u64 iso_file_encrypted::read_at(u64 offset, void* buffer, u64 size)
 			{
 				total_read += m_file.read_at(first_sec.lba_address + ISO_SECTOR_SIZE + inner_sector_offset, aligned_buf, ISO_SECTOR_SIZE);
 
-				m_dec->decrypt(first_sec.lba_address + ISO_SECTOR_SIZE + inner_sector_offset, {reinterpret_cast<u8*>(aligned_buf), ISO_SECTOR_SIZE}, m_meta.name);
+				m_dec->decrypt(first_sec.lba_address + ISO_SECTOR_SIZE + inner_sector_offset, aligned_buf, ISO_SECTOR_SIZE, m_meta.name);
 				std::memcpy(&reinterpret_cast<u8*>(buffer)[first_sec.size + inner_sector_offset], aligned_buf, ISO_SECTOR_SIZE);
 			}
 		}
@@ -753,7 +747,7 @@ u64 iso_file_encrypted::read_at(u64 offset, void* buffer, u64 size)
 
 	total_read += m_file.read_at(last_sec.address_aligned, aligned_buf, last_sec.size_aligned);
 
-	m_dec->decrypt(last_sec.address_aligned, {reinterpret_cast<u8*>(aligned_buf), last_sec.size_aligned}, m_meta.name);
+	m_dec->decrypt(last_sec.address_aligned, aligned_buf, last_sec.size_aligned, m_meta.name);
 	std::memcpy(&reinterpret_cast<u8*>(buffer)[max_size - last_sec.size], aligned_buf, last_sec.size);
 
 	//
@@ -859,13 +853,9 @@ static std::optional<iso_fs_metadata> iso_read_directory_entry(fs::file& entry,
 
 	std::string file_name;
 
-	if (!entry.read(file_name, header.file_name_length))
-	{
-		iso_log.error("iso_archive: Failed to read file name");
-		return std::nullopt;
-	}
+	entry.read(file_name, header.file_name_length);
 
-	if (file_name.size() == 1 && file_name[0] == '\0')
+	if (header.file_name_length == 1 && file_name[0] == 0)
 	{
 		file_name = ".";
 	}
@@ -879,7 +869,7 @@ static std::optional<iso_fs_metadata> iso_read_directory_entry(fs::file& entry,
 		const be_t<u16>* raw = utils::bless<const be_t<u16>>(file_name.data());
 		std::u16string utf16;
 
-		utf16.resize(file_name.size() / 2);
+		utf16.resize(header.file_name_length / 2);
 
 		for (usz i = 0; i < utf16.size(); i++)
 		{
@@ -894,7 +884,7 @@ static std::optional<iso_fs_metadata> iso_read_directory_entry(fs::file& entry,
 		file_name.erase(file_name.end() - 2, file_name.end());
 	}
 
-	if (file_name.size() > 1 && file_name.ends_with("."))
+	if (header.file_name_length > 1 && file_name.ends_with("."))
 	{
 		file_name.pop_back();
 	}
@@ -919,25 +909,17 @@ static std::optional<iso_fs_metadata> iso_read_directory_entry(fs::file& entry,
 	};
 }
 
-static bool iso_form_hierarchy(fs::file& file, iso_fs_node& node, bool use_ucs2_decoding = false, const std::string& parent_path = "")
+static void iso_form_hierarchy(fs::file& file, iso_fs_node& node, bool use_ucs2_decoding = false, const std::string& parent_path = "")
 {
 	if (!node.metadata.is_directory)
 	{
-		return !parent_path.empty();
-	}
-
-	const std::string node_path = parent_path + "/" + node.metadata.name;
-
-	if (!parent_path.empty() && !Emu.IsPathInsideDir(node_path, parent_path, false))
-	{
-		iso_log.error("iso_archive::iso_form_hierarchy: node path outside of parent (parent_path='%s', node_path='%s')", parent_path, node_path);
-		return false;
+		return;
 	}
 
 	std::vector<usz> multi_extent_node_indices;
 
 	// Assuming the directory spans a single extent
-	const auto& directory_extent = ::at32(node.metadata.extents, 0);
+	const auto& directory_extent = node.metadata.extents[0];
 	const u64 end_pos = (directory_extent.start * ISO_SECTOR_SIZE) + directory_extent.size;
 
 	file.seek(directory_extent.start * ISO_SECTOR_SIZE);
@@ -964,7 +946,7 @@ static bool iso_form_hierarchy(fs::file& file, iso_fs_node& node, bool use_ucs2_
 			if (selected_node->metadata.name == entry->name)
 			{
 				// Merge into selected_node
-				selected_node->metadata.extents.push_back(::at32(entry->extents, 0));
+				selected_node->metadata.extents.push_back(entry->extents[0]);
 
 				extent_added = true;
 				break;
@@ -991,14 +973,9 @@ static bool iso_form_hierarchy(fs::file& file, iso_fs_node& node, bool use_ucs2_
 	{
 		if (child_node->metadata.name != "." && child_node->metadata.name != "..")
 		{
-			if (!iso_form_hierarchy(file, *child_node, use_ucs2_decoding, node_path))
-			{
-				return false;
-			}
+			iso_form_hierarchy(file, *child_node, use_ucs2_decoding, parent_path + "/" + node.metadata.name);
 		}
 	}
-
-	return true;
 }
 
 u64 iso_fs_metadata::size() const
@@ -1022,8 +999,8 @@ iso_archive::iso_archive(const std::string& path)
 
 	if (!is_iso_file(m_path))
 	{
+		// Not ISO... TODO: throw something?
 		iso_log.error("iso_archive: Failed to recognize ISO file: '%s'", path);
-		invalidate();
 		return;
 	}
 
@@ -1059,36 +1036,23 @@ iso_archive::iso_archive(const std::string& path)
 
 		iso_file.seek(descriptor_start + ISO_SECTOR_SIZE);
 	}
-	while (descriptor_type != 255 && iso_file.pos() < iso_file.size());
+	while (descriptor_type != 255);
 
-	if (descriptor_type != 255)
-	{
-		iso_log.error("iso_archive: Corrupt ISO file '%s': Volume Descriptor Set Terminator not found", path);
-		invalidate();
-		return;
-	}
-
-	if (!iso_form_hierarchy(iso_file, m_root, use_ucs2_decoding))
-	{
-		iso_log.error("iso_archive: Corrupt ISO file '%s': Failed to form hierarchy", path);
-		invalidate();
-		return;
-	}
+	iso_form_hierarchy(iso_file, m_root, use_ucs2_decoding);
 
 	// Only when the archive object is fully set, we can finally initialize the decryption object needing the archive object
 	m_dec = std::make_shared<iso_file_decryption>();
 
 	if (!m_dec->init(m_path, this))
 	{
-		iso_log.error("iso_archive: Corrupt ISO file '%s': Decryption failed", path);
-		invalidate();
+		// TODO: throw something?
 		return;
 	}
 }
 
 iso_fs_node* iso_archive::retrieve(const std::string& passed_path)
 {
-	if (passed_path.empty() || !is_valid())
+	if (passed_path.empty())
 	{
 		return nullptr;
 	}
@@ -1163,12 +1127,6 @@ iso_fs_node* iso_archive::retrieve(const std::string& passed_path)
 	return search_stack.top();
 }
 
-void iso_archive::invalidate()
-{
-	m_root = {};
-	m_dec.reset();
-}
-
 bool iso_archive::is_valid() const
 {
 	return !m_root.metadata.name.empty();
@@ -1193,11 +1151,6 @@ bool iso_archive::is_file(const std::string& path)
 
 std::unique_ptr<fs::file_base> iso_archive::get_iso_file(const std::string& path, bs_t<fs::open_mode> mode, const iso_fs_node& node)
 {
-	if (!is_valid())
-	{
-		return nullptr;
-	}
-
 	if (m_dec->get_enc_type() == iso_encryption_type::NONE)
 	{
 		return std::make_unique<iso_file>(path, mode, node);
@@ -1256,7 +1209,7 @@ iso_file::iso_file(const std::string& path, bs_t<fs::open_mode> mode, const iso_
 		return;
 	}
 
-	m_file.seek(::at32(m_meta.extents, 0).start * ISO_SECTOR_SIZE);
+	m_file.seek(m_meta.extents[0].start * ISO_SECTOR_SIZE);
 
 	m_raw_device = fs::is_optical_raw_device(path);
 }

rpcs3/Loader/ISO.h

@@ -6,8 +6,6 @@
 #include "util/types.hpp"
 #include "Crypto/aes.h"
 
-#include <span>
-
 bool is_iso_file(const std::string& path, u64* size = nullptr, bool* is_raw_device = nullptr);
 
 void load_iso(const std::string& path);
@@ -78,7 +76,7 @@ public:
 	iso_encryption_type get_enc_type() const { return m_enc_type; }
 
 	bool init(const std::string& path, iso_archive* archive = nullptr);
-	bool decrypt(u64 offset, const std::span<u8> buffer, const std::string& name);
+	bool decrypt(u64 offset, void* buffer, u64 size, const std::string& name);
 };
 
 struct iso_extent_info
@@ -166,8 +164,6 @@ public:
 class iso_archive
 {
 private:
-	void invalidate();
-
 	std::string m_path;
 	iso_fs_node m_root {};
 	std::shared_ptr<iso_file_decryption> m_dec;
@@ -179,7 +175,11 @@ public:
 	const iso_fs_node& root() const { return m_root; }
 
 	iso_fs_node* retrieve(const std::string& path);
+	// Kept from upstream d6d5c6082 while the rest of that ISO series is reverted: System.cpp
+	// calls it, and it is the one piece of the refactor that is not implicated in the region
+	// read failing.
 	bool is_valid() const;
+
 	bool exists(const std::string& path);
 	bool is_file(const std::string& path);