Skip to content

Mod Manager: rework the UI for players; name the copyright holder - #95

Merged
dh0er merged 11 commits into
mainfrom
claude/mod-manager-ui-ux-review-5e8332
Aug 20, 2026
Merged

Mod Manager: rework the UI for players; name the copyright holder#95
dh0er merged 11 commits into
mainfrom
claude/mod-manager-ui-ux-review-5e8332

Conversation

@dh0er

@dh0er dh0er commented Aug 19, 2026

Copy link
Copy Markdown
Owner

Why

The Mod Manager read like a diagnostic tool rather than something a player installs mods with. Statuses and errors spoke in deployment and ownership terms, every component row carried its raw conflict targets plus a floating coverage grade, and two empty states rendered a button caption as their only text.

Separately, the MIT notice still said goresave contributors — a collective placeholder from the pre-rename repo — while the About dialogs said GORE contributors and the shipped binaries said three different things again.

What changed

chore: name Daniel Hoer as the copyright holder (57 files, 80 lines)

All three installers show the same repository LICENSE as their EULA, so the notice, the About dialogs, the VERSIONINFO blocks and the installer publisher fields now agree — including Mod Studio's leftover com.example template default.

Spelled ASCII deliberately: LICENSE carries no BOM and Inno Setup decodes its license page with the system ANSI codepage, so a UTF-8 umlaut would render as mojibake. Hoer is the standard transliteration and matches the existing git author identity.

feat(mod-manager): rework the UI for players, … (55 files, 6 new)

  • One vocabulary, plain language across all twelve locales. A whole-file replacement is named by its destination ("All game text") instead of "File"; the four container mechanisms all read as "game files"; chips and detail rows use the same word for the same thing.
  • New "Advanced details" setting, off by default, holding per-component targets, coverage grades, the import source and match reason, the packaging kind, and the managed-file record. Nothing was deleted — it moved.
  • Coverage grades head the list they grade instead of floating on the row, where a lone "Unknown" read as a verdict on the mod.
  • Start game button at the far right of the tab row, resolving the executable from the configured path (root or exe) and disabled while an operation writes to the installation.
  • Update settings: automatic checks can be turned off and run on demand; the portable build now checks the same feed and points at the download page.
  • Display fixes: long paths wrap and can be copied, the Windows extended-length prefix is stripped, import timestamps are localized, ownership groups are left-aligned, and the redundant "Disabled" label is gone from the list.

Verification

flutter analyze clean in all three apps. Tests: mod-manager 319 (up from 292), save-editor 601, mod-studio 2364, release verifier 12.

The new mod-manager tests pin the plain/advanced split, the shared vocabulary across every locale, executable resolution, and the narrow-window fallbacks — three existing layout tests caught real overflow regressions while the Start-game button was being placed, which is why it ended up in the tab row.

Two non-obvious consequences of the rename are covered too: the release verifier pins the built binaries' publisher metadata, and Inno pads those fields to fixed widths, so the longer name shifts the pinned padding (CompanyName 55 → 49, LegalCopyright 56 → 50).

Not verified here

The portable updater's network path (fetch appcast → compare version → dialog) only runs in a real Windows release build. It is the same code the Save Editor ships, but this is its first use in the Manager.

🤖 Generated with Claude Code


Note

Medium Risk
Adds process launch of the game exe and a new portable update-check path that fetches the appcast and opens a browser. Copy and settings changes are large but do not alter deploy/apply engine logic.

Overview
Reworks Mod Manager as a player-facing 0.2.0: plain-language copy across locales, technical conflict/ownership details behind a new Advanced details setting (off by default), a Start game action, and update checks that can be disabled or run on demand.

Start game sits at the far end of the tab row. It rebuilds G1R-Win64-Shipping.exe from the install root (not whatever .exe was picked), launches detached, and is disabled while a Manager operation is writing the install.

Updates: installer builds still use WinSparkle; portable builds now poll the same appcast and point at the versioned release page. Auto-check is persisted and can be toggled at runtime.

Also names Daniel Hoer as copyright holder in LICENSE, About, and installer VERSIONINFO (ASCII spelling for Inno’s ANSI license page). Status/recovery wording is simplified (e.g. “Remove all from game” instead of Undeploy).

Reviewed by Cursor Bugbot for commit daa6002. Bugbot is set up for automated code reviews on this repo. Configure here.

dh0er and others added 2 commits August 19, 2026 23:47
The MIT notice said "goresave contributors", a collective placeholder left
over from the pre-rename repo, while every About dialog said "GORE
contributors" and the shipped binaries said three different things again
("goresave contributors", "dh0er", and Flutter's "com.example" template
default). There is one author, so name him.

All three installers show the same repository LICENSE as their EULA, so the
notice, the About dialogs, the VERSIONINFO blocks and the installer publisher
fields now agree.

Spelled ASCII on purpose. LICENSE is read by Inno Setup as the license page
and carries no BOM, so Inno decodes it with the system ANSI codepage; a UTF-8
umlaut would render as mojibake there. "Hoer" is the standard transliteration
and matches the existing git author identity.

The release verifier pins the expected publisher metadata of the built
binaries, and Inno pads those VERSIONINFO fields to fixed widths, so the
longer name also shifts the pinned padding (CompanyName 55 -> 49,
LegalCopyright 56 -> 50).

The holder is a name, not a translatable string: every locale now carries the
same value instead of a translated "contributors". The Mod Manager's own
About string lands with the UI commit that rewrites its localization files.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…te settings

The app read like a diagnostic tool: statuses and errors spoke in deployment
and ownership terms, every component row carried its raw conflict targets and
a floating coverage grade, and two empty states rendered a button caption as
their only text. Reword it for the people who install mods, and move the
technical layer behind a switch instead of deleting it.

- One vocabulary, plain language. "Bereitstellung" and friends become what
  actually happened; a whole-file replacement is named by its destination
  ("All game text") rather than "File"; the four container mechanisms all read
  as "game files". Chips and detail rows now use the same word for the same
  thing. All twelve locales.
- New "Advanced details" setting, off by default, holding per-component
  targets, coverage grades, the import source and match reason, the packaging
  kind, and the managed-file record.
- Coverage grades now head the list they grade instead of floating on the row,
  where a lone "Unknown" read as a verdict on the mod.
- Start game button at the far right of the tab row. It resolves the executable
  from the configured path (root or exe) and is disabled while an operation is
  writing to the installation.
- Update settings: automatic checks can be turned off and run on demand. The
  portable build now checks the same feed and points at the download page.
- Display fixes: long paths wrap and can be copied, the Windows extended-length
  prefix is stripped, import timestamps are localized, ownership groups are
  left-aligned, and the redundant "Disabled" label is gone from the list.

Verified with 319 tests (up from 292); the new ones pin the plain/advanced
split, the shared vocabulary across all locales, executable resolution, and the
narrow-window fallbacks that three existing layout tests caught.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dh0er

dh0er commented Aug 19, 2026

Copy link
Copy Markdown
Owner Author

@codex review

Comment thread apps/mod-manager/lib/app/game_paths.dart

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4af07bfa13

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/mod-manager/lib/app/game_paths.dart Outdated
… one

gameExecutableFor returned any existing path ending in .exe before rebuilding
the shipping binary from the install root. The Settings picker accepts any
.exe, and G1R/Binaries/Win64 ships more than one, so Start game could spawn a
helper binary instead of the game. A stale shared game_path could do the same.

Always rebuild from the resolved install root. That still resolves correctly
when the real game exe was picked, because the root walk climbs out of
Binaries/Win64 either way, and it yields null — a disabled button — for an .exe
outside any install instead of launching it.

Reported independently by Cursor Bugbot and Codex on #95.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dh0er

dh0er commented Aug 19, 2026

Copy link
Copy Markdown
Owner Author

@codex review

1 similar comment
@dh0er

dh0er commented Aug 19, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 8b266868d8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Close the Unreleased section, bump the app version, and point the root
download table and the app's own prerelease notice at 0.2.0. The release
workflow requires the tag to match the pubspec version exactly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7b29cfaa6f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/mod-manager/lib/app/domain/desktop_updater.dart Outdated
The hourly timer fired _runPortableCheck without tracking the previous one.
That call awaits its own dialog, so an unattended machine with an update
available would stack another modal — and another network request — every hour
the prompt went unanswered.

Guard the whole check with an in-flight flag so a running check, including one
parked on an open dialog, suppresses the next tick. The dialog is modal, so a
manual check cannot overlap one either.

Reported by Codex on #95.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dh0er

dh0er commented Aug 19, 2026

Copy link
Copy Markdown
Owner Author

@codex review

Comment thread apps/mod-manager/lib/app/domain/desktop_updater.dart

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0260920f29

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/mod-manager/lib/app/domain/desktop_updater.dart Outdated
Comment thread apps/mod-manager/lib/app/domain/desktop_updater.dart Outdated
…able

The in-flight guard added in 0260920 was too blunt, and the feed request it
guards had no timeout at all.

- Time-limit every network step (connect, response, body read). A stalled
  connection or a response that opens and never finishes would otherwise hang
  the check forever, and with it the guard, silently disabling every later
  check for the rest of the process.
- Only a background tick yields to a running check. A manual check now waits
  for the active one and then runs its own, because "Check for updates now"
  promises a result and the Settings copy says no control does nothing.
- Contain failures inside the check. Callers fire it unawaited, and a manual
  check may now await a background one, so a failure must not surface as an
  unhandled async error or be rethrown into the caller that waited.

Reported by Codex and Cursor Bugbot on #95.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dh0er

dh0er commented Aug 20, 2026

Copy link
Copy Markdown
Owner Author

@codex review

Comment thread apps/mod-manager/lib/app/domain/desktop_updater.dart Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 02c442bedd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/mod-manager/lib/app/domain/desktop_updater.dart Outdated
02c442b let a manual check wait for a running one, but never re-read the slot
afterwards. The Settings button stays enabled while a check runs, so two clicks
could queue on the same future, both wake up, and both start a check with
overlapping feed requests and stacked dialogs.

Re-read the slot after every wait, and claim it through a Completer assigned
before the check's first suspension point. Assigning the check's own future
left a gap: an async body runs up to its first await before returning, so a
second waiter could claim the slot in between.

Reported by Codex and Cursor Bugbot on #95.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dh0er

dh0er commented Aug 20, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8f7535e921

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/mod-manager/lib/app/domain/desktop_updater.dart Outdated
The update dialog's Download button fired launchUrl unawaited and discarded
its result. launchUrl reports failure both by returning false and by throwing,
so on a machine with no registered browser the dialog closed and nothing
happened — the one outcome a Download button must not have — with a possible
unhandled async error alongside.

Await the launch and, on failure, say so through the root navigator, including
the release page address so it stays reachable. Info dialogs are selectable now
so that address can be copied.

Reported by Codex on #95.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dh0er

dh0er commented Aug 20, 2026

Copy link
Copy Markdown
Owner Author

@codex review

Comment thread apps/mod-manager/lib/app/domain/desktop_updater.dart Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aac29084f3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/mod-manager/lib/home_page.dart
… the lock

Two independent races, both reported on #95.

Start game trusted the enabled state captured when its callback was built. An
Apply starting between that frame and the tap left a stale callback that still
launched, defeating the guard against starting Gothic while its installation is
being written. Recheck inside the handler, as every other action here does.

The Download button did its launching in the dialog's own callback, which
outlives the dialog: popping it completed the check, released the active-check
lock, and let a queued manual check open another modal while the fallback
prompt was still up. The dialog now only reports the choice; the caller does
the launching, inside the lock.

The first race is covered by a test that fires the stale callback directly
while a parked library read holds an operation in flight. Verified it fails
without the guard.

Reported by Codex and Cursor Bugbot on #95.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dh0er

dh0er commented Aug 20, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: b63a51e543

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…est it

Six of the eight bugs found while reviewing this PR were in the portable update
path, and every fix for them landed untested: the whole path is gated behind
kReleaseMode, its functions were private, and it talked to the real network and
real windows. Reviewers were standing in for tests.

Extract the part that was actually wrong — the queueing rules — into
PortableUpdateChecker, with the feed, the version, the two prompts and the
browser behind injectable hooks. The rules are unchanged; they are now
reachable.

Eleven tests cover: a background tick yielding to a running check, a manual
check waiting its turn rather than being dropped, two queued clicks staying
serialized, an unanswered prompt blocking the next tick, a failing check
releasing the slot, a cancelled poll not raising a late prompt, and the
download-page outcomes.

Each was checked against the bug it exists for by reintroducing that bug and
confirming the test fails. One did not, at first: the fake feed returned
without suspending, so two checks could never be observed overlapping and the
serialization assertions passed either way. The fakes now always yield.

Still uncovered: the feed request's own timeouts, which live in the real
HttpClient call.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four files the previous commit reformatted without touching their behaviour.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dh0er

dh0er commented Aug 20, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: daa6002443

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@dh0er

dh0er commented Aug 20, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@dh0er

dh0er commented Aug 20, 2026

Copy link
Copy Markdown
Owner Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit daa6002. Configure here.

@dh0er
dh0er merged commit 228430b into main Aug 20, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant