Skip to content

Remove sparks from lightning - #5396

Closed
Swonkus wants to merge 1 commit into
ss14Starlight:starlight-devfrom
Swonkus:Remove-spark-from-supermatter
Closed

Remove sparks from lightning#5396
Swonkus wants to merge 1 commit into
ss14Starlight:starlight-devfrom
Swonkus:Remove-spark-from-supermatter

Conversation

@Swonkus

@Swonkus Swonkus commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Removes sparking from lightning

Short description

Removes sparking from lightning due to supermatter interaction

Why we need to add this

Supermatter will ignite its tritium when striking entities. (can maybe be re-enabled if it can be moved to the entity being struck instead).

Checks

  • I do not require assistance to complete the PR.
  • Before posting/requesting review of a PR, I have verified that the changes work.
  • I have added screenshots/videos of the changes, or this PR does not change in-game mechanics.
  • I affirm that my changes are licensed under the MIT License and grant permission for use in this repository under its conditions.

Changelog

🆑 Swonki

  • remove: lightning ignition sparks.

Removes sparking from lightning
@Swonkus
Swonkus requested a review from a team July 29, 2026 13:22
@github-actions github-actions Bot added size/XS S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. Changes: C# and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. labels Jul 29, 2026
@Swonkus
Swonkus marked this pull request as draft July 29, 2026 13:23
@Swonkus

Swonkus commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

i want to do just a bit more testing to make sure this works, sparks didnt happen 100% of the time in live server test

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

Lightning sparking behavior

Layer / File(s) Summary
Clear sparking state on lightning hit
Content.Server/Tesla/EntitySystem/LightningSparkingSystem.cs
OnHitByLightning now sets IsSparking to false, preventing the update loop’s sparking behavior from starting for struck entities.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested reviewers: redmushie

🚥 Pre-merge checks | ✅ 8
✅ Passed checks (8 passed)
Check name Status Explanation
Title check ✅ Passed Clear, concise, and accurately reflects the lightning spark removal change.
Description check ✅ Passed The description is directly related to removing lightning sparking and explains the supermatter motivation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No Magic Numbers ✅ Passed PASS — LightningSparkingSystem.cs has no non-obvious numeric literals; the PR only flips IsSparking from true to false.
No Hardcoded Ecs Parameters ✅ Passed Lightning timing remains component-driven; the changed false is just a state toggle, not a hardcoded ECS parameter.
Avoid Service Locator ✅ Passed Nice, clean change: the diff only flips IsSparking and uses existing injected dependencies, with no service-locator calls.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@Content.Server/Tesla/EntitySystem/LightningSparkingSystem.cs`:
- Around line 25-27: Update the sparking-disable logic around
LightningSparkingSystem’s appearance update so TeslaCoilVisuals.Lightning is
cleared when IsSparking is set to false. Preserve the existing timing assignment
and ensure Update retains a cleanup path rather than leaving the lightning
appearance enabled indefinitely.
🪄 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: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 36f18b4c-7109-4574-b04c-b142417b3f59

📥 Commits

Reviewing files that changed from the base of the PR and between cfaef58 and 75f2308.

📒 Files selected for processing (1)
  • Content.Server/Tesla/EntitySystem/LightningSparkingSystem.cs

Comment on lines 25 to +27
_appearance.SetData(uid.Owner, TeslaCoilVisuals.Lightning, true);
uid.Comp.LightningEndTime = _gameTiming.CurTime + TimeSpan.FromSeconds(uid.Comp.LightningTime);
uid.Comp.IsSparking = true;
uid.Comp.IsSparking = false;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Clear the lightning appearance when disabling sparking.

Nice targeted change toward preventing lightning-induced ignition, but Line 25 still enables the lightning appearance while Line 27 makes Update skip the cleanup path. After the first hit, TeslaCoilVisuals.Lightning can remain enabled indefinitely. Set the appearance to false here, or otherwise preserve a cleanup path.

Proposed fix
-        _appearance.SetData(uid.Owner, TeslaCoilVisuals.Lightning, true);
+        _appearance.SetData(uid.Owner, TeslaCoilVisuals.Lightning, false);
         uid.Comp.LightningEndTime = _gameTiming.CurTime + TimeSpan.FromSeconds(uid.Comp.LightningTime);
         uid.Comp.IsSparking = false;
📝 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.

Suggested change
_appearance.SetData(uid.Owner, TeslaCoilVisuals.Lightning, true);
uid.Comp.LightningEndTime = _gameTiming.CurTime + TimeSpan.FromSeconds(uid.Comp.LightningTime);
uid.Comp.IsSparking = true;
uid.Comp.IsSparking = false;
_appearance.SetData(uid.Owner, TeslaCoilVisuals.Lightning, false);
uid.Comp.LightningEndTime = _gameTiming.CurTime + TimeSpan.FromSeconds(uid.Comp.LightningTime);
uid.Comp.IsSparking = false;
🤖 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 `@Content.Server/Tesla/EntitySystem/LightningSparkingSystem.cs` around lines 25
- 27, Update the sparking-disable logic around LightningSparkingSystem’s
appearance update so TeslaCoilVisuals.Lightning is cleared when IsSparking is
set to false. Preserve the existing timing assignment and ensure Update retains
a cleanup path rather than leaving the lightning appearance enabled
indefinitely.

@Swonkus Swonkus closed this Jul 29, 2026
@coderabbitai coderabbitai Bot mentioned this pull request Jul 29, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant