[Audit][Medium] hit_ceiling boolean set on downward Y movement when passing through collision

[MichaelFisher1997]

🔍 Module Scanned\n (automated audit scan)\n\n## 📝 Summary\nIn , the flag is incorrectly set when the player's Y velocity is negative (falling/jumping down) and their downward movement is blocked by collision. This is backwards - hitting a ceiling means moving upward into an overhead block. The flag should only be set when Y velocity is positive (moving up), not when falling.\n\n## 📍 Location\n- File: \n- Function/Scope: Y-axis collision resolution (line 76-98)\n\n## 🔴 Severity: Medium\n- Medium: Performance degradation, missing error handling, suboptimal patterns\n- This is a logic bug where the flag is set incorrectly\n\n## 💥 Impact\nWhen a player falls and hits the ground, will be instead of . This causes:\n- Incorrect game logic that checks (e.g., to suppress jump or trigger animations)\n- and are mutually exclusive but both can be true at once in this edge case\n- The struct documents as "Whether the entity hit a ceiling" - this semantic is violated\n\n## 🔎 Evidence\n\n\nThe condition indicates the player is moving down (negative Y direction), yet is set in the else branch when (moving up or zero). This is backwards.\n\n\n## 🛠️ Proposed Fix\nSwap the logic so that is set when (moving up into something):\n\n\n\nNote: should always execute regardless of collision direction (it's outside the if/else in the current code, but the structure should be clarified).\n\n## ✅ Acceptance Criteria\n- [ ] is only when the player was moving upward (positive Y velocity)\n- [ ] is only when the player was moving downward (negative Y velocity)\n- [ ] A falling player who lands on ground triggers , not \n- [ ] Unit tests verify correct flags for both upward and downward collisions\n\n## 📚 References\n- - struct documentation\n- - from \n- Issue #720 (related): "CollisionConfig.max_iterations is defined but never used in binary search"\n

[github-actions]

The issue's proposed fix is incorrect. Let me clarify:

Current code at modules/engine-physics/src/collision.zig:89-93:

if (move.y < 0) {
    result.grounded = true;      // Falling, hit ground below
} else {
    result.hit_ceiling = true;   // Moving up (or stationary), hit block above
}

The audit claims this is backwards, but the logic is actually correct:

• move.y < 0 = moving DOWN (falling), so grounded = true is correct — player hit the ground
• move.y >= 0 = moving UP or zero, so hit_ceiling = true is correct — player hit something overhead

The semantic is: positive Y = up, negative Y = down (standard 3D coordinate convention used by Vec3).

The audit's proposed fix would break this — swapping the conditions would set hit_ceiling when falling and grounded when jumping, which is inverted.

Bug classification: False positive. The code correctly implements the documented semantics:

• CollisionResult.grounded: "Whether the entity is standing on solid ground" (line 31)
• CollisionResult.hit_ceiling: "Whether the entity hit a ceiling" (line 33)

The only potential improvement would be renaming move.y < 0 to move.y > 0 with a comment clarifying "moving up" for readability, but the underlying logic is sound.

opencode session  |  github run