URA0103: resolve lingering bomb issue; fix failure to destroy on water impact#7077
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughCNeutronClusterBombProjectile now forks a failsafe destroy thread on create and marks ChangesNeutron cluster projectile behavior & config
Sequence Diagram(s)sequenceDiagram
actor Launcher
participant Projectile
participant FailsafeThread
participant World
participant Fragment
Launcher->>Projectile: spawn()
Projectile->>FailsafeThread: Fork(FailsafeDestroyThread)
Note right of FailsafeThread: waits 120 ticks
World->>Projectile: OnImpact(targetType)
Projectile->>Projectile: if not Impacted -> set Impacted = true
alt targetType != "Air"
Projectile->>Fragment: Create child fragments
Fragment->>World: SetVelocity / simulate
end
FailsafeThread-->>Projectile: if not destroyed -> Destroy()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
🧹 Nitpick comments (1)
lua/cybranprojectiles.lua (1)
517-523: Optional: compact fragment velocity definitions into a table-driven loop.This block is correct, but converting the velocity tuples to a list and iterating would reduce repetition and make future tuning safer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lua/cybranprojectiles.lua` around lines 517 - 523, The repeated SpawnFragment(...) calls should be replaced by a table-driven loop: create a local table of velocity tuples (each entry containing the three Random(...) expressions used now), then iterate over that table and call SpawnFragment(v[1], v[2], v[3]) for each entry; update the block around the SpawnFragment calls in lua/cybranprojectiles.lua (referencing the SpawnFragment function and the Random(...) expressions) so tuning only requires editing the tuple list rather than duplicating calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lua/cybranprojectiles.lua`:
- Around line 517-523: The repeated SpawnFragment(...) calls should be replaced
by a table-driven loop: create a local table of velocity tuples (each entry
containing the three Random(...) expressions used now), then iterate over that
table and call SpawnFragment(v[1], v[2], v[3]) for each entry; update the block
around the SpawnFragment calls in lua/cybranprojectiles.lua (referencing the
SpawnFragment function and the Random(...) expressions) so tuning only requires
editing the tuple list rather than duplicating calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6af72121-f5a1-4522-82df-9ff3b996da8e
📒 Files selected for processing (2)
lua/cybranprojectiles.luaprojectiles/CIFNeutronClusterBomb02/CIFNeutronClusterBomb02_proj.bp
| OnCreate = function(self) | ||
| SinglePolyTrailProjectile.OnCreate(self) | ||
| self.Impacted = false | ||
| self.Trash:Add(ForkThread(self.FailsafeDestroyThread, self)) |
There was a problem hiding this comment.
This should never be necessary, the engine gives projectiles a lifetime which will destroy it the same way.
| if self.Impacted == false and targetType ~= 'Air' then | ||
| if self.Impacted == false then |
There was a problem hiding this comment.
an Air impact type occurs when the projectile is in the air and runs out of lifetime. The original OnImpact should destroy the projectile. So previously there was somehow an air collision type which meant the projectile never destroyed itself, and permanently stopped running the collision script with Impacted set to true, so it just stayed around forever.
There was a problem hiding this comment.
I made a mistake in this explanation assuming that OnImpact always destroys the projectile. If the target entity does not allow collisions then OnImpact returns early. Units stop allowing collisions after they die. This leads to a much more reasonable explanation of the bomb impacting an engi directly, but that engi died to earlier bombs in the cluster, so Impacted is set to true, but Destroy is not called.
Description of the proposed changes
Fixes two separate issues:
Example of the bomb lingering issue: #26768413 5:50 at the northern rock base.
Checklist
Summary by CodeRabbit
Bug Fixes
Documentation