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

[Fix] Borg and Golem LayDown #906

Merged
merged 5 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
using System.Linq;
using Content.Server.Administration;
using Content.Server.Silicons.Laws;
using Content.Shared.Administration;
using Content.Shared.Silicons.Laws;
using Content.Shared.Silicons.Laws.Components;
using Robust.Shared.Toolshed;
using Robust.Shared.Toolshed.TypeParsers;

namespace Content.Server.Backmen.Administration.Commands.Toolshed;

[ToolshedCommand, AdminCommand(AdminFlags.Admin)]
public sealed class LawsCommand : ToolshedCommand
{
private SiliconLawSystem? _law;
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

Potential Thread-Safety Issue with Lazy Initialization of _law

The _law field is lazily initialized in multiple methods without synchronization. In a multithreaded context, this could lead to race conditions. Consider initializing _law once in the constructor or at declaration time to ensure thread safety.

Apply this diff to fix the issue:

-private SiliconLawSystem? _law;
+private readonly SiliconLawSystem _law;

+public LawsCommand()
+{
+    _law = GetSys<SiliconLawSystem>();
+}

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


[CommandImplementation("list")]
public IEnumerable<EntityUid> List()
{
var query = EntityManager.EntityQueryEnumerator<SiliconLawBoundComponent>();
while (query.MoveNext(out var uid, out _))
{
yield return uid;
}
}

[CommandImplementation("get")]
public IEnumerable<string> Get([PipedArgument] EntityUid lawbound)
{
_law ??= GetSys<SiliconLawSystem>();

foreach (var law in _law.GetLaws(lawbound).Laws)
{
yield return $"law {law.LawIdentifierOverride ?? law.Order.ToString()}: {Loc.GetString(law.LawString)}";
}
}

[CommandImplementation("set")]
public EntityUid? SetLaws(
[CommandInvocationContext] IInvocationContext ctx,
[PipedArgument] EntityUid input,
[CommandArgument] Prototype<SiliconLawsetPrototype> siliconLawSetPrototype
)
{
_law ??= GetSys<SiliconLawSystem>();

_law.SetLaws(input, siliconLawSetPrototype.Value);
return input;
}

[CommandImplementation("set")]
public IEnumerable<EntityUid> SetLaws(
[CommandInvocationContext] IInvocationContext ctx,
[PipedArgument] IEnumerable<EntityUid> input,
[CommandArgument] Prototype<SiliconLawsetPrototype> siliconLawSetPrototype
)
{
return input.Where(ent => SetLaws(ctx, ent, siliconLawSetPrototype) != null);
}

[CommandImplementation("override")]
public EntityUid? OverrideLaw(
[CommandInvocationContext] IInvocationContext ctx,
[PipedArgument] EntityUid input,
[CommandArgument] int index,
[CommandArgument] string lawString
)
{
if (index < 0)
return null;
Comment on lines +69 to +70
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

Validate Index Before Inserting New Law

When inserting a new law into laws.Laws, the index should be validated to prevent an ArgumentOutOfRangeException. Currently, only negative indices are checked. If the index is greater than the count of existing laws, it will cause an exception.

Apply this diff to fix the issue:

-if (index < 0)
+if (index < 0 || index > laws.Laws.Count)
     return null;

This ensures that the index is within the valid range before insertion.

Also applies to: 77-84

_law ??= GetSys<SiliconLawSystem>();

var laws = _law.GetLaws(input);
var law = laws.Laws.FirstOrDefault(x => x.Order == index);
if (law == null)
{
laws.Laws.Insert(index,
new SiliconLaw()
{
Order = index,
LawString = lawString,
}
);
}
else
{
law.LawString = lawString;
}

return input;
}

[CommandImplementation("override")]
public IEnumerable<EntityUid> OverrideLaw(
[CommandInvocationContext] IInvocationContext ctx,
[PipedArgument] IEnumerable<EntityUid> input,
[CommandArgument] int index,
[CommandArgument] string lawString
)
{
return input.Where(uid => OverrideLaw(ctx, uid, index, lawString) != null);
}
}
46 changes: 19 additions & 27 deletions Content.Server/Silicons/Laws/SiliconLawSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,25 @@ public SiliconLawset GetLawset(ProtoId<SiliconLawsetPrototype> lawset)
return laws;
}

//backmen-start
public void SetLaws(EntityUid uid, SiliconLawsetPrototype prototype)
{
if (_prototype.TryIndex<SiliconLawsetPrototype>(prototype, out var lawSet))
{
var laws = lawSet.Laws
.Select(x => _prototype.Index<SiliconLawPrototype>(x))
.Select(x => new SiliconLaw()
{
Order = x.Order,
LawString = x.LawString,
LawIdentifierOverride = x.LawIdentifierOverride,
})
.ToList();
SetLaws(laws, uid);
}
}
Comment on lines +273 to +288
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

Add error handling and improve consistency with existing patterns

The current implementation has several issues that need to be addressed:

  1. Silent failure when prototype lookup fails
  2. Inconsistent parameter type compared to other methods
  3. Multiple LINQ operations that could be combined
  4. Missing documentation

Apply these improvements:

-    public void SetLaws(EntityUid uid, SiliconLawsetPrototype prototype)
+    /// <summary>
+    /// Sets the laws for a silicon entity using a lawset prototype.
+    /// </summary>
+    /// <param name="uid">The entity to set laws for</param>
+    /// <param name="protoId">The ID of the lawset prototype</param>
+    /// <exception cref="ArgumentException">Thrown when the prototype is not found</exception>
+    public void SetLaws(EntityUid uid, ProtoId<SiliconLawsetPrototype> protoId)
     {
-        if (_prototype.TryIndex<SiliconLawsetPrototype>(prototype, out var lawSet))
-        {
-            var laws = lawSet.Laws
-                .Select(x => _prototype.Index<SiliconLawPrototype>(x))
-                .Select(x => new SiliconLaw()
-                {
-                    Order = x.Order,
-                    LawString = x.LawString,
-                    LawIdentifierOverride = x.LawIdentifierOverride,
-                })
-                .ToList();
-            SetLaws(laws, uid);
-        }
+        var lawSet = _prototype.Index(protoId);
+        var laws = lawSet.Laws
+            .Select(x => _prototype.Index(x))
+            .Select(x => new SiliconLaw
+            {
+                Order = x.Order,
+                LawString = x.LawString,
+                LawIdentifierOverride = x.LawIdentifierOverride,
+            })
+            .ToList();
+        SetLaws(laws, uid);
     }
📝 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
public void SetLaws(EntityUid uid, SiliconLawsetPrototype prototype)
{
if (_prototype.TryIndex<SiliconLawsetPrototype>(prototype, out var lawSet))
{
var laws = lawSet.Laws
.Select(x => _prototype.Index<SiliconLawPrototype>(x))
.Select(x => new SiliconLaw()
{
Order = x.Order,
LawString = x.LawString,
LawIdentifierOverride = x.LawIdentifierOverride,
})
.ToList();
SetLaws(laws, uid);
}
}
/// <summary>
/// Sets the laws for a silicon entity using a lawset prototype.
/// </summary>
/// <param name="uid">The entity to set laws for</param>
/// <param name="protoId">The ID of the lawset prototype</param>
/// <exception cref="ArgumentException">Thrown when the prototype is not found</exception>
public void SetLaws(EntityUid uid, ProtoId<SiliconLawsetPrototype> protoId)
{
var lawSet = _prototype.Index(protoId);
var laws = lawSet.Laws
.Select(x => _prototype.Index(x))
.Select(x => new SiliconLaw
{
Order = x.Order,
LawString = x.LawString,
LawIdentifierOverride = x.LawIdentifierOverride,
})
.ToList();
SetLaws(laws, uid);
}

//backmen-end

/// <summary>
/// Set the laws of a silicon entity while notifying the player.
/// </summary>
Expand Down Expand Up @@ -299,30 +318,3 @@ protected override void OnUpdaterInsert(Entity<SiliconLawUpdaterComponent> ent,
}
}
}

[ToolshedCommand, AdminCommand(AdminFlags.Admin)]
public sealed class LawsCommand : ToolshedCommand
{
private SiliconLawSystem? _law;

[CommandImplementation("list")]
public IEnumerable<EntityUid> List()
{
var query = EntityManager.EntityQueryEnumerator<SiliconLawBoundComponent>();
while (query.MoveNext(out var uid, out _))
{
yield return uid;
}
}

[CommandImplementation("get")]
public IEnumerable<string> Get([PipedArgument] EntityUid lawbound)
{
_law ??= GetSys<SiliconLawSystem>();

foreach (var law in _law.GetLaws(lawbound).Laws)
{
yield return $"law {law.LawIdentifierOverride ?? law.Order.ToString()}: {Loc.GetString(law.LawString)}";
}
}
}
3 changes: 2 additions & 1 deletion Content.Shared/Backmen/Standing/SharedLayingDownSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Content.Shared.Movement.Systems;
using Content.Shared.Physics;
using Content.Shared.Popups;
using Content.Shared.Silicons.Borgs.Components;
using Content.Shared.Standing;
using Content.Shared.Stunnable;
using Content.Shared.Tag;
Expand Down Expand Up @@ -83,7 +84,7 @@ private void OnCheckLegs(Entity<LayingDownComponent> ent, ref StandAttemptEvent
if(!TryComp<BodyComponent>(ent, out var body))
return;

if (body.LegEntities.Count < body.RequiredLegs || body.LegEntities.Count == 0)
if (!HasComp<BorgChassisComponent>(ent) && (body.LegEntities.Count < body.RequiredLegs || body.LegEntities.Count == 0))
args.Cancel(); // no legs bro
}

Expand Down
4 changes: 3 additions & 1 deletion Content.Shared/Body/Systems/SharedBodySystem.Body.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
using Content.Shared.Humanoid;
using Content.Shared.Humanoid.Events;
using Content.Shared.Inventory;
using Content.Shared.Rejuvenate;
using Content.Shared.Silicons.Borgs.Components;
using Content.Shared.Standing;
using Robust.Shared.Audio;
using Robust.Shared.Audio.Systems;
Expand Down Expand Up @@ -139,7 +141,7 @@ private void OnBodyCanDrag(Entity<BodyComponent> ent, ref CanDragEvent args)

private void OnStandAttempt(Entity<BodyComponent> ent, ref StandAttemptEvent args)
{
if (ent.Comp.LegEntities.Count == 0)
if (!HasComp<BorgChassisComponent>(ent) && ent.Comp.LegEntities.Count == 0)
args.Cancel();
}

Expand Down
3 changes: 3 additions & 0 deletions Resources/Locale/ru-RU/backmen/toolshed.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,6 @@ command-description-changeaccessreader-rmGroupAccessReader = Очистить г
command-description-changeaccessreader-setJobAccessReader = Установить доступ в соответствии с JobPrototype
command-description-changeaccessreader-addJobAccessReader = Добавить доступ в соответствии с JobPrototype
command-description-changeaccessreader-rmJobAccessReader = Удалить доступы в соответствии с JobPrototype

command-description-laws-set = Установить законы по прототипу
command-description-laws-override = Изменить закон по индексу, новый закон пишется строкой
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@
- type: Emoting
- type: TTS # Corvax-TTS
voice: Borgs
- type: LayingDown # backmen: LayingDown
- type: GuideHelp
guides:
- Cyborgs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
abstract: true
description: Искусственная конструкция, имитирующая жизнь. У этого еще не установлен кристалл души.
components:
- type: LayingDown # backmen: LayingDown
- type: ComplexInteraction
- type: ActionGrant
actions:
Expand Down
Loading