Skip to content

Keep coupled cars on their own rail height - #39

Merged
ryanbarlow97 merged 1 commit into
mainfrom
fix/coupled-cars-rail-height
Sep 27, 2026
Merged

ryanbarlow97 merged 1 commit into
mainfrom
fix/coupled-cars-rail-height

Conversation

@ryanbarlow97

@ryanbarlow97 ryanbarlow97 commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Since #36, Connector.coupledPose placed each car so its front coupler met the coupler ahead exactly, in three dimensions. Where the grade changes between two cars, their couplers sit at different heights. So a car behind a locomotive on a slope gets pulled up to about a block off its rail.

In the lab on a copy of the Main line (around s 342–350, just past x 5450):

  • Locomotive: 11° nose down, sitting correctly 0.51 above its rail.
  • Passenger car and coal car: on the level just behind it, but 0.44 below their rail, about a block lower than they should be. The wheels were buried in the ballast.

The probe showed the coupling maths itself is right. The predicted coupler position matches the drawn one horizontally, give or take the fixed 0.51 offset both sides use. The problem is only that it forces the heights to match.

Change

coupledPose now meets the coupler ahead seen from above and keeps the car's rail height (nominal.y). The bend fix from #36, meeting horizontally on curves, is unchanged.

Tests

  • ConnectorTest.passengerFrontMeetsLocomotiveBackWithRealPivotsAndBaseRotation: now checks the couplers meet horizontally and the car keeps its rail height.
  • New carOnLevelTrackBehindLocomotiveOnGradeStaysOnItsRail: the lab case. Without the fix the car is placed at y 431.046, where the lab's buried car sat (431.556 − 0.51). With it, the car stays at 432.0.
  • mvn verify: 532 tests pass.

This is live in 2.1.1 and 2.2.0 on Dev and Main.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Train cars now align their couplers horizontally while retaining their nominal rail height, rather than adjusting their height to match the target coupler.
    • Coupling behind a pitched locomotive now preserves the passenger car’s nominal height and keeps its horizontal position close to the expected location.

Since #36, coupledPose placed each car so its front coupler met the car ahead
in all three dimensions. Where the grade changes between two cars, their
couplers sit at different heights, so a car behind a locomotive on a slope was
pulled up to a block below its rail, burying its wheels in the ballast. Meet
the coupler ahead seen from above only, and keep the car's rail height.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 4faa2bbe-83ce-4a73-aaba-ffcbd2236964

📥 Commits

Reviewing files that changed from the base of the PR and between d1aea02 and b25cf52.

📒 Files selected for processing (2)
  • src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/train/Connector.java
  • src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/train/ConnectorTest.java

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

coupledPose now preserves the nominal car height when aligning couplers. Tests check horizontal alignment and cover a passenger car coupled behind a pitched locomotive.

Changes

Train Coupling

Layer / File(s) Summary
Coupled pose height and alignment
src/main/java/net/tfminecraft/vehicleframework/vehicles/handlers/train/Connector.java, src/test/java/net/tfminecraft/vehicleframework/vehicles/handlers/train/ConnectorTest.java
coupledPose uses the nominal car’s Y coordinate while deriving yaw and coupler offset from the horizontal target direction. Tests check horizontal alignment, preserved height, and a passenger car coupled behind a locomotive pitched 11.1 degrees nose-down.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to b25cf

Cars retain their sampled rail height while coupling horizontally across grade changes. The inspected placement behavior and tests support this intended change, with no actionable merge risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preserving coupled cars' nominal rail height.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

A rabbit checks the couplers in a row
The car keeps the height it had below
The x and z align just right
A pitched locomotive joins the sight
The rabbit hops along the track
With steady rails beneath each car’s back

Comment @coderabbitai help to get the list of available commands.

@ryanbarlow97
ryanbarlow97 merged commit dde4657 into main Sep 27, 2026
2 checks passed
@ryanbarlow97
ryanbarlow97 deleted the fix/coupled-cars-rail-height branch September 27, 2026 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant