fix: Input simulation overlays stay usable without shipping runtime DLLs#1285
Merged
Conversation
Keep the runtime assembly attachable without implicit auto-references so package prefabs can resolve their overlay components in consumer projects. Also remove the demo-only mouse overlay tester from the package prefab and add prefab contract tests to catch editor-only asmdef regressions and package-only missing script references.
Ensure the mouse replay E2E runs against the same Full HD Game View size as its fixture and enables replay visualization so the pointer overlay is exercised. Wire the overlay prefab references through Unity serialization, and avoid duplicate UI dispatch when InputSystemUIInputModule already consumes injected mouse state.
Tag every GameObject in the shared input visualization prefab as EditorOnly so Unity can exclude the instantiated overlay hierarchy from player scenes while keeping prefab script references resolvable in the editor.
Keep the runtime asmdef player-compatible so overlay prefabs resolve in the Editor, then remove uLoopMCP.Runtime.dll from Player build assembly lists through Unity's build assembly filter callback.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughWraps runtime overlay and visualization types in editor-only guards, adds a build-time filter to remove the runtime DLL from player builds, introduces prefab import validation tests, refactors InputReplayer to own overlay lifecycle and UI dispatch behavior, removes overlay init from ReplayInputTool, and stabilizes E2E Game View resolution. ChangesEditor-only input visualization and replay control
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Delay non-loop replay completion until InputSystemUIInputModule can consume the terminal release, keep loop cadence gapless, and preserve pointer dispatch mode across press/release. Keep the runtime assembly in player test builds so PlayMode test players can resolve package test references.
Bump the TypeScript server Hono override past the vulnerable <=4.12.20 range so the NPM supply chain security audit passes.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/src/Editor/Api/McpTools/ReplayInput/InputReplayer.cs (1)
155-163:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClamp replay progress during the delayed-completion frame.
On Line 161, the method returns after
_currentFramehas already advanced pastTotalFrames. That leavesIsReplaying == truefor one frame whileCurrentFrame/Progressexpose an unrecorded extra frame, which can leak a> 1.0progress value to any observer.Suggested fix
if (_eventIndex >= _data.Frames.Count && _currentFrame > _data.Metadata.TotalFrames) { if (_delayCompletionForUiModule) { _delayCompletionForUiModule = false; + _currentFrame = _data.Metadata.TotalFrames; return; } if (_loop)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/src/Editor/Api/McpTools/ReplayInput/InputReplayer.cs` around lines 155 - 163, When delaying completion in InputReplayer (check _delayCompletionForUiModule inside the block where _currentFrame was just incremented and _eventIndex >= _data.Frames.Count), clamp the exposed replay position so observers don't see a frame > TotalFrames: before returning, set _currentFrame (or the value used by CurrentFrame/Progress) to Math.Min(_currentFrame, _data.Metadata.TotalFrames) (or decrement _currentFrame back to _data.Metadata.TotalFrames) so IsReplaying can remain true for the extra UI frame without letting CurrentFrame/Progress exceed TotalFrames; update the branch that returns when _delayCompletionForUiModule is true to perform this clamp.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Packages/src/Editor/Api/McpTools/ReplayInput/InputReplayer.cs`:
- Around line 155-163: When delaying completion in InputReplayer (check
_delayCompletionForUiModule inside the block where _currentFrame was just
incremented and _eventIndex >= _data.Frames.Count), clamp the exposed replay
position so observers don't see a frame > TotalFrames: before returning, set
_currentFrame (or the value used by CurrentFrame/Progress) to
Math.Min(_currentFrame, _data.Metadata.TotalFrames) (or decrement _currentFrame
back to _data.Metadata.TotalFrames) so IsReplaying can remain true for the extra
UI frame without letting CurrentFrame/Progress exceed TotalFrames; update the
branch that returns when _delayCompletionForUiModule is true to perform this
clamp.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3e31ba8c-ba5e-4d3c-8d4f-7ad0760f9269
⛔ Files ignored due to path filters (1)
Assets/Tests/PlayMode/InputReplayerCompletionTests.cs.metais excluded by none and included by none
📒 Files selected for processing (5)
Assets/Tests/Editor/RuntimeAssemblyBuildFilterTests.csAssets/Tests/PlayMode/InputReplayerCompletionTests.csAssets/Tests/PlayMode/SimulateMouseDemoE2ETests.csPackages/src/Editor/Api/McpTools/ReplayInput/InputReplayer.csPackages/src/Editor/Build/RuntimeAssemblyBuildFilter.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- Assets/Tests/Editor/RuntimeAssemblyBuildFilterTests.cs
- Packages/src/Editor/Build/RuntimeAssemblyBuildFilter.cs
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
1 participant
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
Changes
Verification
uloop compile --wait-for-domain-reload trueuloop run-tests --test-mode EditMode --filter-type regex --filter-value io.github.hatayama.uLoopMCP.InputVisualizationCanvasPrefabTestsuloop run-tests --test-mode EditMode --filter-type regex --filter-value io.github.hatayama.uLoopMCP.(RuntimeAssemblyBuildFilterTests|InputVisualizationCanvasPrefabTests)uLoopMCP.Runtime.dllwas absent from the Player managed assemblies.Overview
This PR fixes a regression where input simulation overlay prefab script references were not resolving in the Editor and prevents the prefab-only runtime DLL from being included in Player builds when overlay objects are excluded. Overlays show UI again in Editor Play Mode while Player builds no longer include the small prefab-only runtime assembly when overlays are excluded.
Changes Summary
#ifUNITY_EDITOR) to exclude overlay classes from non-editor compilation:Tests & Verification
Architecture
classDiagram class RuntimeAssemblyBuildFilter { -callbackOrder: int +OnFilterAssemblies(BuildOptions, string[]) string[] -IsRuntimeAssembly(string) bool } class InputReplayer { -_showOverlay: bool +EnsureOverlayForReplay() +ClearOverlayState() -ClearPointerState() `#ApplyUiEvents`() } class McpConstants { +RUNTIME_ASSEMBLY_FILE_NAME: string } RuntimeAssemblyBuildFilter --|> IFilterBuildAssemblies RuntimeAssemblyBuildFilter --> McpConstants InputReplayer -.-> ReplayInputOverlayUser Impact
Commit Notes