Conversation
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughSwitches tail-segment handling to use the entity's parent context and adds guards and explicit joint removal: OnComponentShutdown removes joints and clears segments before deletion, InitializeTailSegments and segment spawning use ParentUid, and segment velocity/rotation updates skip segments without a valid parent. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Content.Shared/_Exodus/Tailed/TailedEntitySystem.cs`:
- Around line 54-60: The removal loop in the tail shutdown uses prev
unconditionally, which can produce mismatched joint IDs when some segments lack
a PhysicsComponent; update the loop in the removal path (the foreach over
component.TailSegments that calls _joint.RemoveJoint) to mirror
InitializeTailSegments by first checking for a PhysicsComponent on the current
segment, only calling _joint.RemoveJoint and updating prev when that
PhysicsComponent exists, ensuring joint IDs match creation and dangling joints
are not left behind.
|
Кролик полезный |
64f8cbe to
8cdf89f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Content.Shared/_Exodus/Tailed/TailedEntitySystem.cs (1)
81-91:⚠️ Potential issue | 🟠 MajorConvert world position to parent-local coordinates before spawning segments.
spawnPosis derived from world-space operations (GetWorldPositionandGetWorldRotation), butEntityCoordinates(parentUid, spawnPos)interprets it as a local offset relative toparentUid. This works only whenparentUidis the map entity. If the tailed entity is parented to a grid or container, segments spawn at incorrect locations.Convert to local coordinates using
GetInvWorldMatrixbefore creatingEntityCoordinates:Proposed fix
- var segment = PredictedSpawnAtPosition(comp.Prototype, new EntityCoordinates(parentUid, spawnPos)); + var localPos = _transform.GetInvWorldMatrix(parentUid).Transform(spawnPos); + var segment = PredictedSpawnAtPosition(comp.Prototype, new EntityCoordinates(parentUid, localPos));
🤖 Fix all issues with AI agents
In `@Content.Shared/Movement/Systems/SharedMoverController.Tailed.cs`:
- Around line 136-139: The GetWorldPosition call on the segment is happening
before the parent validity check, which could produce incorrect results if the
parent is invalid. Move the guard that checks if
segmentXform.ParentUid.IsValid() to occur before calling
_transform.GetWorldPosition(segment), so invalid parent segments are skipped
before attempting the position lookup. Follow the same pattern used in the
ApplySegmentVelocities method where the parent validity check happens first,
then the transform operations proceed only for valid parents.
| var segmentPos = _transform.GetWorldPosition(segment); | ||
| var segmentXform = Transform(segment); | ||
| if (!segmentXform.ParentUid.IsValid()) | ||
| continue; |
There was a problem hiding this comment.
GetWorldPosition is called before the parent validity check.
_transform.GetWorldPosition(segment) on line 136 runs before the ParentUid.IsValid() guard on line 138. If the segment has no valid parent, this call may produce a garbage position or throw. Move the guard above the position lookup, consistent with how ApplySegmentVelocities handles it (lines 64–68).
Proposed fix
var segment = tail.TailSegments[i];
+ var segmentXform = Transform(segment);
+ if (!segmentXform.ParentUid.IsValid())
+ continue;
+
var segmentPos = _transform.GetWorldPosition(segment);
- var segmentXform = Transform(segment);
- if (!segmentXform.ParentUid.IsValid())
- continue;📝 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.
| var segmentPos = _transform.GetWorldPosition(segment); | |
| var segmentXform = Transform(segment); | |
| if (!segmentXform.ParentUid.IsValid()) | |
| continue; | |
| var segmentXform = Transform(segment); | |
| if (!segmentXform.ParentUid.IsValid()) | |
| continue; | |
| var segmentPos = _transform.GetWorldPosition(segment); |
🤖 Prompt for AI Agents
In `@Content.Shared/Movement/Systems/SharedMoverController.Tailed.cs` around lines
136 - 139, The GetWorldPosition call on the segment is happening before the
parent validity check, which could produce incorrect results if the parent is
invalid. Move the guard that checks if segmentXform.ParentUid.IsValid() to occur
before calling _transform.GetWorldPosition(segment), so invalid parent segments
are skipped before attempting the position lookup. Follow the same pattern used
in the ApplySegmentVelocities method where the parent validity check happens
first, then the transform operations proceed only for valid parents.
About the PR
Why / Balance
How to test
Media
Requirements
Breaking changes
Changelog
Summary by CodeRabbit