feat: 016 — real pig sprite sheet - #15
Merged
Merged
Conversation
Replace emoji placeholder with 4-direction sprite sheet animation. - PigDirection type: "front" | "right" | "back" | "left" - direction4() derives facing from dominant velocity axis (4-way) - PigSprite renders via background-position on the 4×4 sheet (background-size: 192×192 normalises 921×910 sheet to clean 48px frames) - image-rendering: pixelated keeps crisp pixel art at display scale - Bob animation preserved via frame-indexed top offset - vite-env.d.ts adds PNG import types for TypeScript
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughPigs now render from a 4×-direction sprite sheet and use a 4-way ChangesPig rendering & movement (frontend)
Overlay & Tray (Tauri native side)
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App (React)
participant Hook as usePigMovement
participant RAF as rAF loop
participant Renderer as PigSprite / DOM
App->>Hook: mount with focuses, selectedId
RAF->>Hook: tick(dt, viewport)
Hook->>Hook: integrate position, reflect edges, compute direction4
Hook->>Renderer: provide PigState[] (selected pig frozen)
Renderer->>App: click -> set selectedId
App->>Hook: update selectedId (refied for rAF)
sequenceDiagram
autonumber
participant Config as MonitorsState/DisplayConfig
participant Overlay as OverlayManager
participant OS as Windowing system
Config->>Overlay: enabled monitor indices
Overlay->>Overlay: compute union bounding box
alt enabled set non-empty
Overlay->>OS: ensure_shown("overlay-0", bbox)
Overlay->>OS: destroy legacy "overlay-1..n" if exist
else none enabled
Overlay->>OS: destroy("overlay-0") if exists
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Multiple webview windows each run their own React instance → duplicate pig sets. Fix: compute the union bounding box of all enabled monitors and resize a single overlay-0 window to cover them. Pigs roam the full spanning area. - overlay_manager: apply() builds one spanning MonitorInfo from enabled monitor bounds; cleans up legacy overlay-1+ windows - usePigMovement: window.innerWidth/innerHeight instead of window.screen so pig movement bounds match the actual spanning window dimensions
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
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)
45-46:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClamp the initial spawn span for small viewports.
screenW - 2 * EDGE_MARGIN - PIG_SIZEcan go negative once this hook useswindow.innerWidth/innerHeighton a narrow window. That makes the first spawn position start off-screen until the next tick corrects it, so the pig can disappear on initial paint.Fix
- x: EDGE_MARGIN + Math.random() * (screenW - 2 * EDGE_MARGIN - PIG_SIZE), - y: EDGE_MARGIN + Math.random() * (screenH - 2 * EDGE_MARGIN - PIG_SIZE), + x: EDGE_MARGIN + Math.random() * Math.max(0, screenW - 2 * EDGE_MARGIN - PIG_SIZE), + y: EDGE_MARGIN + Math.random() * Math.max(0, screenH - 2 * EDGE_MARGIN - PIG_SIZE),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/usePigMovement.ts` around lines 45 - 46, The initial spawn calculation in usePigMovement can produce a negative span (screenW - 2 * EDGE_MARGIN - PIG_SIZE / screenH) on narrow viewports causing off-screen spawns; fix the x and y initializers by computing a clamped span (e.g., spanX = Math.max(0, screenW - 2 * EDGE_MARGIN - PIG_SIZE) and spanY = Math.max(0, screenH - 2 * EDGE_MARGIN - PIG_SIZE)) and then use EDGE_MARGIN + Math.random() * spanX (and spanY) so the random multiplier never receives a negative range; update the initial x/y expressions where they are set in the usePigMovement hook.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src-tauri/src/app/overlay_manager.rs`:
- Around line 63-84: The current code builds a single union MonitorInfo named
spanning from enabled monitors which unintentionally includes intervening
disabled displays; change this to produce explicit per-monitor regions (e.g.,
collect a Vec<MonitorInfo> called enabled_monitors from
enabled.iter().filter(...).map(|m| MonitorInfo { name: ..., size: ..., position:
... })) and use that list for rendering and hit-testing instead of the single
spanning rectangle, or alternatively keep spanning but also build a region mask
(Vec<PhysicalRect> or similar) of enabled monitor rectangles and apply that mask
wherever overlay interactivity or rendering occurs; update any code paths that
currently consume spanning to iterate these enabled monitor regions (references:
the enabled variable, MonitorInfo, spanning, and DisplayConfig.enabled_indices).
---
Outside diff comments:
In `@src/hooks/usePigMovement.ts`:
- Around line 45-46: The initial spawn calculation in usePigMovement can produce
a negative span (screenW - 2 * EDGE_MARGIN - PIG_SIZE / screenH) on narrow
viewports causing off-screen spawns; fix the x and y initializers by computing a
clamped span (e.g., spanX = Math.max(0, screenW - 2 * EDGE_MARGIN - PIG_SIZE)
and spanY = Math.max(0, screenH - 2 * EDGE_MARGIN - PIG_SIZE)) and then use
EDGE_MARGIN + Math.random() * spanX (and spanY) so the random multiplier never
receives a negative range; update the initial x/y expressions where they are set
in the usePigMovement hook.
🪄 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: 949bc8bf-78ba-4aa6-b9a9-678ec7b7c570
📒 Files selected for processing (2)
src-tauri/src/app/overlay_manager.rssrc/hooks/usePigMovement.ts
Comment on lines
+63
to
+84
| // Union bounding box of all enabled monitors in physical pixels. | ||
| let min_x = enabled.iter().map(|m| m.position.x).min().unwrap_or(0); | ||
| let min_y = enabled.iter().map(|m| m.position.y).min().unwrap_or(0); | ||
| let max_x = enabled | ||
| .iter() | ||
| .map(|m| m.position.x + m.size.width as i32) | ||
| .max() | ||
| .unwrap_or(1920); | ||
| let max_y = enabled | ||
| .iter() | ||
| .map(|m| m.position.y + m.size.height as i32) | ||
| .max() | ||
| .unwrap_or(1080); | ||
|
|
||
| let spanning = MonitorInfo { | ||
| name: None, | ||
| size: tauri::PhysicalSize::new( | ||
| (max_x - min_x).max(1) as u32, | ||
| (max_y - min_y).max(1) as u32, | ||
| ), | ||
| position: tauri::PhysicalPosition::new(min_x, min_y), | ||
| }; |
There was a problem hiding this comment.
Single bounding rectangle can unintentionally include disabled displays
At Line 63 through Line 84, using one min/max union rectangle means non-contiguous monitor selections (e.g., enabled indices [0, 2]) still cover index 1 if it sits between them geometrically. That weakens the meaning of per-display toggles in DisplayConfig.enabled_indices and can render pigs on displays the user explicitly disabled.
Please either keep per-monitor overlay regions for discontiguous selections, or add explicit region masking so only enabled monitor rectangles are interactive/renderable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/app/overlay_manager.rs` around lines 63 - 84, The current code
builds a single union MonitorInfo named spanning from enabled monitors which
unintentionally includes intervening disabled displays; change this to produce
explicit per-monitor regions (e.g., collect a Vec<MonitorInfo> called
enabled_monitors from enabled.iter().filter(...).map(|m| MonitorInfo { name:
..., size: ..., position: ... })) and use that list for rendering and
hit-testing instead of the single spanning rectangle, or alternatively keep
spanning but also build a region mask (Vec<PhysicalRect> or similar) of enabled
monitor rectangles and apply that mask wherever overlay interactivity or
rendering occurs; update any code paths that currently consume spanning to
iterate these enabled monitor regions (references: the enabled variable,
MonitorInfo, spanning, and DisplayConfig.enabled_indices).
- Clicked pig freezes (position + frame) while PigDetail is open; resumes when card is dismissed. selectedId flows via ref so the rAF loop sees it without restarting. - Pigs no longer escape screen: reflect velocity at hard bounds instead of only clamping position. Clamped-but-outward velocity was the escape path when a random turn fired at the boundary.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/App.tsx`:
- Around line 19-20: The app currently mixes viewport sources: usePigMovement
clamps pigs to window.innerWidth/innerHeight while PigDetail is fed
screenW/screenH from useViewport (window.screen.*), causing misaligned
positioning; refactor so both use the same viewport values by creating or
reusing a single helper hook (e.g., useViewport or useWindowSize) and pass its
innerWidth/innerHeight output into usePigMovement and into the PigDetail props
(update the pigs creation call and the PigDetail render to consume the same
screenW/screenH), or change useViewport to return window.innerWidth/innerHeight
instead of window.screen values so usePigMovement and PigDetail share identical
measurements.
🪄 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: 42673ed6-88bf-4e07-953d-06d4258e0701
📒 Files selected for processing (2)
src/components/App.tsxsrc/hooks/usePigMovement.ts
Comment on lines
+19
to
+20
| const pigs = usePigMovement(focuses, selectedId); | ||
| const { screenW, screenH } = useViewport(); |
There was a problem hiding this comment.
Use one viewport source for both pig bounds and PigDetail.
usePigMovement now clamps pigs against window.innerWidth/innerHeight, but useViewport() still feeds PigDetail with window.screen.width/height. Those values diverge in normal windowed desktop usage, so the detail card can be positioned outside the actual play area or drift away from a pig near the edges. Please switch both paths to the same viewport measurement, ideally via a shared helper/hook.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/App.tsx` around lines 19 - 20, The app currently mixes
viewport sources: usePigMovement clamps pigs to window.innerWidth/innerHeight
while PigDetail is fed screenW/screenH from useViewport (window.screen.*),
causing misaligned positioning; refactor so both use the same viewport values by
creating or reusing a single helper hook (e.g., useViewport or useWindowSize)
and pass its innerWidth/innerHeight output into usePigMovement and into the
PigDetail props (update the pigs creation call and the PigDetail render to
consume the same screenW/screenH), or change useViewport to return
window.innerWidth/innerHeight instead of window.screen values so usePigMovement
and PigDetail share identical measurements.
- Use CheckMenuItemBuilder for display toggles (native OS checkmark vs text prefix) - Display items render flat for 1-3 monitors, nested submenu for 4+ - Section moves below focus list, above Quit - usePigMovement: biome-formatted boundary blocks
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/usePigMovement.ts`:
- Around line 62-65: When frozen is true in usePigMovement, don’t just return
the original pig object; also freeze its timers by advancing nextTurnAt and
lastFrameAt by the elapsed time so they don’t “age” while selected. Concretely,
compute elapsed = now - currentTimestamp (use the same time reference used
elsewhere in usePigMovement, e.g. performance.now() or Date.now()), and return a
pig copy with nextTurnAt: pig.nextTurnAt + elapsed and lastFrameAt:
pig.lastFrameAt + elapsed (preserving other fields). This ensures nextTurnAt and
lastFrameAt remain relative and won’t trigger an immediate turn/frame when
unpaused.
🪄 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: 40a0f37c-6d24-4803-acf4-1a4c0e97fd34
📒 Files selected for processing (2)
src-tauri/src/app/tray.rssrc/hooks/usePigMovement.ts
overlay-0 was pre-defined in tauri.conf.json at 400x600. Tauri creates that window before setup runs, so the webview loaded at 400px wide — pigs initialized in a 400px space then became invisible after the spanning resize. Remove the static entry; overlay_manager always creates the window fresh via WebviewWindowBuilder with set_size(PhysicalSize) called synchronously on the main thread before show(), so window.innerWidth in React reflects the actual spanning dimensions from the first frame.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src-tauri/src/app/overlay_manager.rs (1)
63-84:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftSpanning union still includes disabled displays for non-contiguous monitor selections.
At Line 63 through Line 84, computing a single min/max union rectangle can cover disabled monitors that sit between enabled ones (for example, enabled indices
[0, 2]). That breaks the per-display toggle contract and can render pigs on displays the user explicitly disabled.Please switch to explicit enabled-monitor regions (or keep a spanning window but enforce a strict region mask for rendering + hit-testing).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/app/overlay_manager.rs` around lines 63 - 84, The current spanning MonitorInfo (variable spanning) built from min_x/min_y/max_x/max_y can include disabled displays between enabled ones (e.g., enabled indices [0,2]); replace the single-union approach with explicit enabled-monitor regions or add a strict region mask: build a Vec of per-enabled MonitorInfo instances from enabled.iter() (use the same .position/.size fields) and use that Vec for rendering and hit-testing instead of the spanning variable, or if you must keep spanning, compute enabled_rects and apply that mask in all rendering/hit-testing paths so pixels on disabled monitors are never drawn or receive events.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src-tauri/src/app/overlay_manager.rs`:
- Around line 63-84: The current spanning MonitorInfo (variable spanning) built
from min_x/min_y/max_x/max_y can include disabled displays between enabled ones
(e.g., enabled indices [0,2]); replace the single-union approach with explicit
enabled-monitor regions or add a strict region mask: build a Vec of per-enabled
MonitorInfo instances from enabled.iter() (use the same .position/.size fields)
and use that Vec for rendering and hit-testing instead of the spanning variable,
or if you must keep spanning, compute enabled_rects and apply that mask in all
rendering/hit-testing paths so pixels on disabled monitors are never drawn or
receive events.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b23ef7c6-1311-4ece-ab3f-e720cd7b6386
📒 Files selected for processing (2)
src-tauri/src/app/overlay_manager.rssrc-tauri/tauri.conf.json
💤 Files with no reviewable changes (1)
- src-tauri/tauri.conf.json
…apability Click-through issue: polling thread sets ignore_cursor_events=true whenever cursor isn't on a pig rect. PigDetail backdrop and ESC keystrokes were going to the desktop instead of the overlay. Fix: when selectedId is set, syncRects sends a full-viewport sentinel rect so the thread always sees a hit and never enables click-through. Capability fix: add overlay-0 explicitly alongside overlay-* glob so event listen and invoke reliably work for the programmatically created window, ensuring focuses-changed events reach React and the pig/detail close on delete.
- usePigMovement: clamp spawn span with Math.max(0,...) so narrow viewports don't produce negative ranges and off-screen initial positions - usePigMovement: advance nextTurnAt + lastFrameAt by dt while pig is frozen so timers don't expire during the pause, preventing immediate turn/frame jump on unfreeze - useViewport: use innerWidth/innerHeight (same source as pig movement) so PigDetail positioning and pig bounds share identical viewport measurements
delete_focus bug: store.create_focus names directory by slug but writes uuid to frontmatter id field. list() was returning focus.id = uuid, so delete_focus(uuid) tried to rmdir ~/.adhd-ranch/focuses/<uuid>/ which doesn't exist → NotFound, silently swallowed. Fix: list() overrides focus.id with the directory entry name (slug) — the authoritative key for all store path operations. Bounds: switch window.innerWidth → document.documentElement.clientWidth (same source used by browsers for layout viewport; avoids potential physical-pixel return from innerWidth on multi-DPI spanning windows).
This was referenced May 3, 2026
This was referenced May 14, 2026
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
FRAME_INTERVAL; bob animation preservedSprite sheet layout
Display: 48×48px per frame (
PIG_SIZE). Sheet scaled to 192×192 viabackground-size(normalises the 921×910 source to clean integer offsets).image-rendering: pixelatedkeeps crisp edges.Test plan
task checkgreenCloses #9
Summary by CodeRabbit
New Features
Bug Fixes
UI
Style