Repository navigation
Conversation
Joey-Magnus
left a comment
There was a problem hiding this comment.
Thanks for this. Picking a version is a reasonable thing to want, and the tag validation before building API URLs and the "GitHub returned X, not the requested Y" check are good. But by the description's own account this hasn't been compiled or run on Windows yet, and CI has only run the title check (the builder package job hasn't run). Reading the code, I also see a few design problems that need sorting out before it can merge. I covered all five files; I couldn't build the WPF project here, so everything below comes from reading the source.
-
blocking: Untested. "Windows compilation and end-to-end launcher testing remain to be completed." CONTRIBUTING.md asks for the relevant checks and CI to pass before review, and a launcher change has to be run at least once: open the launcher with an update available, switch to an older release, and switch back. Please also add screenshots of the new launcher panel; it shows on screen, so CONTRIBUTING.md (Commits and pull requests, item 5) requires them.
-
blocking
builder/HookshotBuilder/MainWindow.xaml.cs:224,:241-247,:288-307: this replaces the ordinary update flow for every player, not just those who want a specific version. With no pin, the combo selectsupdateTarget, so whenever an update exists the launcher shows "Switch vX to vY: the game will be rebuilt" with a Switch version button. The old highlighted "Update available: X" status is gone, every routine update now goes through a warning dialog, writes a backup, and touches the pin file, andOlderOk's confirmation for an older release on a track switch is dropped. The "X is out: build this folder again…" message for folders without game sources is replaced by a vaguer one. I'd keep the existing Update path unchanged when the selection is the update target, and use the switch dialog and backup only when the player picks some other version. -
should fix (design): pinning and downgrading can't work for any release that exists today.
host/hos_update.c:190-194only honours the pin in games built from this code, and the launcher that reads it is this builder. Switching to any release in the dropdown (all of them predate this PR) installs that release's builder and builds that release's game. From then on, neither the old launcher nor the old game's update offer knows abouthookshot_update_policy.txt: the player is offered the latest version again on every launch, and the picker is gone (as the description notes). So the "Stay on the selected version" checkbox appears to work but has no effect after a downgrade. At a minimum, filter the dropdown to releases that carry this feature, or say plainly in the UI that a pin only holds on versions from X onward. -
question: downgrading runs an older
make_game.py --updateon a folder last built by a newer one (newer compiled objects, caches,hookshot_version.txtcontents). Has anyone checked that an older builder handles a newer folder rather than reusing something incompatible? If that isn't known, a downgrade might be safer as a fresh build into a new folder. -
should fix
builder/HookshotBuilder/VersionPolicy.cs:23-45: the backup goes to%LOCALAPPDATA%\HookshotBuilder\version-backups\<timestamp>with nothing pruning it and nothing telling the player where it is. Every switch adds a full copy of the saves. Show the path after the backup (or in the dialog), and keep the last few. It also skipslanguage.txt, which CONTRIBUTING.md lists with the settings. -
minor
builder/HookshotBuilder/MainWindow.xaml.cs:252-260: changes to the pin checkbox are only saved when the selection is the installed version. If a player picks another release, toggles the checkbox, then picks the installed version again, the checkbox no longer matches the file. ResyncLaunchPinVersion.IsCheckedfromVersionPolicy.PinnedTaginRefreshLaunchSelection. -
minor
builder/HookshotBuilder/Updater.cs:24-44:ReleaseTagsreads one page of 100 releases and filters prereleases afterwards, so once the repository has more than 100 releases (RCs included), older stable ones drop out of the list. It's also a second unauthenticated GitHub API call on every launcher open, on top ofLatestTag(the anonymous limit is 60 an hour). Fetching the list only when the player opens the dropdown would avoid that. -
minor
builder/HookshotBuilder/Updater.cs:61:FromCommandLinenow passesnamed: tag != null, which also changes the game's own update offer (hos_update.cstarts the builder with--update <folder> --tag <release>) from thelatestendpoint to the exact tag. That's probably fine, maybe better, but it changes existing behaviour outside the launcher and should be in the description.
The PR also needs a "why" opening line in player terms, and a note in CONTRIBUTING.md's Hookshot Builder section about the new panel.
Joey-Magnus
left a comment
There was a problem hiding this comment.
Still open from the earlier review, and not yet built or run on Windows by its own description:
- It replaces the normal Update flow for every player. With no pin, every routine update goes through the switch dialog and a backup, and the
OlderOkconfirmation is dropped (MainWindow.xaml.cs~224-307). - Pinning can't hold after a downgrade: every release in the dropdown predates the pin file (
hos_update.c:190), so the old launcher and game ignore it. - Backups pile up with nothing pruning them, and they skip
language.txt. ReleaseTagsonly reads one page of 100 releases.
hckrman101
left a comment
There was a problem hiding this comment.
Thanks for this, a way to pick a release is a good thing to want. I read all five files and the discussion; I did not build it. I agree with Joey-Magnus's two reviews (untested on Windows and no screenshots, the normal Update flow replaced for everyone, pin not holding after a downgrade, unpruned backups, the one-page release list) and won't repeat them. I'd treat the first two as the blockers. A few more points, and how this fits with the other open update PRs.
Further findings
MainWindow.xaml.csLaunchUpdateClick: the pin is written beforeUpdateFolderruns. If the download or build fails, or is cancelled, the folder is still on the old version but the pin file names the new one. The game ignores it (it only honours a pin equal to the installed version), but the launcher then preselectspinned ?? updateTargetand shows a switch to a version that never got installed. Write the pin after a successful switch, or clear it on failure.ShowLauncher: ifReleaseTags()throws (rate limit, bad JSON) the dropdown is reduced to the installed version, so the Update button disappears even whenLatestTagworked and an update is known. KeepupdateTargetin the list on that path, so a releases-list failure doesn't hide a routine update.hos_update.c: the pin check is only in the Windowshos_update_check. Fine for a Windows-only feature, but the Linux function reads the sameuser/folder and will ignore a pin file; a line in the description saves a bug report later.Updater.ValidTag/VersionPolicyrepeat the same tag rule twice, and it rejects+while the shared one in #2430 (TagOk) allows it. Once #2430 is in, use that instead of a third copy.- No test. A source check in
tests/test_update_tracks.py-style (pin line honoured only when equal to the installed version) would be cheap and would run in CI.
- #2519 (NotRexed) adds a guard in
hos_update_checkso a folder newer than the latest release is not offered the older one. Different hunk from the pin check here, so I don't expect a textual conflict, and the two agree: the pin suppresses the offer on the pinned version, #2519 suppresses it for a build newer than the latest. After a downgrade neither applies, which is the problem Joey raised. - #2430 (Joey-Magnus) also edits
Updater.cs(tag validation inUpdateFolder,LatestTag, regexes with\z) andhos_update.c(builder path checks, env handling around the builder start). GitHub says this PR merges cleanly now, but whichever lands second needs a rebase aroundUpdateFolderand the end ofhos_update_check. #2430 already rejects a bad tag at the top ofUpdateFolder, so theValidTagcheck added here becomes redundant.
CI only ran the title check, so nothing builds the launcher yet. I'm leaving this as a comment; whether and when it merges is for the maintainer to decide.
Summary
Adds version management to the Hookshot launcher, allowing users to select and install specific releases instead of always being limited to the latest available version.
Users can now upgrade, downgrade, or stay on their preferred version directly through the launcher.
Changes
Implementation
The changes extend the existing WPF launcher and updater without replacing the game's compilation pipeline.
Version preferences are stored within the installation's
userdirectory, allowing them to persist across rebuilds. The game's update-check logic also respects pinned versions.Notes
Testing