[display-space-ranchanimal-movement]: Display-space seam for RanchAnimal movement - #58
Merged
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughReplaces the Rust-to-React display-region interface from ChangesRanchAnimal Display-Space Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/usePigMovement.ts (1)
134-147:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
hitTestScaleandspannever reachupdatePigRects.These lines start treating Rust
DisplaySpaceas the geometry source, butsyncRects()still sizes/scales rects fromwindow.innerWidth/window.innerHeightandwindow.devicePixelRatio. That leaves the newspanandhitTestScalefields unused, so hit regions can drift on mixed-DPI or non-primary-monitor layouts.Suggested fix
-function syncRects(pigs: PigState[], wide: boolean): void { - const dpr = window.devicePixelRatio || 1; +function syncRects(pigs: PigState[], wide: boolean, displaySpace: DisplaySpace): void { + const scale = displaySpace.hitTestScale; // Wide rect (detail open or dragging) keeps overlay interactive across the full viewport. const rects = wide - ? [{ x: 0, y: 0, size: Math.max(window.innerWidth, window.innerHeight) * dpr * 2 }] - : buildHitRects(pigs, dpr); + ? [{ x: 0, y: 0, size: Math.max(displaySpace.span.w, displaySpace.span.h) * scale * 2 }] + : buildHitRects(pigs, scale); updatePigRects(rects).catch(() => {}); }🤖 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 `@src/hooks/usePigMovement.ts` around lines 134 - 147, displaySpaceRef is updated from Rust but syncRects()/updatePigRects still compute sizes from window.innerWidth/innerHeight/DPR, so new DisplaySpace.hitTestScale and span are never applied; update updatePigRects (and any call sites of syncRects) to read displaySpaceRef.current.hitTestScale and .span (from DisplaySpace via setDisplaySpace/displaySpaceRef) and use those values to scale and position pig rects and hit regions instead of relying on window.* and devicePixelRatio, ensuring hitTestScale multiplies geometry and span is used to offset/translate coordinates for mixed-DPI or non-primary-monitor layouts.
🤖 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.
Inline comments:
In `@src-tauri/src/display/mod.rs`:
- Around line 76-82: compute_display_space is redundantly recomputing the span:
change monitor::compute_display_space to return both the DisplaySpace and the
SpanBounds (e.g., Option<(DisplaySpace, SpanBounds)>) so the span is computed
once inside compute_display_space (use its existing compute_span call and return
its result), then update the caller in mod.rs to destructure the returned tuple
into (display_space, bounds) and remove the second call to
monitor::compute_span; update signatures and all call sites of
compute_display_space accordingly (symbols: compute_display_space, compute_span,
DisplaySpace, SpanBounds).
In `@src/api/pig.ts`:
- Line 3: The API file imports the DisplaySpace type from ranchAnimalMovement
which couples layers; extract DisplaySpace into a shared type module (e.g.,
create src/types/display.ts exporting DisplaySpace) and update both pig.ts and
ranchAnimalMovement.ts to import DisplaySpace from that new module instead of
../lib/ranchAnimalMovement so the API depends only on the shared types.
In `@src/hooks/usePigMovement.ts`:
- Around line 230-241: The initial pig seeding reads displaySpaceRef.current too
early so initPig uses defaultDisplaySpace() before the Rust display-space
listener is installed; fix by ensuring initPig always uses the up-to-date
display-space when mapping focuses: either read displaySpaceRef.current inside
the mapper (move the displaySpace read into the focuses.map callback) or delay
the initial sync until after the subscription effect that installs the Rust
listener updates displaySpaceRef (e.g., trigger setPigs once more when that
subscription sets displaySpaceRef). Update the useEffect that does setPigs, the
initPig calls, and the subscription effect that installs the Rust listener so
the mapper references the live displaySpaceRef (displaySpaceRef, initPig,
setPigs, focuses, subscription effect installing Rust listener).
---
Outside diff comments:
In `@src/hooks/usePigMovement.ts`:
- Around line 134-147: displaySpaceRef is updated from Rust but
syncRects()/updatePigRects still compute sizes from
window.innerWidth/innerHeight/DPR, so new DisplaySpace.hitTestScale and span are
never applied; update updatePigRects (and any call sites of syncRects) to read
displaySpaceRef.current.hitTestScale and .span (from DisplaySpace via
setDisplaySpace/displaySpaceRef) and use those values to scale and position pig
rects and hit regions instead of relying on window.* and devicePixelRatio,
ensuring hitTestScale multiplies geometry and span is used to offset/translate
coordinates for mixed-DPI or non-primary-monitor layouts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b4735f0d-e4d6-40be-b315-1ebbdd48323d
📒 Files selected for processing (10)
CONTEXT.mdissues/049-display-space-pig-movement-seam.mdsrc-tauri/src/display/mod.rssrc-tauri/src/display/monitor.rssrc-tauri/src/display/overlay.rssrc/api/pig.tssrc/hooks/usePigMovement.tssrc/lib/ranchAnimalMovement.test.tssrc/lib/ranchAnimalMovement.tssrc/types/pig.ts
💤 Files with no reviewable changes (1)
- src/types/pig.ts
archae0pteryx
force-pushed
the
codex/display-space-ranchanimal-movement
branch
from
May 14, 2026 19:58
671048d to
e1fbcf2
Compare
archae0pteryx
marked this pull request as ready for review
May 14, 2026 20:34
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.
What changed
DisplaySpacepayload with span, spawn region, movement regions, and hit-test scale.display-space.advanceRanchAnimalmovement module for shared movement behavior.usePigMovementto delegate roam/freeze/friction/turn/soft-steer/hard-clamp behavior to that module.CONTEXT.mdwith the RanchAnimal/display-space decisions.Why
Issue file:
issues/049-display-space-pig-movement-seam.mdCompletion promise:
This keeps monitor geometry policy local to Rust display-space computation and makes movement behavior testable without opening Tauri windows.
Validation
task checkSummary by CodeRabbit