Repository navigation
fix: roll saved template results and refuse outputs that failed their checks - #22
Merged
Merged
Conversation
… checks An output written as result.item: t.<template> was not read as a template: the loader logged it as an invalid item but kept the output, players could start it, the station saved the literal t.<template> as its result, and on completion the item could not be built, so the station stayed stuck. - A saved result that is still a t.* template is rolled when the project completes; the rolled item is saved and granted. A project whose recipe is already complete also finishes when its owner opens it again. - result.item: t.<template> is read as result.template, with a load warning telling staff to move it to result.template. - Outputs that fail their load checks stay listed but cannot be started: the player gets station.output_unavailable (with a built-in fallback for older messages.yml files) and the console gets a warning naming the input and the problem, instead of "Research reload failed". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. Summary
Merge Risk:
 |
github-actions
Bot
added
the
size:L
- Roll a saved template, build the reward, and only then save the rolled ref, so a roll that cannot be built never pins the station; warn when that save fails. - Outputs with no aspects and no aspect_pool.pick now fail their load checks (no live Main, Dev or Tutorial output has one), and the reopen finish skips outputs that failed their checks or have an empty recipe. - The reopen finish runs for the main hand only and tells the player when the reward still cannot be given. - Refused starts are logged once per input and output until the next load, with no second log for an unresolved result. - OutputLoader.failedChecks() is read-only; tests use a package hook. - station.output_unavailable now reads "This research is not working right now", since it also covers a reward that cannot be given. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Wording only: a failed save of a rolled result still gives the reward, and OutputLoader notes that the refusal-log set is the manager's state, main thread only and reset on each load. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A real client sends an off-hand interact in the same tick when the main hand did nothing client-side. After a main-hand reopen finished a project, that off-hand half found the lectern free and started a new project from a research paper held in the off hand (seen on Dev). The manager now remembers the lectern and server tick of each player's last main-hand reopen finish (one entry per player, dropped on the next tick, on quit and on unload) and cancels only that player's off-hand interact on the same lectern in the same tick. Off-hand starts on other ticks or other lecterns work as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The main-hand and off-hand packets of one click can straddle a tick boundary, so the off-hand half is ignored in the finishing tick and the next one; the entry expires beyond that. Tests cover the +1 tick case, a +2 tick start, and a main-hand click in the same tick that still starts normally. The field comment now says how entries expire. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Wording only: the field comment says when entries are removed, and the parameterized start test is now startsStillWorkOutsideTheFinishingClick. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
XxFran10xX
marked this pull request as ready for review
November 13, 2026 23:01
Contributor
Author
|
@coderabbitai review |
Action performed
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Status record
ResearchManagerTest138; fullmvn verifylast run at 6554591: 343 tests). The only full-suite failures are the same 31 Windows-only directory-fsync failures inResearchPersistenceTest,StationBackfillTestandStationEstateTestthat also fail on unmodifiedorigin/main; they leave 16 estate lines inResearchManageruncovered locally. Every changed line is covered. Linux CI enforces the 100% line gate.The bug
OutputDef.ResultSpecreadsresult.itemandresult.templateas separate keys. An output written asresult.item: t.greater_runestonetherefore had an item ref that isn't an item:OutputLoaderloggedhas invalid result item 't.greater_runestone'but still registered the output, so players could start it.ResultResolver.resolvereturned the item ref as given, so the station saved the literalt.greater_runestoneasresolved_result_ref.ItemRef.build("t.greater_runestone")returned null.checkProjectCompletionloggedCould not build result itemand returned. The project never completes; it stays on the lectern for good.Tutorial impact
Tutorial's
outputs/minor_runestone.yml,lesser_runestone.yml,greater_runestone.ymlandascendant_runestone.ymlall useresult.item: t.<template>, and every Tutorial start logs the four "invalid result item" errors. Two Tutorial stations have savedt.greater_runestone:data/stations/TFMC_S2_142_19_347.jsonandTFMC_S2_146_19_347.json. Both are mid-research (magic confirmed, other aspects still testing), so they will hit the failure when they finish. Main and Dev use their owna_*_runestoneoutputs and have not.*saved results.The fix
a. Saved templates roll at completion. When a project completes and its saved result is still a
t.*template ref, the template is rolled at that point (nested templates too). The rolled item ref is saved to the station, built and granted, andResearchCompleteEventcarries the rolled item. The console logsResolved saved result template .... If the template no longer exists, the project stays and the warning names the template, output and station. A project whose recipe was already complete when its reward failed now also finishes when its owner opens the lectern again, so it doesn't need another experiment. The two Tutorial stations therefore finish on their own, with no manual edit. The roll is saved only after its item builds, so a roll that can't be built leaves the template saved and rolls again next time; a failed save is logged. The reopen finish runs only for the main hand, skips outputs that failed their checks or have an empty recipe, and tells the player (station.output_unavailable) if the reward still can't be given.b.
result.item: t.*is read asresult.template. At load, staff get a warning:Output 'x' sets result.item: t.y, which is a template. It is read as result.template; write it as result.template in <file>. Setting bothitem: t.*andtemplateis still the existing "not both" error.c. Outputs that failed a load check can't be started. An output with no aspects and no
aspect_pool.picknow also fails its checks, because it would be complete as soon as it started. No live Main (62), Dev (57) or Tutorial (54) output has an empty recipe.OutputLoadernow records outputs that fail any check (OutputLoader.passedChecks). Starting one refuses before the research paper is taken. The player sees the newstation.output_unavailablemessage, "This research is not working right now. Please tell a staff member.", instead of "Research reload failed. Check console for errors." The console gets a warning such as<player> could not start research '<input>': it rolled output '<id>', which failed its load checks .... Unknown, disabled and unresolvable outputs and inputs without outputs use the same message, each with its own log reason. Each input and output pair is logged once per load, not on every right-click.messages.ymlgets the new key. Livemessages.ymlfiles are never overwritten, so the code has a built-in fallback until the key is added.Off-hand half of a finishing click. A real client sends a second, off-hand interact in the same tick when the main hand did nothing on its side. After a main-hand reopen finishes a project, the manager remembers that lectern and tick (one entry per player; it expires after that tick window and is removed on the next station click, quit or unload) and cancels only that player's off-hand interact on the same lectern in that tick or the next (the two packets can straddle a tick boundary), so a research paper in the off hand no longer starts a new project there. Off-hand starts on other ticks or lecterns work as before.
Not in this PR
The Tutorial config fix (move the four runestone outputs from
result.item: t.<template>toresult.template: t.<template>) is a separate live config change. With this PR those outputs already load and work, with a warning.No version bump: Research sets the version from the release tag.
🤖 Generated with Claude Code