Implement basic scaffold detections - #94
Conversation
…lient position during kb
c4752ad to
c571acf
Compare
📝 WalkthroughWalkthroughAdds ScaffoldB block-face validation, registers scaffold detections, updates closest-point and numeric helpers, and includes the held hotbar slot in interaction diagnostics. ChangesGeometry and math updates
Scaffold detection
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
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
player/detection/scaffold_b.go (1)
160-160: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove 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
📒 Files selected for processing (5)
game/aabb.gogame/math.goplayer/component/world.goplayer/detection/register.goplayer/detection/scaffold_b.go
| // 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 | ||
| } |
There was a problem hiding this comment.
🎯 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.
| // 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.
| // 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 | ||
| } |
There was a problem hiding this comment.
🎯 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}")
PYRepository: 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.
| floorEyeStart := int(startPos[1] + eyeOffset) | ||
| floorEyeEnd := int(endPos[1] + eyeOffset) |
There was a problem hiding this comment.
🎯 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/nullRepository: 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'
doneRepository: 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'
fiRepository: 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.goRepository: 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))
PYRepository: 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.goRepository: 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.
| 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 | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 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/detectionRepository: 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.
Summary by CodeRabbit
New Features
Bug Fixes
Diagnostics