Use bedsim submodule for movement simulation - #153
Conversation
Replace the duplicated anticheat movement physics with oomph-ac/bedsim via a git submodule and go.mod replace, keeping Oomph's liquid exemption until swimming/liquid-layer state is wired through the adapter. Co-authored-by: Hashim <hashim@lunarbedrock.com>
|
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:
📝 WalkthroughWalkthroughThe PR vendors bedsim, migrates anticheat geometry to Dragonfly BBox32 types, adds shared raycast utilities, introduces liquid and world-layer adapters, delegates player movement simulation to bedsim, and removes superseded movement helpers. ChangesBedsim movement and geometry migration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant PlayerMovement
participant BedsimAdapter
participant BedsimSimulator
participant WorldProvider
PlayerMovement->>BedsimAdapter: convert movement and player state
BedsimAdapter->>BedsimSimulator: run SimulateState
BedsimSimulator->>WorldProvider: query blocks, liquids, and collisions
WorldProvider-->>BedsimSimulator: return world data
BedsimSimulator-->>BedsimAdapter: return state and outcome
BedsimAdapter-->>PlayerMovement: apply simulated state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Drop collision/math/constants that only existed for the inlined movement sim, fix a pre-existing gofmt lint failure, and align transferproxy's golangci config with anticheat so CI can pass. Co-authored-by: Hashim <hashim@lunarbedrock.com>
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@anticheat/player/simulation/bedsim_adapter.go`:
- Line 202: Update the coordinate conversion at the visible pos32 initialization
and the related helper conversions below to explicitly cast each df_cube.Pos
integer coordinate to float32 before constructing float_cube.Pos, ensuring all
positional assignments type-check.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c8dc591e-0b6f-4d04-92d6-859478e61271
📒 Files selected for processing (2)
anticheat/player/simulation/bedsim_adapter.gobedsim
🚧 Files skipped from review as they are similar to previous changes (1)
- bedsim
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)
anticheat/world/world.go (1)
86-112: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
World.Blocklikely passes world coordinates where chunk-local coordinates are required.
c.Block(uint8(pos[0]), int16(pos[1]), uint8(pos[2]), 0)truncates the full world x/z coordinate to a byte instead of masking to the chunk-local range. Chunk-internal block storage (Dragonfly/Minecraft convention) expects local coordinates in[0, 15]relative to the chunk (worldX & 15), not the raw world coordinate cast touint8. As soon aspos[0]/pos[2]fall outside[0, 255](i.e., almost anywhere in the world) or go negative, this returns the wrong block (or wraps unpredictably), sinceuint8(pos[0])!=pos[0] & 15. The line-range summary itself notes this replaced a previously "converted position (blockPos-derived)" — consistent with local-coordinate masking having been dropped in this migration.Since
World.Blockbacks virtually every collision/interaction/anti-cheat check (BlockCollisions,PlaceBlock,ValidateInteraction, break-time calc, water detection, and the bedsim world provider), this would corrupt block lookups across the board.🐛 Proposed fix
// TODO: Implement and account for multi-layer blocks. - rid := c.Block(uint8(pos[0]), int16(pos[1]), uint8(pos[2]), 0) + rid := c.Block(uint8(pos[0]&15), int16(pos[1]), uint8(pos[2]&15), 0)Please confirm the exact local-coordinate contract of the chunk lookup API in use here (e.g. via the
ChunkInfo/chunk.Chunktype this wraps) to validate the fix.#!/bin/bash # Locate the ChunkInfo type / w.Chunk(...) definition to inspect the Block method's coordinate contract. rg -n -A5 -B2 'type ChunkInfo' anticheat/world rg -n -A10 'func .*\) Chunk\(' anticheat/world rg -nP '\bBlock\(uint8|\bBlock\(x, y, z' anticheat/world🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@anticheat/world/world.go` around lines 86 - 112, Update World.Block’s c.Block lookup to pass chunk-local x/z coordinates by masking the world coordinates to the 0–15 range, while preserving the existing y coordinate and layer argument. Use the chunk API’s local-coordinate contract confirmed from the wrapped Chunk type, and ensure negative and large world positions resolve consistently to their containing chunk’s local block.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@anticheat/player/world.go`:
- Around line 341-343: Update the prevPos initialization alongside currPos so it
derives from the player’s previous-tick position, using the existing
LastPos()/Pos() movement pattern and retaining the same height offset; leave
currPos based on the current position so face detection compares both ticks.
---
Outside diff comments:
In `@anticheat/world/world.go`:
- Around line 86-112: Update World.Block’s c.Block lookup to pass chunk-local
x/z coordinates by masking the world coordinates to the 0–15 range, while
preserving the existing y coordinate and layer argument. Use the chunk API’s
local-coordinate contract confirmed from the wrapped Chunk type, and ensure
negative and large world positions resolve consistently to their containing
chunk’s local block.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b8afea95-8fca-4eb4-9589-ab024e98da31
⛔ Files ignored due to path filters (4)
anticheat/example/default/go.sumis excluded by!**/*.sumanticheat/example/dragonfly/go.sumis excluded by!**/*.sumanticheat/go.sumis excluded by!**/*.sumexample-proxy/go.sumis excluded by!**/*.sum
📒 Files selected for processing (22)
anticheat/entity/entity.goanticheat/example/default/go.modanticheat/example/dragonfly/go.modanticheat/game/aabb.goanticheat/game/bbox_trace.goanticheat/game/bbox_trace_test.goanticheat/game/raytrace.goanticheat/go.modanticheat/player/component/combat.goanticheat/player/component/movement.goanticheat/player/component/world.goanticheat/player/movement.goanticheat/player/simulation/bedsim_adapter.goanticheat/player/world.goanticheat/utils/collisions/collisions.goanticheat/utils/collisions/register.goanticheat/utils/unsafe.goanticheat/utils/world.goanticheat/world/convert.goanticheat/world/world.gobedsimexample-proxy/go.mod
💤 Files with no reviewable changes (2)
- anticheat/world/convert.go
- anticheat/utils/unsafe.go
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Needs in-game smoke test, other than that LGTM! |
5 participants
Summary
Wires
oomph-ac/bedsiminto the monorepo as a git submodule and replaces the duplicated movement physics inanticheat/player/simulationwith a thin bedsim adapter.Also removes the now-dead sim-only helpers/constants that only served the inlined physics (
BBClipCollide,MCSin/MCCos, bounce/step/climb constants, etc.).What this replaces
simulation/movement.goduplication Hashim called outWhat intentionally stays in Oomph
These are not a second physics engine; they are caller/policy or shared non-sim utilities:
player/component/movement.go— input parsing, sprint timing, impulse clamps, corrections, teleports/KB bookkeepingutils.BlockName/ friction / climbable — still used by combat/world interaction; adapter feeds them into bedsim viaBlockSemanticsfeat/liquid-simulationwithout its duplicated physics) and drop the exemption.Exemptions checked
bedsim’s
simulationIsReliablestill covers the same non-liquid cases as Oomph:Test plan
go vet ./...inanticheatgo test ./...inanticheatgo test ./...inbedsimmake lint(anticheat + transferproxy)Summary by CodeRabbit
Float32-native bedsim adapter
The bedsim submodule now points at the standalone float32-native conversion. The adapter passes Oomph’s existing
mgl32movement state and float32 scalars directly into bedsim. Anticheat geometry now uses Dragonfly’scube.BBox32through the temporaryoomph-ac/dragonflyreplace, removing bothfloat32-cubeand the former float64 vector and bounding-box round trips. This includes the breaking bedsim provider-interface change to Dragonfly float32 boxes.