Skip to content

Implement basic scaffold detections - #94

Open
ethaniccc wants to merge 1387 commits into
stablefrom
feat/scaffold-dtc
Open

Implement basic scaffold detections#94
ethaniccc wants to merge 1387 commits into
stablefrom
feat/scaffold-dtc

Conversation

@ethaniccc

@ethaniccc ethaniccc commented Aug 19, 2025

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • New Features

    • Added scaffold-placement validation to detect invalid block-face interactions and non-contiguous simulated placements.
    • Added inventory-movement detection to the detection system.
    • Improved detection coverage for scaffold-related interactions.
  • Bug Fixes

    • Improved closest-point and bounding-box calculations for more reliable spatial checks.
    • Added more consistent rounding and precision handling for gameplay calculations.
  • Diagnostics

    • Enhanced interaction logs to include the held hotbar slot for easier troubleshooting.

ethaniccc and others added 30 commits June 17, 2025 02:24
@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds ScaffoldB block-face validation, registers scaffold detections, updates closest-point and numeric helpers, and includes the held hotbar slot in interaction diagnostics.

Changes

Geometry and math updates

Layer / File(s) Summary
Geometry and rounding helpers
game/aabb.go, game/math.go
Closest-point handling is updated, and generic rounding plus precision-floor helpers are added.

Scaffold detection

Layer / File(s) Summary
ScaffoldB validation
player/detection/scaffold_b.go
Adds ScaffoldB metadata, packet handling, block validation, face-interaction checks, and simulation placement adjacency tracking.
Detection registration and diagnostics
player/detection/register.go, player/component/world.go
Registers ScaffoldA and ScaffoldB and adds the held slot number to item interaction debug output.

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

Sequence Diagram(s)

sequenceDiagram
  participant InventoryTransaction
  participant ScaffoldB
  participant Player
  InventoryTransaction->>ScaffoldB: block-click action
  ScaffoldB->>Player: read held block and movement state
  Player-->>ScaffoldB: item, eye position, trigger, and motion data
  ScaffoldB->>ScaffoldB: validate target face and simulation adjacency
  ScaffoldB-->>InventoryTransaction: PassDetection or FailDetection
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check ✅ Passed The title clearly matches the main change: adding basic scaffold detection logic and registration.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/scaffold-dtc

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

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

Actionable comments posted: 4

🧹 Nitpick comments (1)
player/detection/scaffold_b.go (1)

160-160: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove commented-out debug artifacts.

Lines 160 and 177 contain commented-out d.mPlayer.Message(...) / fmt.Println(...) debug lines. Drop them to keep the detection path clean.

Also applies to: 177-177

🤖 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 `@player/detection/scaffold_b.go` at line 160, Remove the commented-out debug
statements at the indicated points in the detection path, including the
d.mPlayer.Message and fmt.Println artifacts, while leaving the surrounding
detection logic unchanged.
🤖 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 `@game/math.go`:
- Around line 57-65: Update Round so negative inputs with fractional magnitude
at least 0.5 decrement the truncated base, while positive inputs retain the
existing increment behavior. Use the sign of a and the fractional component to
return the nearest integer correctly for both positive and negative values,
including values such as -1.7 returning -2.
- Around line 67-75: Update PrecisionFloor32 to derive floorVal with
math32.Floor(a) rather than int(a), preserving the existing precision-threshold
adjustment so negative and positive inputs are handled consistently.

In `@player/detection/scaffold_b.go`:
- Around line 131-148: In the simulation placement logic, save the previous
value of d.hasPrevSimBlockPos before setting it to true, then gate the
continuity check and interactable-face loop on that saved value. Keep the first
placement unconditional while preserving chain validation for subsequent
placements.
- Around line 120-121: Update the eye Y calculations in the detection logic
around floorEyeStart and floorEyeEnd to use mathematical floor semantics before
converting to block coordinates, matching cube.PosFromVec3 for negative values.
Preserve the existing isBelowBlock and isAboveBlock comparisons while ensuring
both eye positions use the floored Y values.

---

Nitpick comments:
In `@player/detection/scaffold_b.go`:
- Line 160: Remove the commented-out debug statements at the indicated points in
the detection path, including the d.mPlayer.Message and fmt.Println artifacts,
while leaving the surrounding detection logic unchanged.
🪄 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: 01b2fe32-259e-4062-812e-7e1ad075cd33

📥 Commits

Reviewing files that changed from the base of the PR and between d0cb30d and c571acf.

📒 Files selected for processing (5)
  • game/aabb.go
  • game/math.go
  • player/component/world.go
  • player/detection/register.go
  • player/detection/scaffold_b.go

Comment thread game/math.go
Comment on lines +57 to +65
// Round will round a number to the nearest integer.
func Round[T float32 | float64, V uint | int | uint8 | int8 | uint16 | int16 | uint32 | int32 | uint64 | int64](a T) V {
baseFloat := a - T(V(a))
base := V(a)
if baseFloat >= 0.5 {
base++
}
return base
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Round mis-rounds negative values.

base := V(a) truncates toward zero (Go's float→int conversion), and the code only ever increments base, never decrements. For negative a, this means values whose fractional magnitude is ≥ 0.5 round toward zero instead of away from it, e.g. Round[float32, int](-1.7) returns -1 instead of the correct -2. Since this helper is generic and likely used on world/block coordinates that can be negative, this is a real arithmetic bug, not just an edge case.

🐛 Proposed fix
 // Round will round a number to the nearest integer.
 func Round[T float32 | float64, V uint | int | uint8 | int8 | uint16 | int16 | uint32 | int32 | uint64 | int64](a T) V {
-	baseFloat := a - T(V(a))
-	base := V(a)
-	if baseFloat >= 0.5 {
-		base++
-	}
-	return base
+	if a < 0 {
+		return V(a - 0.5)
+	}
+	return V(a + 0.5)
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Round will round a number to the nearest integer.
func Round[T float32 | float64, V uint | int | uint8 | int8 | uint16 | int16 | uint32 | int32 | uint64 | int64](a T) V {
baseFloat := a - T(V(a))
base := V(a)
if baseFloat >= 0.5 {
base++
}
return base
}
// Round will round a number to the nearest integer.
func Round[T float32 | float64, V uint | int | uint8 | int8 | uint16 | int16 | uint32 | int32 | uint64 | int64](a T) V {
if a < 0 {
return V(a - 0.5)
}
return V(a + 0.5)
}
🤖 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 `@game/math.go` around lines 57 - 65, Update Round so negative inputs with
fractional magnitude at least 0.5 decrement the truncated base, while positive
inputs retain the existing increment behavior. Use the sign of a and the
fractional component to return the nearest integer correctly for both positive
and negative values, including values such as -1.7 returning -2.

Comment thread game/math.go
Comment on lines +67 to +75
// PrecisionFloor32 floors a number to the given precision.
func PrecisionFloor32(a float32, precision float32) int {
increaseValueAt := float32(1) - precision
floorVal := int(a)
if a-float32(floorVal) >= increaseValueAt {
floorVal++
}
return floorVal
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== game/math.go (imports + target function) ==\n'
cat -n game/math.go | sed -n '1,130p'

printf '\n== math32 usages in game/math.go ==\n'
rg -n 'math32|PrecisionFloor32|Round32|Round64' game/math.go

printf '\n== locate math32 dependency usage across repo ==\n'
rg -n 'math32\.Floor|import .*math32|github.com.*math32' -g '!**/vendor/**' .

Repository: oomph-ac/oomph

Length of output: 7591


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== PrecisionFloor32 call sites ==\n'
rg -n 'PrecisionFloor32\(' -g '!**/vendor/**' .

printf '\n== nearby context in game/raytrace.go ==\n'
cat -n game/raytrace.go | sed -n '70,105p'

printf '\n== behavior probe for representative inputs ==\n'
python3 - <<'PY'
import math

def precision_floor32(a, precision):
    increaseValueAt = 1 - precision
    floorVal = int(a)
    if a - float(floorVal) >= increaseValueAt:
        floorVal += 1
    return floorVal

cases = [
    (-1.99, 0.05),
    (-1.01, 0.05),
    (-1.00, 0.05),
    (-0.01, 0.05),
    ( 1.99, 0.05),
    ( 1.01, 0.05),
    ( 0.99, 0.05),
]
for a, p in cases:
    got = precision_floor32(a, p)
    ideal = math.floor(a) if p == 1 else None
    print(f"a={a:6}, p={p:4} -> {got}")
PY

Repository: oomph-ac/oomph

Length of output: 1086


PrecisionFloor32 mishandles negative inputs. Start from math32.Floor(a) instead of int(a) so the precision threshold works on both sides of zero; int(a) truncates toward zero and returns -1 for values like -1.99.

🤖 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 `@game/math.go` around lines 67 - 75, Update PrecisionFloor32 to derive
floorVal with math32.Floor(a) rather than int(a), preserving the existing
precision-threshold adjustment so negative and positive inputs are handled
consistently.

Comment on lines +120 to +121
floorEyeStart := int(startPos[1] + eyeOffset)
floorEyeEnd := int(endPos[1] + eyeOffset)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Look for a floor helper in game/math.go to use instead of int() truncation
fd -e go . game | xargs rg -n 'func (Floor|FloorNum|Round)' 
rg -nP 'func PosFromVec3' $(fd -t f -e go | head -50) 2>/dev/null

Repository: oomph-ac/oomph

Length of output: 556


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- scaffold_b.go (relevant range) ---'
sed -n '90,160p' player/detection/scaffold_b.go

echo
echo '--- PosFromVec3 references ---'
rg -n 'func PosFromVec3|PosFromVec3\(' -S .

echo
echo '--- cube package files ---'
fd -e go cube .

echo
echo '--- candidate cube source files outline ---'
for f in $(fd -e go cube .); do
  echo "### $f"
  ast-grep outline "$f" | sed -n '1,120p'
done

Repository: oomph-ac/oomph

Length of output: 3827


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- PosFromVec3 definition ---'
rg -n 'func PosFromVec3' -S .

echo
echo '--- Pos type / floor helpers ---'
rg -n 'type Pos|Floor|floor' -S game player utils . | sed -n '1,220p'

echo
echo '--- files that mention cube package path ---'
rg -n 'cube\.PosFromVec3|package cube|PosFromVec3' -S . | sed -n '1,220p'

Repository: oomph-ac/oomph

Length of output: 183


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- go env ---'
go env GOPATH GOMODCACHE 2>/dev/null || true

echo
echo '--- relevant import for cube package ---'
rg -n '"cube"|cube\.' player/detection/scaffold_b.go go.mod . | sed -n '1,160p'

echo
echo '--- search module cache for PosFromVec3 ---'
MODCACHE="$(go env GOMODCACHE 2>/dev/null || true)"
if [ -n "$MODCACHE" ] && [ -d "$MODCACHE" ]; then
  rg -n 'func PosFromVec3' "$MODCACHE" 2>/dev/null | sed -n '1,80p'
fi

Repository: oomph-ac/oomph

Length of output: 14782


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- PosFromVec3 implementation ---'
sed -n '120,155p' /home/jailuser/go/pkg/mod/github.com/ethaniccc/float32-cube@v0.0.0-20250511224129-7af1f8c4ee12/cube/pos.go

echo
echo '--- round helpers in game/math.go ---'
sed -n '1,140p' game/math.go

echo
echo '--- relevant block interaction logic around scaffold_b.go ---'
sed -n '140,210p' player/detection/scaffold_b.go

Repository: oomph-ac/oomph

Length of output: 7063


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import math
vals = [-58.4, -58.0, -0.1, 0.1, 58.4]
print("int truncation vs floor examples:")
for v in vals:
    print(v, int(v), math.floor(v))
PY

Repository: oomph-ac/oomph

Length of output: 243


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- PosFromVec3 implementation ---'
sed -n '120,155p' /home/jailuser/go/pkg/mod/github.com/ethaniccc/float32-cube@v0.0.0-20250511224129-7af1f8c4ee12/cube/pos.go

echo
echo '--- game/math.go ---'
sed -n '1,140p' game/math.go

echo
echo '--- scaffold_b.go Y-face logic ---'
sed -n '140,210p' player/detection/scaffold_b.go

Repository: oomph-ac/oomph

Length of output: 7019


Use floor for the eye Y comparison. int(startPos[1] + eyeOffset) truncates toward zero, while cube.PosFromVec3 floors; below y=0 this can make isBelowBlock / isAboveBlock disagree with the block-position checks.

🤖 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 `@player/detection/scaffold_b.go` around lines 120 - 121, Update the eye Y
calculations in the detection logic around floorEyeStart and floorEyeEnd to use
mathematical floor semantics before converting to block coordinates, matching
cube.PosFromVec3 for negative values. Preserve the existing isBelowBlock and
isAboveBlock comparisons while ensuring both eye positions use the floored Y
values.

Comment on lines +131 to +148
prevPos := d.prevSimBlockPos
d.prevSimBlockPos = blockPos
d.hasPrevSimBlockPos = true

if d.hasPrevSimBlockPos {
found := false
for iFace := range interactableFaces {
if prevPos.Side(iFace) == blockPos {
found = true
break
}
}
// Simulation placements must be in a chain and not seperated from each other.
if !found {
d.mPlayer.Log().Debug("scaffold_b (invalid sim placement)", "blockPos", blockPos, "prevSimBlockPos", prevPos, "interactableFaces", interactableFaces)
return false
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the file and inspect relevant sections.
ast-grep outline player/detection/scaffold_b.go --view expanded || true

echo '--- scaffold_b.go (around lines 1-220) ---'
cat -n player/detection/scaffold_b.go | sed -n '1,220p'

echo '--- search for relevant symbols/usages ---'
rg -n "hasPrevSimBlockPos|prevSimBlockPos|TriggerTypePlayerInput|FailDetection|scaffold_b" player/detection

Repository: oomph-ac/oomph

Length of output: 12292


Fix the simulation-chain guard in player/detection/scaffold_b.go:131-148
hasPrevSimBlockPos is set to true before this check, so the chain-continuity test runs on the first simulation placement too and can false-fail against the stale prevSimBlockPos. Save the old flag first, then gate the check on that value.

🤖 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 `@player/detection/scaffold_b.go` around lines 131 - 148, In the simulation
placement logic, save the previous value of d.hasPrevSimBlockPos before setting
it to true, then gate the continuity check and interactable-face loop on that
saved value. Keep the first placement unconditional while preserving chain
validation for subsequent placements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants