feat(launcher): added profile management and save importing #1179
Closed
timstr97 wants to merge 11 commits from
builder/profiles into main
pull from: builder/profiles
merge into: MagicBeanDev:main
MagicBeanDev:main
MagicBeanDev:github/pr/2893
MagicBeanDev:github/pr/2890
MagicBeanDev:github/pr/2887
MagicBeanDev:github/pr/2878
MagicBeanDev:github/pr/2875
MagicBeanDev:github/pr/2874
MagicBeanDev:github/pr/2873
MagicBeanDev:github/pr/2871
MagicBeanDev:github/pr/2756
MagicBeanDev:github/pr/2897
MagicBeanDev:github/pr/2896
MagicBeanDev:github/pr/2718
MagicBeanDev:github/pr/2842
MagicBeanDev:github/pr/2691
MagicBeanDev:github/pr/2895
MagicBeanDev:github/pr/2743
MagicBeanDev:github/pr/2765
MagicBeanDev:github/pr/2708
MagicBeanDev:github/pr/2732
MagicBeanDev:github/pr/2755
MagicBeanDev:github/pr/2817
MagicBeanDev:github/pr/2876
MagicBeanDev:release-please--branches--main
MagicBeanDev:release-please--branches--main--release-notes
MagicBeanDev:github/pr/2778
MagicBeanDev:github/pr/2888
MagicBeanDev:github/pr/2866
MagicBeanDev:github/pr/2877
MagicBeanDev:github/pr/2800
MagicBeanDev:github/pr/2829
MagicBeanDev:github/pr/2850
MagicBeanDev:github/pr/2889
MagicBeanDev:github/pr/2882
MagicBeanDev:github/pr/2860
MagicBeanDev:github/pr/2849
MagicBeanDev:github/pr/2886
MagicBeanDev:github/pr/2794
MagicBeanDev:github/pr/2840
MagicBeanDev:github/pr/2830
MagicBeanDev:github/pr/2834
MagicBeanDev:github/pr/2719
MagicBeanDev:github/pr/2742
MagicBeanDev:github/pr/2741
MagicBeanDev:github/pr/2820
MagicBeanDev:github/pr/2771
MagicBeanDev:github/pr/2859
MagicBeanDev:github/pr/2858
MagicBeanDev:github/pr/2823
MagicBeanDev:github/pr/2757
MagicBeanDev:github/pr/2769
MagicBeanDev:github/pr/2844
MagicBeanDev:github/pr/2843
MagicBeanDev:github/pr/2673
MagicBeanDev:github/pr/2855
MagicBeanDev:github/pr/2739
MagicBeanDev:github/pr/2807
MagicBeanDev:github/pr/2851
MagicBeanDev:github/pr/2789
MagicBeanDev:github/pr/2814
MagicBeanDev:github/pr/2818
MagicBeanDev:github/pr/2774
MagicBeanDev:github/pr/2772
MagicBeanDev:github/pr/2750
MagicBeanDev:github/pr/2786
MagicBeanDev:github/pr/2785
MagicBeanDev:github/pr/2782
MagicBeanDev:github/pr/2779
MagicBeanDev:github/pr/2775
MagicBeanDev:github/pr/2706
MagicBeanDev:github/pr/2766
MagicBeanDev:github/pr/2747
MagicBeanDev:github/pr/2759
MagicBeanDev:github/pr/2745
MagicBeanDev:github/pr/2740
MagicBeanDev:github/pr/2733
MagicBeanDev:github/pr/2725
MagicBeanDev:github/pr/2727
MagicBeanDev:github/pr/2728
MagicBeanDev:github/pr/2729
MagicBeanDev:github/pr/2721
MagicBeanDev:github/pr/2722
MagicBeanDev:github/pr/2715
MagicBeanDev:github/pr/2714
MagicBeanDev:github/pr/2707
MagicBeanDev:github/pr/2703
MagicBeanDev:github/pr/2705
MagicBeanDev:github/pr/2697
MagicBeanDev:github/pr/2679
MagicBeanDev:github/pr/2676
MagicBeanDev:github/pr/2669
MagicBeanDev:docs/what-hookshot-connects-to
MagicBeanDev:fix/hdr-encode-test
MagicBeanDev:fix/main-tests
MagicBeanDev:fix/play-cache-files
MagicBeanDev:perf/game-barrier-order
MagicBeanDev:feat/heap-lazy
MagicBeanDev:feat/texture-detail
MagicBeanDev:perf/astc-opaque-bc1
MagicBeanDev:fix/fs-dir-lock
MagicBeanDev:fix/framegen-native-kept
MagicBeanDev:fix/update-no-downgrade
MagicBeanDev:port/k10-gcn-f16-mirrors
MagicBeanDev:fix/framegen-mult-cap
MagicBeanDev:chore/release-as-2.1.1
MagicBeanDev:perf/pcache-load-switch
MagicBeanDev:fix/prepare-progress-log
MagicBeanDev:fix/update-closes-stale-game
MagicBeanDev:fix/hdr-scaled-sampled
MagicBeanDev:perf/first-start-preset
MagicBeanDev:perf/zero-upload
MagicBeanDev:fix/vcore-escape-2s
MagicBeanDev:test/fake-gh-atomic
MagicBeanDev:perf/batch-idle-flush
MagicBeanDev:test/finish-stub-a
MagicBeanDev:chore/record-2.1.0
MagicBeanDev:fix/main-red-tests
MagicBeanDev:fix/test-display-startup-driver-cache
MagicBeanDev:feat/mesh-shader-mode
MagicBeanDev:fix/menu-probes-at-startup
MagicBeanDev:fix/fps-counter-classic
MagicBeanDev:fix/test-log-batch-flake
MagicBeanDev:fix/narrow-screen-hud
MagicBeanDev:ci/prereleases
MagicBeanDev:release/v2.0.15
MagicBeanDev:hotfix/v2.0.15
MagicBeanDev:fix/present-ahead-replay
MagicBeanDev:release/v2.0.13
MagicBeanDev:fix/bracket-paths-2013
MagicBeanDev:fix/hdr-overlays-2013
MagicBeanDev:fix/fps-counter-scale-2013
MagicBeanDev:fix/stick-seen-at-rest
MagicBeanDev:hotfix/menu-fits-screen
MagicBeanDev:fix/f1-always-opens-menu
MagicBeanDev:fix/ini-keep-lines
MagicBeanDev:claude/cranky-pike-7cf247
MagicBeanDev:perf/texture-answer-flat
MagicBeanDev:fix/stamp-test-modules
MagicBeanDev:fix/appimage-fonts
MagicBeanDev:fix/display-awake
MagicBeanDev:release/v2.0.12
MagicBeanDev:fix/discord-windows
MagicBeanDev:rc/v2.0.12-tests
MagicBeanDev:fix/dlss-keep-feature
MagicBeanDev:rc/v2.0.12
MagicBeanDev:button-icons
No reviewers
Labels
Clear labels
accessibility
Barrier affecting people with disabilities
autorelease: pending
autorelease: tagged
bug
Something isn't working
dependencies
Pull requests that update a dependency file
documentation
Improvements or additions to documentation
duplicate
This issue or pull request already exists
enhancement
New feature or request
github_actions
Pull requests that update GitHub Actions code
good first issue
Good for newcomers
help wanted
Extra attention is needed
invalid
This doesn't seem right
question
Further information is requested
testing wanted
wontfix
This will not be worked on
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Hookshot!1179
Loading…
Reference in a new issue
No description provided.
Delete branch "builder/profiles"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What
The launcher (Windows and Linux) gets profiles, each with saves of its own, and can import a save from a console or an emulator.
HOOKSHOT_PROFILE=<id>; the game uses that profile as its user, so it mountssave/<id>in the user folder (host/nn_applet.c).account.sav,option.sav,slot_00…) or an emulator's whole save folder (save/0000000000000000/<user>/0400486024DEC000); when that holds several users, the launcher asks which one, with each save's slot count and last-saved time.save-backups/<id>-<time>. The copy goes to a staging folder first, so a failed import leaves the profile as it was.5a3c9e1f0b7d24680000000000000001): existing saves stay where they are. It is marked "(default)" in the list, even after a rename, and cannot be removed, since the game plays it when started without the launcher.Why
There was one user and one set of saves per PC. Several players on one PC overwrote each other's files, and saves from a console or an emulator had to be copied by hand into the right hashed folder.
How
host/nn_applet.c:pick_user()readsHOOKSHOT_PROFILE(32 hex digits, the save folder's name). It is an environment variable rather than an argument so that an older game, started by a newer launcher before it updates, ignores it instead of reading it as an image path. Anything that is not 32 hex digits plays the default profile.builder/linux/profiles.pyandbuilder/HookshotBuilder/Profiles.cshold the same logic and write the sameprofiles.txt(<id> <name>per line) in the user folder:user/in the game folder on Windows,~/.local/share/hookshoton Linux.hookshot_builder.py(launch page) andhookshot-launch(--playpasses the profile chosen last, frombuilder.ini).MainWindow.xaml(.cs). The chosen profile is kept inbuilder-settings.json, and the ready page's Play uses it too. The Manage menu is a popup in the app's dark style (newMenuRowstyle inApp.xaml).Testing
python -I -B tests/run_all.pyon Linux, after merging currentmain: 143 passed, 0 failed.tests/test_launcher_profiles.py: profiles.py; Linux only, added to the skip list like the other Linux-launcher tests.tests/test_account_profile.py: buildspick_userfromnn_applet.cand checks which save folder each value leads to, including invalid and path-like values.tests/launcher_profiles: C# tests forProfiles.cs, including reading aprofiles.txtas the Linux launcher writes it. Added tosource-regressions.ymlon Windows.tests/test_hookshot_launch.py:--playpasses the chosen profile.HookshotBuilder.csprojbuilds with 0 warnings (checked on Linux withEnableWindowsTargeting).HOOKSHOT_PROFILE=…0002logsaccount: profile …0002, mountssave/…0002and lists that profile's imported saves.🤖 Generated with Claude Code
Hi! This PR has merge conflicts with
mainnow (a lot has landed today). Could you mergemainin or rebase and resolve them? Happy to give it a proper review once it's up to date.Merged it with main. Please review the windows version as i have not tested it in person
Thanks for this, Tim. The branch merges cleanly with main now. Profiles as
save/<id>selected throughHOOKSHOT_PROFILE, the default profile keeping the old uid, and moving saves tosave-backupsinstead of deleting them are a good design, and I like that the env var keeps older games from misreading an argument. Thepick_user()word order matches whatnn_fs.cexpects for the default uid, and the staging copy plus the two confirmation dialogs cover the main data-safety cases. I haven't run the code, this is from reading the diff. A few things before merge:Import takes more than the save and checks very little.
is_save/IsSaveaccept a folder if it contains any one ofaccount.sav,option.savor aslot_NNdirectory, and then the whole folder is copied. If someone points the importer at a bigger folder that happens to contain anoption.sav, everything in it ends up in the profile. Nothing looks at file sizes, and an empty or truncatedaccount.savis accepted. I'd copy onlyaccount.sav,option.savandslot_NN/{progress,caption}.sav, and reject zero-byte files at least. The existing checks are a good start but the size check is the cheap win.Backup and replace aren't one step. In both implementations the profile's folder is moved to
save-backupsand then the staging folder is moved into place. If that second move fails, the profile is left with no saves (they're safe in the backup, but the user is told nothing). A try/except that moves the backup back would close that.Game running. Nothing stops Import or Remove while the game is running on that profile. On Linux the saves get moved from under a running game, which can then write them back out on its next save. Either detect the process or at least mention it in the confirmation text.
hookshot-launch --playdoesn't validate the profile.chosen_profile()in the Python builder falls back to the default when the id is no longer in the list, but the shell script exports whatever is inbuilder.ini. After a removal,--playwould start the game on a fresh, empty save folder for the removed id. Checking againstprofiles.txtthere (or having the game fall back) would be enough.Import folder scan runs on the UI thread (both launchers), with depth 6 and symlinks skipped. Choosing something like the home folder will freeze the window for a while. A lower depth, or a worker thread, would help.
Overlap with #1190. #1190 adds
scripts/oot_saves.pyand docs/saves.md for moving the same files between Hookshot and the emulators (known folders per emulator, copies only the known files, checks that the game or emulator isn't running, backs up the side it overwrites). The two don't touch the same files, but they do the same job: locating emulator saves, picking the right user, copying and backing up. It would be good to agree on one place for the "what is a valid OoT save folder" logic, and for the launcher to adopt the running-process check from there. #1190's--profileis the emulator's profile, not a Hookshot launcher profile, so the two should compose fine once the import target is a profile folder.Testing. The Windows launcher UI hasn't been seen on Windows (as noted in the description), and no checks have run on the PR yet, so the C# tests in
source-regressions.ymlhaven't run in CI. Please keep an eye on that once a maintainer approves the workflows.The Linux unit tests and the pick_user test look thorough. None of the above is a big rewrite.
@ -0,0 +1,162 @@using System.IO;Same as on Linux: this copies the whole folder, and follows junctions/links. Limiting it to the known save files and checking sizes would be safer.
If
Directory.Movefails here afterBackup, the profile is left empty. Restoring the backup in a catch would avoid that.The id from builder.ini is exported without checking it is still in profiles.txt, unlike
chosen_profile()in hookshot_builder.py. After removing a profile,--playwould start the game on a new empty save folder for that id.find_saveswalks up to 6 levels on the UI thread, so choosing a big folder (home) freezes the window. A lower depth or a thread would help.@ -0,0 +1,161 @@"""The launcher's profiles: each one is a user of the game with saves of its own, save/<id> in the game's user folder.This copies everything in
src, not just the game's files. Copying only account.sav, option.sav and slot_NN/{progress,caption}.sav (and refusing zero-byte files) would make the import safer.If
os.replacebelow fails after_backup, the profile is left without saves (they're in the backup). Moving the backup back on failure would keep the 'a failed import leaves the profile as it was' promise.Thanks Tim, the last push closes nearly everything from my first review. I read the diff only, nothing was run.
Fixed:
scripts/oot_saves.pyon both launchers, so only the known save files are copied, zero-byte files and links are refused, and the old profile folder is put back if the copy fails. That covers the "copies the whole folder", the "no size check" and the "failed move leaves the profile empty" points, and removes the duplicatedis_save/IsSavelogic (and the overlap with #1190).hookshot-launch --playnow checks the id againstprofiles.txtbefore exportingHOOKSHOT_PROFILE.Still open:
oot_saves.pick()now fails with "pick one with --profile ", and neither launcher can pass--profile. So choosing an emulator's save folder that has more than one user ends in an error naming a command-line flag. Please either ask the user in both launchers (list the ids fromoot_saves.find_saves, pass--profile) or change the description and the message. The inline comment is on the Linux call.save-backups/<id>-<time>for imports). Imports now back up assave-backups/import-<name>-<time>. Please update it, and the test list if it changed.MainWindow.xaml,MainWindow.xaml.cs,BuilderSettings.cs,hookshot_builder.py,readme.mdandtests/run_all.py, which you also change. GitHub hasn't computed mergeability yet, so I can't say whether it conflicts, but please merge main again.source-regressions.ymland the Windows build are unverified, and so is the Windows launcher itself (as you said).Nit:
Profiles.remove/Removestore the new list after moving the saves, so if writingprofiles.txtfails the profile stays listed with its saves in the backup. Rare, fine to leave.Nit: the Windows import reads the script's output synchronously on the UI thread, so the window pauses for the copy and the process check. Saves are small, so this is only worth a note.
@ -374,13 +387,190 @@ class Builder(tk.Tk):ttk.Label(f, text=folder, wraplength=self.px(560)).pack(anchor="w", **self.pad)When the chosen folder holds saves for more than one user,
oot_savesfails here with "pick one with --profile " and the launcher shows that as is. The launcher has no way to pass--profile, so the user is stuck (the description says the launcher asks which one). Same on Windows. Listingoot_saves.find_saves(chosen)and passing the pick as--profilewould fix it.