Skip to content

feat: centralize vanilla movement block semantics - #11

Open
HashimTheArab wants to merge 3 commits into
mainfrom
liquid-simulation
Open

feat: centralize vanilla movement block semantics#11
HashimTheArab wants to merge 3 commits into
mainfrom
liquid-simulation

Conversation

@HashimTheArab

@HashimTheArab HashimTheArab commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace the legacy split BlockSemantics adapter with one complete BlockMovementSemanticsProvider contract
  • centralize ground friction, soul sand/soil friction adjustment, climbability, cobweb detection, slime/bed bounce, and bamboo reliability handling
  • add conformance tests for default and custom semantics, invalid friction fallback, and soul-soil grounded travel
  • document the intentional breaking API change and refresh module sums for the upstream Dragonfly dependency graph

Validation

  • go test -mod=readonly ./...
  • go vet -mod=readonly ./...

This PR intentionally does not preserve the old BlockName/BlockFriction/BlockClimbable provider interface.

Summary by CodeRabbit

  • New Features

    • Added unified block movement semantics for friction, climbing, cobwebs, and bounce behavior.
    • Added support for slime, beds, ladders, vines, cobwebs, soul sand, and soul soil.
    • Added customizable movement-semantics providers for special blocks.
  • Bug Fixes

    • Invalid friction values now safely fall back to defaults.
    • Bamboo no longer causes simulation reliability checks to fail.
    • Ground movement now correctly applies soul-soil friction.
  • Documentation

    • Expanded documentation covering movement semantics, customization, friction handling, and setup requirements.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR introduces consolidated block movement semantics, adds built-in handlers for special blocks, updates simulator movement logic to use semantic properties, and replaces the previous block metadata provider interface.

Changes

Movement semantics

Layer / File(s) Summary
Movement semantics and built-in owners
block/semantics.go, block/*.go
The block package defines movement properties and built-in owners for friction, soul blocks, ladders, vines, cobwebs, slime, beds, and ice.
Provider contract and resolution
interfaces.go, simulator.go, block.go, README.md, go.mod
The simulator uses BlockMovementSemanticsProvider. Default resolution combines semantic properties and validates ground friction. Legacy helpers are removed.
Simulation movement integration
simulation.go
Movement, climbing, bouncing, and cobweb handling use resolved semantics. The bamboo reliability rejection is removed.
Semantics and simulator validation
block_semantics_test.go, simulator_test.go, liquid_test.go
Tests cover built-in semantics, custom providers, invalid friction fallback, bamboo handling, and soul-soil movement.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Simulation
  participant DefaultBlockSemantics
  participant block.Resolve
  Simulation->>DefaultBlockSemantics: request BlockMovementSemantics
  DefaultBlockSemantics->>block.Resolve: resolve block movement properties
  block.Resolve-->>DefaultBlockSemantics: return MovementSemantics
  DefaultBlockSemantics-->>Simulation: provide movement properties
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. 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 PR's primary change: centralizing vanilla movement block semantics.
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.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch liquid-simulation

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.

@HashimTheArab
HashimTheArab marked this pull request as ready for review August 6, 2026 19:19

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
simulation.go (1)

1001-1005: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use BlockCollisions for cobweb contact detection.

The full-block cube at Line 1005 can apply cobweb slowdown when the player intersects an empty part of a semantic cobweb block. Resolve the block-local collision boxes with WorldProvider.BlockCollisions(pos), translate each box by posVec3(pos), and test those world-space boxes against bb.

Proposed fix
-		if bb.IntersectsWith(cube.Box32(0, 0, 0, 1, 1, 1).Translate(posVec3(pos))) {
-			insideCobweb = true
+		for _, collision := range s.World.BlockCollisions(pos) {
+			if bb.IntersectsWith(collision.Translate(posVec3(pos))) {
+				insideCobweb = true
+				break
+			}
 		}

Based on learnings, WorldProvider.BlockCollisions(pos) returns block-local boxes and requires translation before world-space intersection. As per coding guidelines, collision methods must provide consistent collision information.

🤖 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 `@simulation.go` around lines 1001 - 1005, Update the cobweb contact logic
around blockMovementSemantics and bb.IntersectsWith to use
s.WorldProvider.BlockCollisions(pos) instead of a full-block cube. Translate
each returned block-local collision box by posVec3(pos), and apply cobweb
slowdown only when bb intersects at least one translated box.

Sources: Coding guidelines, Learnings

🤖 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.

Outside diff comments:
In `@simulation.go`:
- Around line 1001-1005: Update the cobweb contact logic around
blockMovementSemantics and bb.IntersectsWith to use
s.WorldProvider.BlockCollisions(pos) instead of a full-block cube. Translate
each returned block-local collision box by posVec3(pos), and apply cobweb
slowdown only when bb intersects at least one translated box.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 61e2de1a-e556-4785-98c0-78d732da403e

📥 Commits

Reviewing files that changed from the base of the PR and between 8987ee5 and 8450578.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (15)
  • README.md
  • block.go
  • block/bounce.go
  • block/climbable.go
  • block/contact.go
  • block/friction.go
  • block/ground.go
  • block/semantics.go
  • block_semantics_test.go
  • go.mod
  • interfaces.go
  • liquid_test.go
  • simulation.go
  • simulator.go
  • simulator_test.go
💤 Files with no reviewable changes (1)
  • block.go

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