-
Notifications
You must be signed in to change notification settings - Fork 17
Review physics module code quality #1223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: devel
Are you sure you want to change the base?
Review physics module code quality #1223
Conversation
Documents bugs, missing features, and improvements found during code review of shards/modules/physics: P0 (Critical): - Memory leaks in body/constraint cleanup - ApplyForceAt logic bug (&&/|| operator) - Typo in assertion macro P1 (Missing Features): - No raycasting/shape queries - Missing shape types (cylinder, mesh, heightfield) - Missing constraint types (hinge, cone, etc) - Body sleeping disabled P2 (Improvements): - Soft body param detection broken - Hardcoded system limits - Various minor issues
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR documents findings from a comprehensive code review of the shards/modules/physics module, categorizing issues by priority (P0-Critical, P1-Missing Features, P2-Improvements). The documentation provides detailed descriptions, code examples, impact analysis, and suggested fixes for each identified issue, serving as a roadmap for physics module improvements.
Key changes:
- Documents 5 critical bugs including memory leaks and logic errors
- Identifies 5 missing feature categories (raycasting, shapes, constraints, sleeping, character controllers)
- Lists 7 improvement opportunities ranging from broken parameter detection to empty help text
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| docs/physics-issues/P0-critical-bugs.md | Documents 5 critical bugs including memory leaks in body/constraint cleanup, ApplyForceAt logic error, and assertion macro typo |
| docs/physics-issues/P1-missing-features.md | Catalogs missing physics capabilities: raycasting/queries, additional shape types, constraint types, body sleeping, and character controller support |
| docs/physics-issues/P2-improvements.md | Lists 7 lower-priority improvements including broken soft body parameter detection, hardcoded system limits, and missing documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ```cpp | ||
| void cleanup(SHContext *context) { | ||
| if (_instance.node && _instance.node->node) { | ||
| auto &bodyNode = _instance->node; |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable name in the code example is inconsistent. Line 14 checks if (_instance.node && _instance.node->node) but line 15 declares auto &bodyNode = _instance->node;. This should be auto &bodyNode = _instance.node; to match the condition check.
| auto &bodyNode = _instance->node; | |
| auto &bodyNode = _instance.node; |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## devel #1223 +/- ##
==========================================
- Coverage 77.74% 77.73% -0.01%
==========================================
Files 386 386
Lines 59575 59575
==========================================
- Hits 46315 46313 -2
- Misses 13260 13262 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@claude can you open the issues listed in the PR .md files |
|
Claude finished @sinkingsugar's task —— View job Opening Issues from Physics Module ReviewStatus: Unable to Create IssuesI've analyzed all the documentation files and found 17 issues to create, but I don't have permission to create GitHub issues via the Summary of Issues FoundP0 - Critical Bugs (5 issues):
P1 - Missing Features (5 issues):
P2 - Improvements (7 issues):
Next StepsTo create these issues, you can either:
|
Documents bugs, missing features, and improvements found during code review of shards/modules/physics:
P0 (Critical):
P1 (Missing Features):
P2 (Improvements):