Keep saved Steam screenshots separate by headset and game

Co-authored-by: SAPHID <saphid@users.noreply.github.com>

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+- Save Steam screenshots under a separate folder for each headset, Steam
+  account and game. Equal screenshot filenames no longer overwrite one another;
+  existing files in the old flat folder are left in place.
+  [Copying guide](docs/recording.md).
+
 - Compare two explicit Windows Wireless Adapter doctor reads in local memory.
   Highlight exact PnP instances and known state changes after a dropout;
   incomplete inventories stay unknown. A Forget button clears the comparison.

docs/recording.md

@@ -107,6 +107,15 @@ about once a minute and copies what's new with the same code as **Save** and
 **Export**: screenshots to `~/Pictures/SteamFrame`, clips to
 `~/Videos/SteamFrame`.
 
+Each newly saved screenshot lives under
+`~/Pictures/SteamFrame/headset-<id>/<Steam account>/<game ID>/<filename>`.
+The headset folder uses a short hash of Frame Control's device identity.
+Steam can use the same screenshot filename for two games or accounts; these
+folders keep both originals. Existing screenshots in the old flat
+`~/Pictures/SteamFrame/<filename>` layout stay where they are. They cannot be
+matched reliably to one headset and game, so **Copy everything now** can create
+a new, correctly identified copy. It never overwrites the old file.
+
 - **Only what's new.** It copies what you take after turning it on, so the
   shots and clips already on the headset don't all arrive at once. **Copy
   everything now** copies everything that isn't on this computer yet, once.

tests/test_shot_copy.py

@@ -0,0 +1,92 @@
+"""Screenshot copies retain the headset, Steam account and game identity."""
+import sandbox  # noqa: F401
+import shutil
+import subprocess
+import sys
+import tempfile
+import unittest
+from pathlib import Path
+from unittest import mock
+
+ROOT = Path(__file__).resolve().parents[1]
+sys.path.insert(0, str(ROOT / "ui"))
+import server  # noqa: E402
+
+
+NAME = "20261025020000_1.jpg"
+FIRST = f"1/620/{NAME}"
+SECOND = f"1/730/{NAME}"
+
+
+class ShotCopyTest(unittest.TestCase):
+    def setUp(self):
+        self.folder = Path(tempfile.mkdtemp(prefix="frame-shot-copy-"))
+        self.addCleanup(shutil.rmtree, self.folder, ignore_errors=True)
+        patch = mock.patch.object(server, "SHOTS_DIR", self.folder)
+        patch.start()
+        self.addCleanup(patch.stop)
+        patch = mock.patch.object(server, "LINK", None)
+        patch.start()
+        self.addCleanup(patch.stop)
+        self.payloads = {
+            server.shot_path(FIRST): b"first game",
+            server.shot_path(SECOND): b"second game",
+        }
+
+    def scp(self, argv, **_kwargs):
+        target = Path(argv[-1])
+        for source in argv[1:-1]:
+            if not source.startswith("frame-a:") and not source.startswith("frame-b:"):
+                continue
+            remote = source.split(":", 1)[1]
+            (target / Path(remote).name).write_bytes(self.payloads[remote])
+        return subprocess.CompletedProcess(argv, 0, "", "")
+
+    def test_equal_names_survive_across_games_and_headsets(self):
+        # A pre-upgrade flat file has no provable source identity. Leave it alone.
+        legacy = self.folder / NAME
+        legacy.write_bytes(b"older screenshot")
+        with mock.patch.object(server, "FRAME", "frame-a"), \
+             mock.patch.object(server, "ensure_master"), \
+             mock.patch.object(server.frame_host, "run_ssh", side_effect=self.scp) as copy:
+            self.assertEqual(server.save_shots({"ids": [FIRST, SECOND]})["saved"], 2)
+            self.assertEqual(copy.call_count, 2)  # scp cannot flatten equal names together
+            first = server.shot_local_path(FIRST)
+            second = server.shot_local_path(SECOND)
+            self.assertEqual(first.read_bytes(), b"first game")
+            self.assertEqual(second.read_bytes(), b"second game")
+            self.assertNotEqual(first, second)
+            self.assertEqual(legacy.read_bytes(), b"older screenshot")
+            listing = "\n".join([
+                f"1/760/remote/620/screenshots/{NAME}\t10\t1792893600",
+                f"1/760/remote/730/screenshots/{NAME}\t11\t1792893601",
+            ])
+            with mock.patch.object(server, "ssh", return_value=listing):
+                self.assertTrue(all(s["saved"] for s in server.list_shots()["shots"]))
+            with mock.patch.object(server.frame_host, "reveal_path") as reveal:
+                server.open_thing({"what": "shot", "id": SECOND})
+                reveal.assert_called_once_with(second)
+
+        self.payloads[server.shot_path(FIRST)] = b"another headset"
+        with mock.patch.object(server, "FRAME", "frame-b"), \
+             mock.patch.object(server, "ensure_master"), \
+             mock.patch.object(server.frame_host, "run_ssh", side_effect=self.scp):
+            self.assertEqual(server.save_shots({"ids": [FIRST]})["saved"], 1)
+            other = server.shot_local_path(FIRST)
+            self.assertNotEqual(other, first)
+            self.assertEqual(other.read_bytes(), b"another headset")
+            self.assertEqual(first.read_bytes(), b"first game")
+
+    def test_missing_staged_file_does_not_claim_saved(self):
+        with mock.patch.object(server, "FRAME", "frame-a"), \
+             mock.patch.object(server, "ensure_master"), \
+             mock.patch.object(server.frame_host, "run_ssh",
+                               return_value=subprocess.CompletedProcess([], 0, "", "")):
+            with self.assertRaisesRegex(server.Failure, "did not contain every"):
+                server.save_shots({"ids": [FIRST]})
+            self.assertFalse(server.shot_local_path(FIRST).exists())
+            self.assertFalse(list(self.folder.glob(".incoming-*")))
+
+
+if __name__ == "__main__":
+    unittest.main()

ui/server.py

@@ -635,6 +635,7 @@ def _headset_view():
 SHOT_ROOT = ".local/share/Steam/userdata"
 SHOT_ID = re.compile(r"(\d{1,12})/(\d{1,20})/(\d{14}_\d{1,4}\.(?:jpg|png))")
 SHOTS_DIR = Path.home() / "Pictures" / "SteamFrame"
+_shots_copy_lock = threading.Lock()
 LIST_SHOTS = f"""cd ~/{SHOT_ROOT} 2>/dev/null || exit 0
 find . -mindepth 6 -maxdepth 6 -path './*/760/remote/*/screenshots/*' -type f \\
   \\( -name '*.jpg' -o -name '*.png' \\) -printf '%P\\t%s\\t%T@\\n'"""
@@ -647,8 +648,19 @@ def shot_path(shot_id, thumb=False):
     return f"{SHOT_ROOT}/{m[1]}/760/remote/{m[2]}/screenshots/{'thumbnails/' if thumb else ''}{m[3]}"
 
 
+def shot_local_path(shot_id, device_id=None):
+    """Keep equal Steam filenames from different headsets/accounts/games separate."""
+    m = SHOT_ID.fullmatch(shot_id) if isinstance(shot_id, str) else None
+    if not m:
+        raise Failure("bad screenshot id", 400)
+    identity = device_id if device_id is not None else agent_target()["id"]
+    headset = hashlib.sha256(identity.encode("utf-8")).hexdigest()[:12]
+    return SHOTS_DIR / f"headset-{headset}" / m[1] / m[2] / m[3]
+
+
 def list_shots():
     shots = []
+    device_id = agent_target()["id"]
     for line in ssh(LIST_SHOTS, timeout=20).splitlines():
         rel, _, rest = line.partition("\t")
         parts = rel.split("/")  # account/760/remote/appid/screenshots/file
@@ -660,7 +672,7 @@ def list_shots():
             when = float(mtime)
         except ValueError:
             continue
-        local = SHOTS_DIR / parts[-1]
+        local = shot_local_path(shot_id, device_id)
         shots.append({"id": shot_id, "appid": parts[3], "file": parts[-1], "size": int(size), "time": when,
                       "saved": local.exists() and local.stat().st_size == int(size)})
     shots.sort(key=lambda s: s["time"], reverse=True)
@@ -682,31 +694,51 @@ def shot_image(query):
 
 
 def save_shots(body):
-    """Copy screenshots to ~/Pictures/SteamFrame, skipping ones already there."""
+    """Copy screenshots to per-headset/account/game folders without name collisions."""
     ids = body.get("ids")
     if not isinstance(ids, list) or not 0 < len(ids) <= 1000:
         raise Failure("ids must be a list of 1-1000 screenshot ids", 400)
-    paths = [shot_path(i) for i in ids]
-    todo = [p for p in paths if not (SHOTS_DIR / p.rsplit("/", 1)[-1]).exists()]
-    if todo:
-        SHOTS_DIR.mkdir(parents=True, exist_ok=True)
-        ensure_master()
-        # Copy into a hidden folder and move complete files in, so a cut-off
-        # copy never looks saved. -p keeps the time the shot was taken.
-        incoming = Path(tempfile.mkdtemp(prefix=".incoming-", dir=SHOTS_DIR))
-        try:
+    device_id = agent_target()["id"]
+    items = [(shot_path(i), shot_local_path(i, device_id)) for i in ids]
+    n = 0
+    with _shots_copy_lock:
+        todo = [(remote, local) for remote, local in items if not local.exists()]
+        if todo:
+            SHOTS_DIR.mkdir(parents=True, exist_ok=True)
+            ensure_master()
+        # scp flattens source paths into one destination directory. Split equal
+        # basenames into separate transfers before assigning each file its ID path.
+        while todo:
+            names, group, rest = set(), [], []
+            for item in todo:
+                if item[1].name in names:
+                    rest.append(item)
+                else:
+                    names.add(item[1].name)
+                    group.append(item)
+            todo = rest
+            # Stage under the destination filesystem, then move complete files in.
+            # -p keeps Steam's screenshot timestamp.
+            incoming = Path(tempfile.mkdtemp(prefix=".incoming-", dir=SHOTS_DIR))
             try:
-                r = frame_host.run_ssh(["scp", "-p", *SSH[1:], *(f"{FRAME}:{p}" for p in todo), str(incoming)],
-                                       capture_output=True, stdin=subprocess.DEVNULL, text=True, timeout=300)
-            except subprocess.TimeoutExpired:
-                raise Failure("Copying screenshots timed out")
-            if r.returncode != 0:
-                raise Failure(strip_ansi(r.stderr).strip() or f"scp exited {r.returncode}")
-            for f in incoming.iterdir():
-                os.replace(f, SHOTS_DIR / f.name)
-        finally:
-            shutil.rmtree(incoming, ignore_errors=True)
-    n, skipped = len(todo), len(ids) - len(todo)
+                try:
+                    r = frame_host.run_ssh(["scp", "-p", *SSH[1:],
+                                            *(f"{FRAME}:{remote}" for remote, _ in group), str(incoming)],
+                                           capture_output=True, stdin=subprocess.DEVNULL, text=True, timeout=300)
+                except subprocess.TimeoutExpired:
+                    raise Failure("Copying screenshots timed out")
+                if r.returncode != 0:
+                    raise Failure(strip_ansi(r.stderr).strip() or f"scp exited {r.returncode}")
+                if any(not (incoming / local.name).is_file() for _, local in group):
+                    raise Failure("Screenshot copy did not contain every requested file")
+                for _, local in group:
+                    local.parent.mkdir(parents=True, exist_ok=True)
+                    if not local.exists():
+                        os.replace(incoming / local.name, local)
+                        n += 1
+            finally:
+                shutil.rmtree(incoming, ignore_errors=True)
+    skipped = len(ids) - n
     msg = f"Saved {n} screenshot{'s' * (n != 1)} to ~/Pictures/SteamFrame"
     return {"message": msg + (f" ({skipped} already there)" if skipped else ""), "saved": n}
 
@@ -1936,7 +1968,7 @@ def open_thing(body):
             frame_host.open_path(frame_clips.VIDEOS_DIR)
             return {"message": f"Opened {frame_clips.VIDEOS_DIR} in {frame_host.FILE_MANAGER}"}
         if what == "shot":
-            saved = SHOTS_DIR / shot_path(body.get("id")).rsplit("/", 1)[-1]
+            saved = shot_local_path(body.get("id"))
             if not saved.exists():
                 raise Failure("That screenshot isn't saved on this computer yet", 404)
             frame_host.reveal_path(saved)