chore: Document reflection-based state recovery and pause-point capture edge cases#1874
Merged
Merged
Conversation
… edge cases
Second-round PlayMode/E2E verification surfaced several pause-point
and dynamic-code techniques that were only discovered ad hoc during
testing, not written down anywhere:
- execute-dynamic-code: writing private fields via reflection to
recover or steer a stuck PlayMode session without stopping Play,
editing code, or recompiling.
- pause-point: UnityEngine.Object captures only ever carry identifying
info, never custom fields; nested-field previews off `this` bottom
out at MaxCollectionPreviewDepth (2) per captured variable; setting
up a self-progressing game's board via TryGetCapturedValue("this")
while paused, reserving real input for only the verified move; using
a temporary if-guarded Debug.Assert as an observation line when every
reachable line runs unconditionally every frame; and that
await-pause-point now extends a marker's own timeout before waiting,
so enable's --timeout-seconds no longer needs to be inflated to
survive a slow multi-step CLI sequence.
Regenerated .claude/.agents skill copies with `uloop skills install`
and confirmed byte-identical match against source. This also picks up
one already-merged source change (#1729's zero-hit hint) whose
generated copies had drifted out of sync before this PR.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughChangesSkill guidance documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Advisor review of PR-I found three documentation defects: the timeout note conflated the setup window with the wait window even though a marker already expired before await-pause-point starts cannot be resurrected; the if+Debug.Assert observation pattern guarded on the same postcondition it asserted, making the assert a tautology instead of checking the actual mutation; and the reflection-is-otherwise-disallowed line stated the author's personal global policy as if it were a project-wide skill contract, which this skill package ships to every user.
The previous fix commit reworded these sections in the implementer's own words. The user asked that the skill wording itself come from the advisor, so this replaces that wording verbatim with the text the advisor supplied for the timeout note, the if+Debug.Assert observation pattern, and the reflection rationale.
Advisor-authored pass over the remaining implementer-written additions: - Depth-cap note now names the symptom (type name where contents were expected), shows the this-vs-local contrast with a concrete example, and states that primitives and ToString-overriding types always render - UnityEngine.Object note drops the incorrect ToString() suggestion (the formatter only reads .name) and shows the rename workaround - Self-progressing-game note explains why the race is lost (CLI round trips outlast the game tick) before giving the pattern - Live-state-injection intro replaces the self-contradictory "unrecoverable state" framing with concrete trigger cases Generated copies regenerated via uloop skills install --claude --agents.
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.
Summary
User Impact
execute-dynamic-code/await-pause-pointhad no documented way to recover a stuck PlayMode session without stopping Play and recompiling, no warning aboutUnityEngine.Objectcapture limits or capture-depth limits, no documented pattern for arranging state in a self-progressing game, no tip for pausing on lines that run every frame, and no note that a marker's timeout no longer needs padding for slow setup steps.Changes
ExecuteDynamicCodeskill: documents using reflection to write private fields directly and recover/steer a stuck PlayMode session, without stopping Play, editing code, or recompiling.PausePointskill:UnityEngine.Objectvalue only ever carries identifying info, never custom fields, and how to get a custom field's value instead.MaxCollectionPreviewDepth(2-level) budget on nested-field previews offthis, independent per captured variable, with a tip to target a line where the value is a direct local/parameter instead.TryGetCapturedValue("this")while paused, reserving real simulated input for only the verified move.Debug.Assertas an observation line for cases where every reachable line runs unconditionally every frame.await-pause-pointnow extends a marker's own timeout before waiting (see the pause-point marker expiry fix), soenable-pause-point --timeout-secondsno longer needs to be inflated to survive a slow multi-step CLI sequence..claude/.agentsskill copies viauloop skills install --claude --agents. This also picks up one already-merged source change (the find-game-objects zero-hit hint) whose generated copies had drifted out of sync before this PR.Verification
dist/darwin-arm64/uloop compile --project-path <repo>-> 0 errors, 0 warnings (doc-only change; confirms no incidental breakage).diffbetween each edited sourceSKILL.mdand its regenerated.claude/.agentscopy -> byte-identical.