fix: correct movement parity regressions on main - #15
Conversation
Bubble columns applied one impulse per overlapped block cell. An entity carries a single resolved column contact per tick regardless of how many cells its hitbox spans, so a 1.8-block-tall player — who almost always overlaps two cells — accelerated at roughly double the real rate for every tick spent in a column. Resolve one contact from the topmost overlapped cell, since only that cell can have the open air above it that selects the surface form, and apply once. Column contact also clears fall distance, which was not modelled. Sneak edge protection ran on airborne ticks. The gate was widened from on-ground to a fall-distance leniency borrowed from Java's Player.isAboveGround; vanilla has no fall-distance input to this decision at all and requires ground strictly. A sneaking player stepping off a ledge was clamped when they should move freely. Honey wall slide required a horizontal collision and stopped after the first block. Contact with the block volume is sufficient, and each overlapped block compounds the horizontal factor, carrying the clamped vertical velocity between them. The accompanying fall-distance reset is left out: its shape is known but its thresholds could not be measured, and guessing them would trade one wrong behaviour for another. Bed restitution and its cap were changed to -0.75/0.75 with no stated source, against -0.66/1.0 from the reference docs, the Dragonfly implementation and Java. Neither value is proven here, so this restores the corroborated one and names the cap; revert if a capture says otherwise.
|
Warning Review limit reached
Next review available in: 19 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
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. Comment |
1 participant
What changed
Four movement-parity regressions that landed on
mainin #6, found by auditing the merged diff against vanilla behaviour rather than against the reference docs.Bubble columns applied one impulse per overlapped cell
An entity carries a single resolved column contact per tick, no matter how many column cells its hitbox spans. A 1.8-block-tall player almost always overlaps two, so bedsim accelerated at roughly double the real rate for every tick spent in a column — the hottest of these paths, since bubble elevators are common.
Now resolved once, from the topmost overlapped cell: only that cell can have the open air above it that selects the surface form. Column contact also clears fall distance, which was not modelled at all.
TestBubbleColumnAppliesForEachOccupiedCellasserted the cumulative behaviour and is replaced byTestBubbleColumnAppliesOnceForOverlappedCells.Sneak edge protection ran while airborne
#6 widened the gate from
state.OnGroundto anisAboveGroundfall-distance leniency, which is Java'sPlayer.isAboveGround(). Vanilla's sneak-movement path has no fall-distance input to this decision at all and requires ground strictly. A sneaking player stepping off a ledge was clamped when they should move freely.Reverted to the on-ground gate;
isAboveGroundhad no other caller and is removed.Honey wall slide over-gated and under-applied
It required
CollideX || CollideZand returned after the first block. Contact with the block volume is sufficient — no horizontal collision is needed — and each overlapped block compounds the horizontal factor while carrying the clamped vertical velocity between them.The accompanying fall-distance reset is deliberately not implemented: its shape is known (feet near the block top, descending, within a horizontal margin) but its thresholds could not be measured, and guessing them would trade one wrong behaviour for another. Follow-up.
Bed restitution reverted to the corroborated value
#6 changed
BedBounceMultiplierto-0.75and the cap to0.75with no stated source. The reference docs, the Dragonfly implementation and Java all say-0.66with a cap of1.0, and the test that asserted0.75was written alongside the change, so it is self-confirming rather than evidence.Neither value is proven here. This restores the multiply-corroborated one and names the cap as
BedBounceCap. Straightforward to revert if someone has a capture showing 0.75.Not addressed
StepHeight0.5625 vs 0.6, the1e-8velocity epsilon, and the pose heights (sneak 1.49, crawl 0.6) all remain unverified. They want an on-device capture, not a code change.TestBlockNameCachesRawHashPairandTestBlockNameCachesMaxStateWithKnownBasefail under repeated runs (-count=2) becauseblockNameCacheis a global that the tests never reset. Pre-existing onmainand already fixed in fix: close BedSim movement parity edge cases #14, so it is left alone here to avoid a conflict.Checks
go build ./...go vet ./...go test -count=1 ./...gofmt -l .clean,git diff --checkclean