[stale] Improve off-screen waypoint visibility and clarity #416

zardoy · 2025-08-15T22:23:18Z

Enhance waypoint visibility by adding directional arrows for off-screen waypoints and improving text clarity with higher resolution and larger fonts.

Summary by CodeRabbit

New Features
- Offscreen waypoint arrows now display when a waypoint is outside the view, guiding you toward targets.
- Added an option to hide offscreen waypoint arrows.
Improvements
- Sharper waypoint visuals and text via higher-resolution rendering.
- Waypoint arrows are enabled by default.
- Smoother visibility transitions between on-screen and offscreen indicators for clearer navigation.
Chores
- Improved resource cleanup for waypoint visuals to maintain performance.

Co-authored-by: vital2580 <[email protected]>

cursor · 2025-08-15T22:23:19Z

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
_{Learn more about Cursor Agents}

coderabbitai · 2025-08-15T22:26:42Z

Walkthrough

Introduces an offscreen waypoint arrow sprite with configurable visibility, positioning, and texture updates. Expands rendering configuration (canvas size/scale, layout). Updates drawing pipeline to support scaled canvases and optional contexts. Ensures both on-screen and offscreen sprites synchronize color, label, and distance, and disposes related resources.

Changes

Cohort / File(s)	Summary
Waypoint sprite rendering & offscreen overlay `renderer/viewer/three/waypointSprite.ts`	Adds offscreen arrow sprite with creation/management via ensureOffscreenSprite; extends visibility logic with ARROW.hideOffscreenSprite; propagates updates (color/label/distance) to offscreen texture; positions/scales offscreen sprite toward screen center; increases CANVAS_SIZE 256→512 and CANVAS_SCALE 2→3; adjusts LAYOUT.DISTANCE_Y 0.55→0.62; enables ARROW.enabledDefault true; drawCombinedCanvas now accepts optional ctx/size for high-res rendering; dispose cleans offscreen resources.
Waypoints usage note `renderer/viewer/three/waypoints.ts`	Adds a clarifying inline comment about always enabling offscreen arrows; no functional change.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App
  participant Waypoints as Waypoints
  participant Sprite as WaypointSprite
  participant Scene

  App->>Waypoints: addWaypoint(data)
  Waypoints->>Sprite: new WaypointSprite(data)
  Waypoints->>Sprite: enableOffscreenArrow(true)
  Sprite->>Sprite: ensureOffscreenSprite() [lazy create]
  Sprite->>Scene: attach main sprite
  Sprite->>Scene: attach offscreen sprite

  rect rgba(200,240,255,0.3)
  note over Sprite: On update (position/color/label/distance)
  Sprite->>Sprite: drawCombinedCanvas(ctx?, size?)<br/>update textures (main + offscreen)
  end

  alt Waypoint off-screen
    Sprite->>Sprite: compute offset toward screen center
    Sprite->>Sprite: show offscreen sprite
    Sprite->>Sprite: toggle main visibility per hideOffscreenSprite
  else Waypoint on-screen
    Sprite->>Sprite: show main sprite
    Sprite->>Sprite: hide offscreen sprite
  end

  App-->>Sprite: dispose()
  Sprite->>Scene: detach sprites (main + offscreen)
  Sprite->>Sprite: release textures/resources

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

I hop between the screen and sky,
An arrow twin to guide your eye.
One stays near, one drifts afar—
Both painted bright on higher char.
With bigger canvases we glide,
Offscreen, onscreen—side by side. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly captures the PR's primary intent—improving off-screen waypoint visibility and label clarity—which matches the changes adding offscreen directional arrows and higher-resolution label rendering; it is concise and relevant to the changeset. The only issue is the extraneous "[stale]" prefix which may confuse reviewers about PR status.

✨ Finishing touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch cursor/improve-off-screen-waypoint-visibility-and-clarity-3e0e

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (7)

renderer/viewer/three/graphicsBackend.ts (1)
56-110: Optional: Type augment the Window interface for the new helpers

If you want stronger typing (and to avoid future TS complaints about unknown window.* props), add a global declaration:
declare global {
  interface Window {
    testWaypoints: () => void
    addTestWaypoint: (id: string, x: number, y: number, z: number, color?: number, label?: string) => void
    removeWaypoint: (id: string) => void
    listWaypoints: () => void
    world?: WorldRendererThree
  }
}
waypoint-test.html (1)

1-145: Test page reads well; no blocking issues

Content is clear and aligns with the PR’s goals. Consider linking to docs or including a short “when these commands are available” note (i.e., only after window.world is set) to reduce confusion, but it’s already implied in the “fully loaded” note.

If this is meant to live in the repo long-term, think about where it’s hosted/served and whether it should be excluded from production builds.
renderer/viewer/three/waypointSprite.ts (4)
126-174: Avoid baking the arrow into the same texture if you plan to rotate it

Baking the arrow overlay and the text into a single texture forces you to rotate both together. Consider either:

Two-sprite approach: unrotated base sprite + small arrow-only sprite on top, rotated per frame, or

Dynamic canvas redraw per frame: redraw base (once or cached) and overlay the arrow with ctx.rotate(angle); update the texture only if the angle meaningfully changed.

This will keep labels readable off-screen and opens the door for animating/skinable arrows later.

345-345: Comment mismatch: 5% radius, not ~12%

The code uses 5% of canvas height. Update the comment or adjust the constant.
-  const radius = Math.round(sizeCanvas * 0.05) // Dot takes up ~12% of canvas height
+  const radius = Math.round(sizeCanvas * 0.05) // Dot takes up ~5% of canvas height
18-21: Unused config field: ARROW.pixelSize

ARROW.pixelSize appears unused after switching to TARGET_SCREEN_PX-based sizing. Consider removing it to avoid confusion.

128-166: Fix ESLint: trailing spaces

Strip trailing spaces on the flagged lines to satisfy @stylistic/no-trailing-spaces.
-    if (arrowSprite) return
-    
+    if (arrowSprite) return
...
-    const ctx = canvas.getContext('2d')!
-    
+    const ctx = canvas.getContext('2d')!
...
-    drawCombinedCanvas(color, currentLabel, '0m', ctx, size)
-    
+    drawCombinedCanvas(color, currentLabel, '0m', ctx, size)
...
-    const centerY = size / 2
-    
+    const centerY = size / 2
...
-    ctx.translate(centerX, centerY)
-    
+    ctx.translate(centerX, centerY)
...
-    ctx.closePath()
-    
+    ctx.closePath()
...
-    ctx.stroke()
-    
+    ctx.stroke()
...
-    ctx.restore()
-    
+    ctx.restore()
...
-    // Calculate the angle in the XZ plane (horizontal direction)
-    const horizontalAngle = Math.atan2(direction.x, direction.z)
-    
+    // Calculate the angle in the XZ plane (horizontal direction)
+    const horizontalAngle = Math.atan2(direction.x, direction.z)
Also applies to: 268-271
WAYPOINT_IMPROVEMENTS.md (1)

1-137: Minor editorial: keep “off-screen” wording consistent and tighten phrasing

You mix “offscreen” and “off-screen”. Pick one and use consistently (project seems to prefer “off-screen” in most places). Also a few sentences can be tightened, but nothing blocking.

If desired, I can submit a quick pass normalizing “off-screen” and polishing a few lines.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between cc4f705 and e4541cd.

📒 Files selected for processing (5)

WAYPOINT_IMPROVEMENTS.md (1 hunks)
renderer/viewer/three/graphicsBackend.ts (1 hunks)
renderer/viewer/three/waypointSprite.ts (4 hunks)
renderer/viewer/three/waypoints.ts (1 hunks)
waypoint-test.html (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

renderer/**/*.ts

📄 CodeRabbit Inference Engine (.cursor/rules/vars-usage.mdc)

renderer/**/*.ts: Do not use the global variable bot directly in any file under the renderer/ directory or its subfolders (e.g., renderer/viewer/three/worldrendererThree.ts).
In renderer code, all bot/player state and events must be accessed via explicit interfaces, state managers, or passed-in objects, never by referencing bot directly.
In renderer code (such as in WorldRendererThree), use the playerState property (e.g., worldRenderer.playerState.gameMode) to access player state. The implementation for playerState lives in src/mineflayer/playerState.ts.

Files:

renderer/viewer/three/waypoints.ts
renderer/viewer/three/graphicsBackend.ts
renderer/viewer/three/waypointSprite.ts

🧬 Code Graph Analysis (2)

renderer/viewer/three/graphicsBackend.ts (2)

src/react/RendererDebugMenu.tsx (1)

worldRenderer (10-13)

renderer/viewer/three/primitives.js (3)

color (8-8)

color (21-21)

color (37-37)

renderer/viewer/three/waypointSprite.ts (1)

renderer/viewer/three/primitives.js (1)

THREE (3-3)

🪛 GitHub Check: build-and-deploy

renderer/viewer/three/graphicsBackend.ts

[failure] 90-90:
Cannot find name 'worldRenderer'.

[failure] 89-89:
Cannot find name 'worldRenderer'.

[failure] 81-81:
Cannot find name 'worldRenderer'.

[failure] 80-80:
Cannot find name 'worldRenderer'.

[failure] 69-69:
Cannot find name 'worldRenderer'.

[failure] 68-68:
Cannot find name 'worldRenderer'.

[failure] 60-60:
Cannot find name 'worldRenderer'.

[failure] 59-59:
Cannot find name 'worldRenderer'.

🪛 ESLint

renderer/viewer/three/graphicsBackend.ts

[error] 66-66: Trailing spaces not allowed.