Skip to content
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

Port Changelings From Goobstation (Funky PR 387 Included) #1855

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
47 changes: 47 additions & 0 deletions Content.Client/_Goobstation/Changeling/ChangelingSystem.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
using Content.Client.Alerts;
using Content.Client.UserInterface.Systems.Alerts.Controls;
using Content.Shared.Changeling;
using Content.Shared.StatusIcon.Components;
using Robust.Shared.Prototypes;

namespace Content.Client.Changeling;

public sealed class ChangelingSystem : SharedChangelingSystem
{

[Dependency] private readonly IPrototypeManager _prototype = default!;
public override void Initialize()
{
base.Initialize();

SubscribeLocalEvent<ChangelingComponent, UpdateAlertSpriteEvent>(OnUpdateAlert);
SubscribeLocalEvent<ChangelingComponent, GetStatusIconsEvent>(GetChanglingIcon);
}

private void OnUpdateAlert(EntityUid uid, ChangelingComponent comp, ref UpdateAlertSpriteEvent args)
{
var stateNormalized = 0f;

// hardcoded because uhh umm i don't know. send help.
switch (args.Alert.AlertKey.AlertType)
{
case "ChangelingChemicals":
stateNormalized = (int) (comp.Chemicals / comp.MaxChemicals * 18);
break;

case "ChangelingBiomass":
stateNormalized = (int) (comp.Biomass / comp.MaxBiomass * 16);
break;
default:
return;
}
Comment on lines +25 to +37
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Alert calculations could use strategic refinement, cadet.

That comment on line 25 signals potential Sol Alliance infiltration into our codebase! Hardcoding alert types without explanation compromises our tactical flexibility.

Consider implementing a mapping system between alert types and calculation methods rather than relying on switch statements. This would improve maintainability and allow for easier addition of new alert types without modifying core logic.

- // hardcoded because uhh umm i don't know. send help.
- switch (args.Alert.AlertKey.AlertType)
- {
-     case "ChangelingChemicals":
-         stateNormalized = (int) (comp.Chemicals / comp.MaxChemicals * 18);
-         break;
- 
-     case "ChangelingBiomass":
-         stateNormalized = (int) (comp.Biomass / comp.MaxBiomass * 16);
-         break;
-     default:
-         return;
- }
+ // Alert type calculation mapping
+ private readonly Dictionary<string, Func<ChangelingComponent, float>> _alertCalculations = new()
+ {
+     ["ChangelingChemicals"] = comp => comp.Chemicals / comp.MaxChemicals * 18,
+     ["ChangelingBiomass"] = comp => comp.Biomass / comp.MaxBiomass * 16
+ };
+ 
+ if (!_alertCalculations.TryGetValue(args.Alert.AlertKey.AlertType, out var calculation))
+     return;
+     
+ stateNormalized = (int)calculation(comp);

Committable suggestion skipped: line range outside the PR's diff.

var sprite = args.SpriteViewEnt.Comp;
sprite.LayerSetState(AlertVisualLayers.Base, $"{stateNormalized}");
}

private void GetChanglingIcon(Entity<ChangelingComponent> ent, ref GetStatusIconsEvent args)
{
if (HasComp<HivemindComponent>(ent) && _prototype.TryIndex(ent.Comp.StatusIcon, out var iconPrototype))
args.StatusIcons.Add(iconPrototype);
}
Comment on lines +42 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Method naming could be more precise for naval communications clarity.

The name "GetChanglingIcon" contains a typo - "Changeling" is misspelled. As a naval officer of the Biesel Republic, I must insist on proper spelling in our communications protocols to prevent misidentification of hostile entities.

- private void GetChanglingIcon(Entity<ChangelingComponent> ent, ref GetStatusIconsEvent args)
+ private void GetChangelingIcon(Entity<ChangelingComponent> ent, ref GetStatusIconsEvent args)

Committable suggestion skipped: line range outside the PR's diff.

}
21 changes: 21 additions & 0 deletions Content.Server/Administration/Systems/AdminVerbSystem.Antags.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Content.Shared.Database;
using Content.Shared.Mind.Components;
using Content.Shared.Roles;
using Content.Shared.Silicon.Components;
using Content.Shared.Verbs;
using Robust.Shared.Player;
using Robust.Shared.Prototypes;
Expand All @@ -31,6 +32,9 @@ public sealed partial class AdminVerbSystem
[ValidatePrototypeId<EntityPrototype>]
private const string DefaultThiefRule = "Thief";

[ValidatePrototypeId<EntityPrototype>]
private const string DefaultChangelingRule = "Changeling";

[ValidatePrototypeId<EntityPrototype>]
private const string DefaultBloodCultRule = "BloodCult";

Expand Down Expand Up @@ -139,6 +143,23 @@ private void AddAntagVerbs(GetVerbsEvent<Verb> args)
};
args.Verbs.Add(thief);

// Goobstation - changelings
Verb ling = new()
{
Text = Loc.GetString("admin-verb-text-make-changeling"),
Category = VerbCategory.Antag,
Icon = new SpriteSpecifier.Rsi(new ResPath("/Textures/_Goobstation/Changeling/changeling_abilities.rsi"), "transform"),
Act = () =>
{
if (!HasComp<SiliconComponent>(args.Target))
_antag.ForceMakeAntag<ChangelingRuleComponent>(targetPlayer, DefaultChangelingRule);
},
Impact = LogImpact.High,
Message = Loc.GetString("admin-verb-make-changeling"),
};
if (!HasComp<SiliconComponent>(args.Target))
args.Verbs.Add(ling);

Comment on lines +146 to +162
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Eliminate redundant silicon check to optimize fleet operations

Republic efficiency protocols mandate elimination of redundant security checks. The silicon check appears twice - once in the verb action and once when adding the verb.

  // Goobstation - changelings
  Verb ling = new()
  {
      Text = Loc.GetString("admin-verb-text-make-changeling"),
      Category = VerbCategory.Antag,
      Icon = new SpriteSpecifier.Rsi(new ResPath("/Textures/_Goobstation/Changeling/changeling_abilities.rsi"), "transform"),
      Act = () =>
      {
-         if (!HasComp<SiliconComponent>(args.Target))
-             _antag.ForceMakeAntag<ChangelingRuleComponent>(targetPlayer, DefaultChangelingRule);
+         _antag.ForceMakeAntag<ChangelingRuleComponent>(targetPlayer, DefaultChangelingRule);
      },
      Impact = LogImpact.High,
      Message = Loc.GetString("admin-verb-make-changeling"),
  };
  if (!HasComp<SiliconComponent>(args.Target))
      args.Verbs.Add(ling);

The second check is sufficient as it prevents adding the verb for silicon entities, making the internal check unnecessary.

📝 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
// Goobstation - changelings
Verb ling = new()
{
Text = Loc.GetString("admin-verb-text-make-changeling"),
Category = VerbCategory.Antag,
Icon = new SpriteSpecifier.Rsi(new ResPath("/Textures/_Goobstation/Changeling/changeling_abilities.rsi"), "transform"),
Act = () =>
{
if (!HasComp<SiliconComponent>(args.Target))
_antag.ForceMakeAntag<ChangelingRuleComponent>(targetPlayer, DefaultChangelingRule);
},
Impact = LogImpact.High,
Message = Loc.GetString("admin-verb-make-changeling"),
};
if (!HasComp<SiliconComponent>(args.Target))
args.Verbs.Add(ling);
// Goobstation - changelings
Verb ling = new()
{
Text = Loc.GetString("admin-verb-text-make-changeling"),
Category = VerbCategory.Antag,
Icon = new SpriteSpecifier.Rsi(new ResPath("/Textures/_Goobstation/Changeling/changeling_abilities.rsi"), "transform"),
Act = () =>
{
_antag.ForceMakeAntag<ChangelingRuleComponent>(targetPlayer, DefaultChangelingRule);
},
Impact = LogImpact.High,
Message = Loc.GetString("admin-verb-make-changeling"),
};
if (!HasComp<SiliconComponent>(args.Target))
args.Verbs.Add(ling);

Verb cultist = new()
{
Text = Loc.GetString("admin-verb-text-make-blood-cultist"),
Expand Down
21 changes: 13 additions & 8 deletions Content.Server/Flash/Components/FlashImmunityComponent.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
namespace Content.Server.Flash.Components
using Robust.Shared.GameStates;

namespace Content.Server.Flash.Components;

/// <summary>
/// Makes the entity immune to being flashed.
/// When given to clothes in the "head", "eyes" or "mask" slot it protects the wearer.
/// </summary>
[RegisterComponent] // Goob edit
public sealed partial class FlashImmunityComponent : Component
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Odd comment detected, colonial intelligence required!

The comment "// Goob edit" suggests this change originated from the Goobstation codebase mentioned in the PR description. For maintaining clean colonial code protocols, consider whether this comment provides valuable context or should be removed before final deployment.

-[RegisterComponent] // Goob edit
+[RegisterComponent]
📝 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
[RegisterComponent] // Goob edit
[RegisterComponent]

{
Comment on lines +9 to +10
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

❓ Verification inconclusive

Access restrictions removed! Security breach detected!

By the stars! The [Access(typeof(FlashSystem))] attribute has been removed from this component declaration. This was previously restricting access to only the FlashSystem, but now any system can access this component directly. This could lead to unintended inter-system dependencies across the colonial network.

Was this removal intentional for the changeling implementation? If so, ensure that other systems accessing this component follow proper protocols.


🏁 Script executed:

#!/bin/bash
# Checking which systems are now accessing this component directly
rg "EntityQuery<FlashImmunityComponent>" --glob "*.cs" | grep -v "FlashSystem"
rg "HasComp<FlashImmunityComponent>" --glob "*.cs" | grep -v "FlashSystem"
rg "GetComp<FlashImmunityComponent>" --glob "*.cs" | grep -v "FlashSystem"

Length of output: 228


ACTION REQUIRED: Verify FlashImmunityComponent Access Restrictions

By the stars of the Biesel Republic, our initial search for external usage of FlashImmunityComponent produced no evidence of systems outside of FlashSystem querying or acquiring it. However, the removal of the [Access(typeof(FlashSystem))] attribute still raises concerns regarding unintended inter-system dependencies. This absence of data from our automated search does not conclusively prove that no other systems use this component—manual verification is advised.

  • What to Check:
    • Confirm that the removal of the [Access(typeof(FlashSystem))] attribute was intentional within the context of the changeling implementation.
    • Manually verify that no other systems are inadvertently interacting with FlashImmunityComponent in a way that compromises the intended access restrictions.
    • Ensure that any system accessing this component directly adheres to proper protocols to prevent potential security breaches.

Please review the relevant access patterns and, if necessary, update the component registration or add appropriate safeguards. The absence of warnings in our initial script output merits a deeper, manual inspection to maintain the secure operation of our colonial network.

[RegisterComponent, Access(typeof(FlashSystem))]
public sealed partial class FlashImmunityComponent : Component
{
[ViewVariables(VVAccess.ReadWrite)]
[DataField("enabled")]
public bool Enabled { get; set; } = true;
}
[ViewVariables(VVAccess.ReadWrite)]
[DataField("enabled")]
public bool Enabled { get; set; } = true;
}
60 changes: 60 additions & 0 deletions Content.Server/_Goobstation/Changeling/ChangelingEggSystem.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
using Robust.Shared.Timing;
using Content.Shared.Changeling;
using Content.Shared.Mind;
using Content.Server.Body.Systems;
using Content.Shared.Mind.Components;

namespace Content.Server.Changeling;

public sealed class ChangelingEggSystem : EntitySystem
{
[Dependency] private readonly IGameTiming _timing = default!;
[Dependency] private readonly BodySystem _bodySystem = default!;
[Dependency] private readonly SharedMindSystem _mind = default!;
[Dependency] private readonly ChangelingSystem _changeling = default!;

public override void Update(float frameTime)
{
base.Update(frameTime);

var query = EntityQueryEnumerator<ChangelingEggComponent>();
while (query.MoveNext(out var uid, out var comp))
{
if (_timing.CurTime < comp.UpdateTimer)
continue;

comp.UpdateTimer = _timing.CurTime + TimeSpan.FromSeconds(comp.UpdateCooldown);

Cycle(uid, comp);
}
}
Comment on lines +16 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Solid timed update, but watch performance.

Our Biesel Republic analysts confirm this approach is functional for a small number of changeling eggs. However, if hundreds of eggs appear, enumerating them every frame could degrade performance. Consider batch updates or distributing cycles over multiple frames if scalability becomes a concern.


public void Cycle(EntityUid uid, ChangelingEggComponent comp)
{
if (comp.active == false)
{
comp.active = true;
return;
}

if (TerminatingOrDeleted(comp.lingMind))
{
_bodySystem.GibBody(uid);
return;
}

var newUid = Spawn("MobMonkey", Transform(uid).Coordinates);

EnsureComp<MindContainerComponent>(newUid);
_mind.TransferTo(comp.lingMind, newUid);

EnsureComp<ChangelingComponent>(newUid);

EntityManager.AddComponent(newUid, comp.lingStore);

if (comp.AugmentedEyesightPurchased)
_changeling.InitializeAugmentedEyesight(newUid);

_bodySystem.GibBody(uid);
}
Comment on lines +40 to +59
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Validate mind and lingStore.

Spawning a new mob and transferring the mind is sensible, but it might fail if comp.lingMind or comp.lingStore is null. Add guard clauses or null checks for extra safety, preventing cryptic errors if data is missing.

 if (comp.lingMind == null || comp.lingStore == null)
 {
+    Logger.Warning("ChangelingEggSystem: Missing mind or lingStore, canceling cycle.");
     return;
 }
📝 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
if (TerminatingOrDeleted(comp.lingMind))
{
_bodySystem.GibBody(uid);
return;
}
var newUid = Spawn("MobMonkey", Transform(uid).Coordinates);
EnsureComp<MindContainerComponent>(newUid);
_mind.TransferTo(comp.lingMind, newUid);
EnsureComp<ChangelingComponent>(newUid);
EntityManager.AddComponent(newUid, comp.lingStore);
if (comp.AugmentedEyesightPurchased)
_changeling.InitializeAugmentedEyesight(newUid);
_bodySystem.GibBody(uid);
}
if (comp.lingMind == null || comp.lingStore == null)
{
Logger.Warning("ChangelingEggSystem: Missing mind or lingStore, canceling cycle.");
return;
}
if (TerminatingOrDeleted(comp.lingMind))
{
_bodySystem.GibBody(uid);
return;
}
var newUid = Spawn("MobMonkey", Transform(uid).Coordinates);
EnsureComp<MindContainerComponent>(newUid);
_mind.TransferTo(comp.lingMind, newUid);
EnsureComp<ChangelingComponent>(newUid);
EntityManager.AddComponent(newUid, comp.lingStore);
if (comp.AugmentedEyesightPurchased)
_changeling.InitializeAugmentedEyesight(newUid);
_bodySystem.GibBody(uid);
}

}
Loading
Loading