player moves with line

[gurkvatten]

Summary by CodeRabbit

• New Features

  • Improved player visualization on the map with enhanced interpolated positioning, providing smoother and more accurate representation of player movement and locations during gameplay.

• Removed Features

  • Removed the mock journey feature that was previously available for testing and development purposes.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes add interpolated player positioning to MapVisualizer via a new drawPlayersInterpolated method, and update TravelGameController to compute and render players at interpolated grid coordinates derived from journey history data instead of static positions. Mock journey logic and helper methods were removed.

Changes

Cohort / File(s)	Summary
Player Interpolation src/main/java/org/example/MapVisualizer.java	Added drawPlayersInterpolated(List<double[]> players, int currentIndex, double totalWidth, double totalHeight) method to render players with interpolated positioning, mirroring existing drawPlayers logic with halo and token rendering.
Controller Integration & Position Calculation src/main/java/org/example/TravelGameController.java	Switched player drawing from drawPlayers to drawPlayersInterpolated; added computePlayerGridPos(Traveler t) to calculate interpolated grid coordinates from journey history with bounds clamping; removed startMockJourney() and distinctLocationsById() helper; refactored getLocationByName() to query with ordering and handle empty results by throwing.

Sequence Diagram

sequenceDiagram
    participant Controller as TravelGameController
    participant Journey as Journey History
    participant LocationLink as LocationLink Data
    participant Calc as Position Calculation
    participant Visualizer as MapVisualizer
    participant UI as Map UI

    Controller->>Journey: Retrieve player journey data
    Controller->>LocationLink: Get current & next location
    Controller->>Calc: computePlayerGridPos(Traveler)
    Calc->>Calc: Extract current & next coordinates
    Calc->>Calc: Calculate progress along journey
    Calc->>Calc: Interpolate position & apply bounds clamping
    Calc-->>Controller: Return interpolated double[] coordinates
    Controller->>Visualizer: drawPlayersInterpolated(interpolated positions)
    Visualizer->>Visualizer: Compute cell dimensions
    Visualizer->>Visualizer: Render token at interpolated position
    Visualizer->>Visualizer: Draw halo for current player
    Visualizer-->>UI: Update map display

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• PR #21: Modifies TravelGameController's player positioning logic and map-rendering code to change how player positions are gathered and drawn.
• PR #23: Updates both MapVisualizer's player-rendering visual behavior and TravelGameController's journey-related logic, including the removal of startMockJourney and refactored player position handling.

Poem

🐰 A rabbit hops through journey paths,
Where interpolated positions trace their wrath,
From start to goal, smooth motion flows,
Mock journeys fade, true travel shows!
✨ twitch nose ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'player moves with line' is vague and does not clearly describe the main changes in the changeset, which involve adding interpolated player positioning and animation logic.	Consider a more descriptive title such as 'Add interpolated player movement visualization' or 'Implement animated player positioning with interpolation' to better reflect the actual changes.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

🧹 Recent nitpick comments

src/main/java/org/example/MapVisualizer.java (1)

103-141: Consider extracting shared rendering logic to reduce duplication.

This method is nearly identical to drawPlayers (lines 63-101), differing only in the coordinate type (double[] vs int[]). The rendering logic for halo, token styling, and layer management is duplicated.

♻️ Suggested refactor to unify both methods

Extract the shared logic into a private helper that accepts double coordinates:

+    private void drawPlayerToken(double gridX, double gridY, int playerIndex, int currentIndex, double cellWidth, double cellHeight, double totalHeight) {
+        double centerX = (gridX * cellWidth) + (cellWidth / 2);
+        double centerY = totalHeight - (gridY * cellHeight) - (cellHeight / 2);
+        double rPlayer = (cellWidth / 2) * 0.8;
+
+        if (playerIndex == currentIndex) {
+            Color c = colorForPlayer(playerIndex);
+            Circle halo = new Circle(centerX, centerY, rPlayer * 1.25);
+            halo.setFill(c.deriveColor(0, 1, 1, 0.25));
+            halo.setMouseTransparent(true);
+            playerLayer.getChildren().add(halo);
+        }
+
+        Circle token = new Circle(centerX, centerY, rPlayer);
+        token.setFill(colorForPlayer(playerIndex));
+        token.setStroke(playerIndex == currentIndex ? Color.WHITE : Color.BLACK);
+        token.setStrokeWidth(playerIndex == currentIndex ? 3 : 1.5);
+        token.setMouseTransparent(true);
+        playerLayer.getChildren().add(token);
+    }

Then both public methods can delegate to this helper, with drawPlayers casting int to double.

src/main/java/org/example/TravelGameController.java (1)

413-420: Consider clamping fracAfter to [0, 1] for defensive interpolation.

If getRemainingDistance() ever returns a negative value (e.g., due to a bug or data inconsistency), fracAfter could exceed 1.0, causing the player position to extrapolate beyond the destination. While lines 419-420 clamp the final grid coordinates, clamping the fraction itself provides clearer semantics.

♻️ Suggested fix

         int remainingAfter = j.getRemainingDistance();
         double fracAfter = (double) (total - remainingAfter) / total;
+        fracAfter = Math.max(0.0, Math.min(1.0, fracAfter));

         double gx = from.getX() + (to.getX() - from.getX()) * fracAfter;
         double gy = from.getY() + (to.getY() - from.getY()) * fracAfter;
-
-        gx = Math.max(0.0, Math.min(GRID_SIZE - 1, gx));
-        gy = Math.max(0.0, Math.min(GRID_SIZE - 1, gy));

You can keep the grid clamping as an additional safeguard if from/to coordinates could be out of bounds.

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4f84037 and 8e2b1f5.

📒 Files selected for processing (2)

• src/main/java/org/example/MapVisualizer.java
• src/main/java/org/example/TravelGameController.java

🔇 Additional comments (2)

src/main/java/org/example/TravelGameController.java (2)

373-380: LGTM!

The logic correctly handles cases where the entity manager is unavailable or the entity cannot be found, with appropriate null checks and fallback behavior.

---

559-565: LGTM!

Good refactor. Using a list query with setMaxResults(1) avoids the NoResultException from getSingleResult(), and the explicit ORDER BY l.id ensures deterministic results when duplicate names exist.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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