[PORT] B.O.R.I.S ai remote control brain (#2745)#505
Open
ren0san wants to merge 3 commits intocorvax-team:masterfrom
Open
[PORT] B.O.R.I.S ai remote control brain (#2745)#505ren0san wants to merge 3 commits intocorvax-team:masterfrom
ren0san wants to merge 3 commits intocorvax-team:masterfrom
Conversation
* ai remote brain * system fixes, code formatting * GoobChanges * fixes * more fixes * ehh * 1 * chore: Automatically update REUSE headers * chore: Automatically update REUSE headers * fix spelling error * chore: Automatically update REUSE headers * bruh * fixes * ok * OK * Revert "ok" This reverts commit a05b43e01d3d1c6d9aed64452637f82f40a1ea36. * chore: Automatically update REUSE headers * chore: Automatically update REUSE headers * Apply some suggestions from code review Co-authored-by: gluesniffler <159397573+gluesniffler@users.noreply.github.com> * chore: Automatically update REUSE headers * comments * chore: Automatically update REUSE headers * oops * ok i think.. * chore: Automatically update REUSE headers --------- Co-authored-by: KillanGenifer <killangenifer@gmail.com> Co-authored-by: ImHoks <imhokzzzz@gmail.com> Co-authored-by: GoobBot <uristmchands@proton.me> Co-authored-by: gluesniffler <159397573+gluesniffler@users.noreply.github.com>
github-actions
bot
added
S: Untriaged
S: Needs Review
size/M
Changes: UI
Changes: Localization
and removed
S: Needs Review
size/M
labels
Contributor
📝 WalkthroughWalkthroughAdds a new AI remote-control feature: client UI for selecting remote devices, server systems for mind transfer and device management, shared components for remote brains/controllers, localization, prototypes, and recipes to support remote AI control and handover flows. Changes
Sequence DiagramsequenceDiagram
participant Client as Client UI
participant Server as AiRemoteControlSystem
participant Mind as SharedMindSystem
participant Station as Station AI Core
participant Law as SiliconLawSystem
participant Target as Target Borg
Client->>Server: Toggle/Send RemoteDeviceAction(TakeControl, target)
activate Server
Server->>Server: Capture transmitter & radio channel state
Server->>Station: Retrieve Station AI core & remote entity
Server->>Mind: Request/transfer linked mind from remote entity
Mind-->>Server: Provide linked mind
Server->>Target: Assign mind to target borg
Target-->>Server: Confirm assignment
Server->>Station: Swap/restore radio channels
Server->>Law: RewriteLaws(source, target)
Law-->>Server: Confirm laws synced
Server->>Client: Update RemoteDevicesBuiState(updatedList)
deactivate Server
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Content.Shared/Silicons/Borgs/SharedBorgSystem.cs (1)
249-255:⚠️ Potential issue | 🟡 MinorShow the “panel not open” popup for AI remote brains too.
AiRemoteBrainComponentfollows the same insertion constraints, but currently gets no feedback in the closed-panel path.Proposed fix
- if (brain != null || module != null) + if (brain != null || module != null || aiBrain != null) { _popup.PopupClient(Loc.GetString("borg-panel-not-open"), chassis, args.User); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/Silicons/Borgs/SharedBorgSystem.cs` around lines 249 - 255, The closed-panel branch only shows the popup when a local brain or module is present; include AI remotes too by checking for AiRemoteBrainComponent. Update the condition in the TryComp<WiresPanelComponent> block (where you currently test panel.Open and then "if (brain != null || module != null) _popup.PopupClient(...)") to also detect remote brains—e.g. extend the condition to "if (brain != null || module != null || TryComp<AiRemoteBrainComponent>(chassis, out _))" and call _popup.PopupClient(Loc.GetString("borg-panel-not-open"), chassis, args.User) in that case so AI remote brains receive the same feedback.
🧹 Nitpick comments (6)
Content.Shared/_CorvaxNext/Silicons/Borgs/Components/SharedAiRemoteControllerComponent.cs (1)
65-67: Defensively copydeviceListin BUI state constructor.Line 67 keeps the caller’s mutable
Listreference. Later mutations can unintentionally alter already-built UI state.Proposed fix
public RemoteDevicesBuiState(List<RemoteDevicesData> deviceList) { - DeviceList = deviceList; + ArgumentNullException.ThrowIfNull(deviceList); + DeviceList = new List<RemoteDevicesData>(deviceList); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_CorvaxNext/Silicons/Borgs/Components/SharedAiRemoteControllerComponent.cs` around lines 65 - 67, The RemoteDevicesBuiState constructor currently assigns the caller's mutable List directly to DeviceList; change the constructor for RemoteDevicesBuiState to defensively copy the input (e.g., create a new List<RemoteDevicesData> from the passed deviceList or use deviceList?.ToList() and fall back to an empty list) so future external mutations don't alter stored BUI state; locate the RemoteDevicesBuiState(List<RemoteDevicesData> deviceList) constructor and replace the direct assignment with a new list copy.Content.Server/Silicons/Laws/SiliconLawSystem.cs (1)
317-324: Avoid sharing one mutableList<SiliconLaw>instance across targets.Right now the same list reference can be assigned to multiple entities, which makes later inserts/removals on one side leak to others. Prefer copying on assignment.
Proposed refactor
public void SetLaws(List<SiliconLaw> newLaws, EntityUid target, SoundSpecifier? cue = null) { if (!TryComp<SiliconLawProviderComponent>(target, out var component)) return; if (component.Lawset == null) component.Lawset = new SiliconLawset(); - component.Lawset.Laws = newLaws; + component.Lawset.Laws = new List<SiliconLaw>(newLaws); NotifyLawsChanged(target, cue); } public void SetLawsSilent(List<SiliconLaw> newLaws, EntityUid target, SoundSpecifier? cue = null) { if (!TryComp<SiliconLawProviderComponent>(target, out var component)) return; if (component.Lawset == null) component.Lawset = new SiliconLawset(); - component.Lawset.Laws = newLaws; + component.Lawset.Laws = new List<SiliconLaw>(newLaws); }Also applies to: 328-337
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Server/Silicons/Laws/SiliconLawSystem.cs` around lines 317 - 324, The code assigns the same mutable List<SiliconLaw> instance (lawset.Laws) to multiple entities which can cause shared-state bugs; update SetLaws (or the call sites in SiliconLawSystem.cs where SetLaws is invoked) to store/assign a defensive copy (e.g., new List<SiliconLaw>(laws)) instead of the original collection. Specifically, when calling SetLaws(lawset.Laws, update, provider.LawUploadSound) and SetLaws(lawset.Laws, heldComp.CurrentConnectedEntity.Value, provider.LawUploadSound) (inside the TryComp<StationAiHeldComponent> / HasComp<SiliconLawProviderComponent> branch), ensure the assigned law list is cloned so mutations on one entity do not affect others; apply the same fix to the other occurrences around lines 328-337.Content.Shared/_CorvaxNext/Silicons/Borgs/SharedAiRemoteControlSystem.cs (2)
33-36: Redundant null-conditional check onremoteComp.The
TryCompon line 30 already guaranteesremoteCompis non-null when the method continues past line 31. The?.operator on line 33 is unnecessary.Suggested fix
- if (remoteComp?.AiHolder == null + if (remoteComp.AiHolder == null || !_stationAiSystem.TryGetCore(remoteComp.AiHolder.Value, out var stationAiCore) || stationAiCore.Comp?.RemoteEntity == null) return;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_CorvaxNext/Silicons/Borgs/SharedAiRemoteControlSystem.cs` around lines 33 - 36, The null-conditional operator on remoteComp in the conditional is redundant because TryComp ensures remoteComp is non-null; update the check to use remoteComp.AiHolder directly and keep the subsequent checks as-is (i.e., replace remoteComp?.AiHolder == null with remoteComp.AiHolder == null) so the condition reads against remoteComp.AiHolder and then calls _stationAiSystem.TryGetCore(remoteComp.AiHolder.Value, out var stationAiCore) and verifies stationAiCore.Comp?.RemoteEntity as before.
23-26: EmptyInitialize()override is unnecessary.Since this override only calls
base.Initialize()without additional logic, it can be removed entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_CorvaxNext/Silicons/Borgs/SharedAiRemoteControlSystem.cs` around lines 23 - 26, Remove the redundant override of Initialize in the SharedAiRemoteControlSystem class: delete the public override void Initialize() { base.Initialize(); } method so the class uses the base implementation directly (i.e., remove the empty Initialize() override in SharedAiRemoteControlSystem.cs).Content.Server/_CorvaxNext/Silicons/Borgs/AiRemoteControlSystem.cs (2)
74-87: Consider validating that the target isn't already under AI control.The verb is added without checking if the remote device is already being controlled by another AI (i.e.,
entity.Comp.AiHolder != null). This could lead to unexpected behavior if multiple AIs attempt to control the same borg simultaneously.Suggested fix
private void OnGetVerbs(Entity<AiRemoteControllerComponent> entity, ref GetVerbsEvent<AlternativeVerb> args) { var user = args.User; if (!TryComp<StationAiHeldComponent>(user, out var stationAiHeldComp)) return; + // Don't show verb if already controlled by an AI + if (entity.Comp.AiHolder != null) + return; + var verb = new AlternativeVerb { Text = Loc.GetString("ai-remote-control"), Act = () => AiTakeControl(user, entity) }; args.Verbs.Add(verb); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Server/_CorvaxNext/Silicons/Borgs/AiRemoteControlSystem.cs` around lines 74 - 87, OnGetVerbs is adding the "ai-remote-control" AlternativeVerb without checking whether the target AiRemoteControllerComponent is already controlled; update the method to inspect entity.Comp.AiHolder (or equivalent property) and skip adding the verb (or add a disabled/alternative verb) when AiHolder != null so multiple AIs cannot take control, keeping the existing call to AiTakeControl only when AiHolder is null.
141-153: Consider filtering remote devices by accessibility or distance.The current implementation queries all
AiRemoteControllerComponententities in the game regardless of their location, grid, or station. Depending on game design intent, you may want to filter devices to only those accessible to the AI (e.g., same station, powered, etc.). If this is intentional for AI omniscience, a comment explaining this design choice would be helpful.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Server/_CorvaxNext/Silicons/Borgs/AiRemoteControlSystem.cs` around lines 141 - 153, The code indiscriminately gathers every AiRemoteControllerComponent into remoteDevices (using EntityManager.EntityQueryEnumerator<AiRemoteControllerComponent> and building RemoteDevicesData via GetNetEntity and Comp<MetaDataComponent>), which may expose unreachable or irrelevant devices; update the loop to filter by the desired accessibility criteria (for example same grid/station, powered state, range/distance from the AI, or visibility) before adding to remoteDevices, using available checks (grid/transform components, PowerComponent, distance calculations, or access flags) and/or add a clear comment in the AiRemoteControllerComponent collection block explaining that global collection is intentional if omniscience is desired.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Content.Client/_CorvaxNext/Silicons/Laws/Ui/RemoteDeviceDisplay.xaml`:
- Around line 4-8: The per-device row XAML uses PanelContainer and nested
BoxContainer with VerticalExpand="True", causing rows to stretch; change the
per-row containers (PanelContainer and/or the BoxContainer in
RemoteDeviceDisplay.xaml) to not force vertical expansion—remove
VerticalExpand="True" or set VerticalExpand="False" on those elements so each
device row sizes to its content instead of growing to fill the list. Ensure
HorizontalExpand remains as needed and keep StyleClasses="BackgroundDark"
unchanged.
In `@Content.Server/_CorvaxNext/Silicons/Borgs/AiRemoteControlSystem.cs`:
- Around line 92-131: AiTakeControl must guard against taking control of an
entity already controlled by another AI: inside AiTakeControl, after obtaining
the AiRemoteControllerComponent (aiRemoteComp), check aiRemoteComp.AiHolder
(and/or aiRemoteComp.LinkedMind) and return early (or abort) if it is
non-null/assigned to avoid stomping the previous controller; do this before
altering transmitter/active radio channels, setting AiHolder/LinkedMind, or
calling _mind.ControlMob and _stationAiSystem.SwitchRemoteEntityMode so the
previous controlling AI state is preserved.
- Around line 159-182: The method OnUiRemoteAction uses GetEntity to populate
local variable target but never checks it for null before calling
HasComp<AiRemoteControllerComponent>(target) or using target.Value in
SetCoordinates and AiTakeControl; add an explicit null check after var target =
GetEntity(...) (e.g., if (target == null) return;) so HasComp,
Transform(target.Value) and AiTakeControl(uid, target.Value) only run when
target is non-null, and remove redundant null-conditional operators where
appropriate.
- Around line 60-61: The local variable backArgs (an instance of
ReturnMindIntoAiEvent) is created and assigned Performer but never used; remove
the dead code by deleting the lines that instantiate and set backArgs (the
ReturnMindIntoAiEvent backArgs = new ... and backArgs.Performer = entity lines)
and rely on the existing call to ReturnMindIntoAi(entity), or alternatively, if
intent was to raise the event, replace the direct ReturnMindIntoAi(entity) call
with the appropriate event raise using backArgs; update references in
AiRemoteControlSystem.cs accordingly.
In
`@Content.Shared/_CorvaxNext/Silicons/Borgs/Components/SharedAiRemoteControllerComponent.cs`:
- Around line 27-31: Change the RemoteAction field on RemoteDeviceActionMessage
to be non-nullable to match the constructor and strengthen the message contract:
update the declaration of RemoteAction (currently "RemoteDeviceActionEvent?
RemoteAction") to "RemoteDeviceActionEvent RemoteAction" and ensure the
RemoteDeviceActionMessage constructor still assigns the incoming
RemoteDeviceActionEvent to that field; adjust any callers or serializers if
needed to guarantee a non-null payload is always provided when instantiating
RemoteDeviceActionMessage.
In `@Content.Shared/_CorvaxNext/Silicons/Borgs/SharedAiRemoteControlSystem.cs`:
- Around line 44-52: The code updates networked component fields
(stationAiHeldComp.CurrentConnectedEntity, remoteComp.AiHolder,
remoteComp.LinkedMind) but never marks those components dirty, so clients won't
receive the updates; after setting stationAiHeldComp.CurrentConnectedEntity =
null, call stationAiHeldComp.Dirty(), and after setting remoteComp.AiHolder =
null and remoteComp.LinkedMind = null call remoteComp.Dirty() (or the
appropriate component instance Dirty() method) so the network state is
synchronized; ensure you call Dirty() on the exact component instances you
modified (stationAiHeldComp and remoteComp) immediately after their field
assignments.
In `@Content.Shared/Silicons/StationAi/SharedStationAiSystem.cs`:
- Around line 246-255: The current code calls
_remoteSystem.ReturnMindIntoAi(stationAiHeldComp.CurrentConnectedEntity.Value)
before verifying the slot transfer succeeded, which can prematurely disconnect
remote control; change the flow so you only call
_remoteSystem.ReturnMindIntoAi(...) after _slots.TryInsert(...) returns true.
Specifically, move the block that checks ent.Comp.Slot.Item, resolves
TryComp<StationAiHeldComponent> and verifies
stationAiHeldComp.CurrentConnectedEntity != null so that the call to
_remoteSystem.ReturnMindIntoAi(...) occurs only inside the success branch where
_slots.TryInsert(args.Args.Target.Value, targetHolder.Slot,
ent.Comp.Slot.Item!.Value, args.User, excludeUserAudio: true) returns true.
In `@Resources/Locale/ru-RU/_corvaxnext/silicons/ai-remote.ftl`:
- Line 14: The Russian description string .desc in ai-remote.ftl is
grammatically awkward; update its value to a natural phrase such as "Карта
управления киборгом с искусственным интеллектом." by replacing the current text
"Карта управления киборгом Искусственный Интеллектом." in the .desc entry so the
UI shows a correct, fluent Russian description.
---
Outside diff comments:
In `@Content.Shared/Silicons/Borgs/SharedBorgSystem.cs`:
- Around line 249-255: The closed-panel branch only shows the popup when a local
brain or module is present; include AI remotes too by checking for
AiRemoteBrainComponent. Update the condition in the TryComp<WiresPanelComponent>
block (where you currently test panel.Open and then "if (brain != null || module
!= null) _popup.PopupClient(...)") to also detect remote brains—e.g. extend the
condition to "if (brain != null || module != null ||
TryComp<AiRemoteBrainComponent>(chassis, out _))" and call
_popup.PopupClient(Loc.GetString("borg-panel-not-open"), chassis, args.User) in
that case so AI remote brains receive the same feedback.
---
Nitpick comments:
In `@Content.Server/_CorvaxNext/Silicons/Borgs/AiRemoteControlSystem.cs`:
- Around line 74-87: OnGetVerbs is adding the "ai-remote-control"
AlternativeVerb without checking whether the target AiRemoteControllerComponent
is already controlled; update the method to inspect entity.Comp.AiHolder (or
equivalent property) and skip adding the verb (or add a disabled/alternative
verb) when AiHolder != null so multiple AIs cannot take control, keeping the
existing call to AiTakeControl only when AiHolder is null.
- Around line 141-153: The code indiscriminately gathers every
AiRemoteControllerComponent into remoteDevices (using
EntityManager.EntityQueryEnumerator<AiRemoteControllerComponent> and building
RemoteDevicesData via GetNetEntity and Comp<MetaDataComponent>), which may
expose unreachable or irrelevant devices; update the loop to filter by the
desired accessibility criteria (for example same grid/station, powered state,
range/distance from the AI, or visibility) before adding to remoteDevices, using
available checks (grid/transform components, PowerComponent, distance
calculations, or access flags) and/or add a clear comment in the
AiRemoteControllerComponent collection block explaining that global collection
is intentional if omniscience is desired.
In `@Content.Server/Silicons/Laws/SiliconLawSystem.cs`:
- Around line 317-324: The code assigns the same mutable List<SiliconLaw>
instance (lawset.Laws) to multiple entities which can cause shared-state bugs;
update SetLaws (or the call sites in SiliconLawSystem.cs where SetLaws is
invoked) to store/assign a defensive copy (e.g., new List<SiliconLaw>(laws))
instead of the original collection. Specifically, when calling
SetLaws(lawset.Laws, update, provider.LawUploadSound) and SetLaws(lawset.Laws,
heldComp.CurrentConnectedEntity.Value, provider.LawUploadSound) (inside the
TryComp<StationAiHeldComponent> / HasComp<SiliconLawProviderComponent> branch),
ensure the assigned law list is cloned so mutations on one entity do not affect
others; apply the same fix to the other occurrences around lines 328-337.
In
`@Content.Shared/_CorvaxNext/Silicons/Borgs/Components/SharedAiRemoteControllerComponent.cs`:
- Around line 65-67: The RemoteDevicesBuiState constructor currently assigns the
caller's mutable List directly to DeviceList; change the constructor for
RemoteDevicesBuiState to defensively copy the input (e.g., create a new
List<RemoteDevicesData> from the passed deviceList or use deviceList?.ToList()
and fall back to an empty list) so future external mutations don't alter stored
BUI state; locate the RemoteDevicesBuiState(List<RemoteDevicesData> deviceList)
constructor and replace the direct assignment with a new list copy.
In `@Content.Shared/_CorvaxNext/Silicons/Borgs/SharedAiRemoteControlSystem.cs`:
- Around line 33-36: The null-conditional operator on remoteComp in the
conditional is redundant because TryComp ensures remoteComp is non-null; update
the check to use remoteComp.AiHolder directly and keep the subsequent checks
as-is (i.e., replace remoteComp?.AiHolder == null with remoteComp.AiHolder ==
null) so the condition reads against remoteComp.AiHolder and then calls
_stationAiSystem.TryGetCore(remoteComp.AiHolder.Value, out var stationAiCore)
and verifies stationAiCore.Comp?.RemoteEntity as before.
- Around line 23-26: Remove the redundant override of Initialize in the
SharedAiRemoteControlSystem class: delete the public override void Initialize()
{ base.Initialize(); } method so the class uses the base implementation directly
(i.e., remove the empty Initialize() override in
SharedAiRemoteControlSystem.cs).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
Content.Client/_CorvaxNext/Silicons/Borgs/AiRemoteControlSystem.csContent.Client/_CorvaxNext/Silicons/Laws/Ui/RemoteDeviceDisplay.xamlContent.Client/_CorvaxNext/Silicons/Laws/Ui/RemoteDeviceDisplay.xaml.csContent.Client/_CorvaxNext/Silicons/Laws/Ui/RemoteDevicesBoundUserInterface.csContent.Client/_CorvaxNext/Silicons/Laws/Ui/RemoteDevicesMenu.xamlContent.Client/_CorvaxNext/Silicons/Laws/Ui/RemoteDevicesMenu.xaml.csContent.Server/Silicons/Borgs/BorgSwitchableTypeSystem.csContent.Server/Silicons/Borgs/BorgSystem.Transponder.csContent.Server/Silicons/Laws/SiliconLawSystem.csContent.Server/_CorvaxNext/Silicons/Borgs/AiRemoteControlSystem.csContent.Shared/Robotics/RoboticsConsoleUi.csContent.Shared/Silicons/Borgs/SharedBorgSystem.csContent.Shared/Silicons/Laws/SharedSiliconLawSystem.csContent.Shared/Silicons/StationAi/SharedStationAiSystem.csContent.Shared/Silicons/StationAi/StationAiHeldComponent.csContent.Shared/_CorvaxNext/Silicons/Borgs/Components/AiRemoteBrainComponent.csContent.Shared/_CorvaxNext/Silicons/Borgs/Components/SharedAiRemoteControllerComponent.csContent.Shared/_CorvaxNext/Silicons/Borgs/SharedAiRemoteControlSystem.csResources/Locale/en-US/_corvaxnext/silicons/ai-remote.ftlResources/Locale/ru-RU/_corvaxnext/silicons/ai-remote.ftlResources/Prototypes/Entities/Mobs/Player/silicon.ymlResources/Prototypes/Recipes/Lathes/Packs/robotics.ymlResources/Prototypes/Recipes/Lathes/robot_parts.ymlResources/Prototypes/Research/experimental.ymlResources/Prototypes/_CorvaxNext/Actions/station_ai.ymlResources/Prototypes/_CorvaxNext/Entities/Objects/Specific/Robotics/airemote.yml
Comment on lines
+4
to
+8
| <PanelContainer VerticalExpand="True" StyleClasses="BackgroundDark"> | ||
| <BoxContainer Orientation="Vertical" | ||
| HorizontalExpand="True" | ||
| VerticalExpand="True" | ||
| Margin="5 5 5 5"> |
Contributor
There was a problem hiding this comment.
Avoid vertical expansion on per-device row containers.
Line 4 and Line 7 force row expansion, which can cause oversized entries in a vertical list. Prefer size-to-content rows.
Proposed fix
- <PanelContainer VerticalExpand="True" StyleClasses="BackgroundDark">
+ <PanelContainer StyleClasses="BackgroundDark">
<BoxContainer Orientation="Vertical"
HorizontalExpand="True"
- VerticalExpand="True"
Margin="5 5 5 5">📝 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
| <PanelContainer VerticalExpand="True" StyleClasses="BackgroundDark"> | |
| <BoxContainer Orientation="Vertical" | |
| HorizontalExpand="True" | |
| VerticalExpand="True" | |
| Margin="5 5 5 5"> | |
| <PanelContainer StyleClasses="BackgroundDark"> | |
| <BoxContainer Orientation="Vertical" | |
| HorizontalExpand="True" | |
| Margin="5 5 5 5"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Client/_CorvaxNext/Silicons/Laws/Ui/RemoteDeviceDisplay.xaml` around
lines 4 - 8, The per-device row XAML uses PanelContainer and nested BoxContainer
with VerticalExpand="True", causing rows to stretch; change the per-row
containers (PanelContainer and/or the BoxContainer in RemoteDeviceDisplay.xaml)
to not force vertical expansion—remove VerticalExpand="True" or set
VerticalExpand="False" on those elements so each device row sizes to its content
instead of growing to fill the list. Ensure HorizontalExpand remains as needed
and keep StyleClasses="BackgroundDark" unchanged.
Comment on lines
+60
to
+61
| var backArgs = new ReturnMindIntoAiEvent(); | ||
| backArgs.Performer = entity; |
Contributor
There was a problem hiding this comment.
Dead code: backArgs is created but never used.
The ReturnMindIntoAiEvent instance is instantiated and configured but never raised or consumed. The method calls ReturnMindIntoAi(entity) directly on line 71 instead.
Suggested fix - remove dead code
private void OnShutdown(Entity<AiRemoteControllerComponent> entity, ref ComponentShutdown args)
{
_actions.RemoveAction(entity.Owner, entity.Comp.BackToAiActionEntity);
- var backArgs = new ReturnMindIntoAiEvent();
- backArgs.Performer = entity;
-
if (TryComp(entity, out IntrinsicRadioTransmitterComponent? transmitter)
&& entity.Comp.PreviouslyTransmitterChannels != null)📝 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
| var backArgs = new ReturnMindIntoAiEvent(); | |
| backArgs.Performer = entity; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Server/_CorvaxNext/Silicons/Borgs/AiRemoteControlSystem.cs` around
lines 60 - 61, The local variable backArgs (an instance of
ReturnMindIntoAiEvent) is created and assigned Performer but never used; remove
the dead code by deleting the lines that instantiate and set backArgs (the
ReturnMindIntoAiEvent backArgs = new ... and backArgs.Performer = entity lines)
and rely on the existing call to ReturnMindIntoAi(entity), or alternatively, if
intent was to raise the event, replace the direct ReturnMindIntoAi(entity) call
with the appropriate event raise using backArgs; update references in
AiRemoteControlSystem.cs accordingly.
Comment on lines
+92
to
+131
| public void AiTakeControl(EntityUid ai, EntityUid entity) | ||
| { | ||
| if (!_mind.TryGetMind(ai, out var mindId, out var mind)) | ||
| return; | ||
|
|
||
| if (!TryComp<StationAiHeldComponent>(ai, out var stationAiHeldComp)) | ||
| return; | ||
|
|
||
| if (!TryComp<AiRemoteControllerComponent>(entity, out var aiRemoteComp)) | ||
| return; | ||
|
|
||
| if (TryComp(entity, out IntrinsicRadioTransmitterComponent? transmitter)) | ||
| { | ||
| aiRemoteComp.PreviouslyTransmitterChannels = [.. transmitter.Channels]; | ||
|
|
||
| if (TryComp(ai, out IntrinsicRadioTransmitterComponent? stationAiTransmitter)) | ||
| transmitter.Channels = [.. stationAiTransmitter.Channels]; | ||
| } | ||
|
|
||
| if (TryComp(entity, out ActiveRadioComponent? activeRadio)) | ||
| { | ||
| aiRemoteComp.PreviouslyActiveRadioChannels = [.. activeRadio.Channels]; | ||
|
|
||
| if (TryComp(ai, out ActiveRadioComponent? stationAiActiveRadio)) | ||
| activeRadio.Channels = [.. stationAiActiveRadio.Channels]; | ||
| } | ||
|
|
||
| _mind.ControlMob(ai, entity); | ||
| aiRemoteComp.AiHolder = ai; | ||
| aiRemoteComp.LinkedMind = mindId; | ||
|
|
||
| stationAiHeldComp.CurrentConnectedEntity = entity; | ||
|
|
||
| if (!_stationAiSystem.TryGetCore(ai, out var stationAiCore)) | ||
| return; | ||
|
|
||
| _stationAiSystem.SwitchRemoteEntityMode(stationAiCore, false); | ||
|
|
||
| RewriteLaws(ai, entity); | ||
| } |
Contributor
There was a problem hiding this comment.
Missing guard against taking control of an already-controlled entity.
AiTakeControl doesn't verify that the target entity isn't already being controlled by another AI (aiRemoteComp.AiHolder != null). Taking control without this check could corrupt the state of the previous controlling AI and cause desynchronization issues.
Suggested fix
public void AiTakeControl(EntityUid ai, EntityUid entity)
{
if (!_mind.TryGetMind(ai, out var mindId, out var mind))
return;
if (!TryComp<StationAiHeldComponent>(ai, out var stationAiHeldComp))
return;
if (!TryComp<AiRemoteControllerComponent>(entity, out var aiRemoteComp))
return;
+ // Prevent taking control if already controlled by another AI
+ if (aiRemoteComp.AiHolder != null)
+ return;
+
if (TryComp(entity, out IntrinsicRadioTransmitterComponent? transmitter))📝 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 AiTakeControl(EntityUid ai, EntityUid entity) | |
| { | |
| if (!_mind.TryGetMind(ai, out var mindId, out var mind)) | |
| return; | |
| if (!TryComp<StationAiHeldComponent>(ai, out var stationAiHeldComp)) | |
| return; | |
| if (!TryComp<AiRemoteControllerComponent>(entity, out var aiRemoteComp)) | |
| return; | |
| if (TryComp(entity, out IntrinsicRadioTransmitterComponent? transmitter)) | |
| { | |
| aiRemoteComp.PreviouslyTransmitterChannels = [.. transmitter.Channels]; | |
| if (TryComp(ai, out IntrinsicRadioTransmitterComponent? stationAiTransmitter)) | |
| transmitter.Channels = [.. stationAiTransmitter.Channels]; | |
| } | |
| if (TryComp(entity, out ActiveRadioComponent? activeRadio)) | |
| { | |
| aiRemoteComp.PreviouslyActiveRadioChannels = [.. activeRadio.Channels]; | |
| if (TryComp(ai, out ActiveRadioComponent? stationAiActiveRadio)) | |
| activeRadio.Channels = [.. stationAiActiveRadio.Channels]; | |
| } | |
| _mind.ControlMob(ai, entity); | |
| aiRemoteComp.AiHolder = ai; | |
| aiRemoteComp.LinkedMind = mindId; | |
| stationAiHeldComp.CurrentConnectedEntity = entity; | |
| if (!_stationAiSystem.TryGetCore(ai, out var stationAiCore)) | |
| return; | |
| _stationAiSystem.SwitchRemoteEntityMode(stationAiCore, false); | |
| RewriteLaws(ai, entity); | |
| } | |
| public void AiTakeControl(EntityUid ai, EntityUid entity) | |
| { | |
| if (!_mind.TryGetMind(ai, out var mindId, out var mind)) | |
| return; | |
| if (!TryComp<StationAiHeldComponent>(ai, out var stationAiHeldComp)) | |
| return; | |
| if (!TryComp<AiRemoteControllerComponent>(entity, out var aiRemoteComp)) | |
| return; | |
| // Prevent taking control if already controlled by another AI | |
| if (aiRemoteComp.AiHolder != null) | |
| return; | |
| if (TryComp(entity, out IntrinsicRadioTransmitterComponent? transmitter)) | |
| { | |
| aiRemoteComp.PreviouslyTransmitterChannels = [.. transmitter.Channels]; | |
| if (TryComp(ai, out IntrinsicRadioTransmitterComponent? stationAiTransmitter)) | |
| transmitter.Channels = [.. stationAiTransmitter.Channels]; | |
| } | |
| if (TryComp(entity, out ActiveRadioComponent? activeRadio)) | |
| { | |
| aiRemoteComp.PreviouslyActiveRadioChannels = [.. activeRadio.Channels]; | |
| if (TryComp(ai, out ActiveRadioComponent? stationAiActiveRadio)) | |
| activeRadio.Channels = [.. stationAiActiveRadio.Channels]; | |
| } | |
| _mind.ControlMob(ai, entity); | |
| aiRemoteComp.AiHolder = ai; | |
| aiRemoteComp.LinkedMind = mindId; | |
| stationAiHeldComp.CurrentConnectedEntity = entity; | |
| if (!_stationAiSystem.TryGetCore(ai, out var stationAiCore)) | |
| return; | |
| _stationAiSystem.SwitchRemoteEntityMode(stationAiCore, false); | |
| RewriteLaws(ai, entity); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Server/_CorvaxNext/Silicons/Borgs/AiRemoteControlSystem.cs` around
lines 92 - 131, AiTakeControl must guard against taking control of an entity
already controlled by another AI: inside AiTakeControl, after obtaining the
AiRemoteControllerComponent (aiRemoteComp), check aiRemoteComp.AiHolder (and/or
aiRemoteComp.LinkedMind) and return early (or abort) if it is non-null/assigned
to avoid stomping the previous controller; do this before altering
transmitter/active radio channels, setting AiHolder/LinkedMind, or calling
_mind.ControlMob and _stationAiSystem.SwitchRemoteEntityMode so the previous
controlling AI state is preserved.
Comment on lines
+159
to
+182
| private void OnUiRemoteAction(EntityUid uid, StationAiHeldComponent component, AiRemoteControllerComponent.RemoteDeviceActionMessage msg) | ||
| { | ||
| if (msg.RemoteAction == null) | ||
| return; | ||
|
|
||
| var target = GetEntity(msg.RemoteAction?.Target); | ||
|
|
||
| if (!HasComp<AiRemoteControllerComponent>(target)) | ||
| return; | ||
|
|
||
| switch (msg.RemoteAction?.ActionType) | ||
| { | ||
| case RemoteDeviceActionEvent.RemoteDeviceActionType.MoveToDevice: | ||
| if (!_stationAiSystem.TryGetCore(uid, out var stationAiCore) | ||
| || stationAiCore.Comp?.RemoteEntity == null) | ||
| return; | ||
| _xformSystem.SetCoordinates(stationAiCore.Comp.RemoteEntity.Value, Transform(target.Value).Coordinates); | ||
| break; | ||
|
|
||
| case RemoteDeviceActionEvent.RemoteDeviceActionType.TakeControl: | ||
| AiTakeControl(uid, target.Value); | ||
| break; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Potential null reference: validate target has a value before use.
On line 164, GetEntity could return null if the network entity is invalid. Line 166's HasComp and line 175/179's target.Value accesses assume target is valid. Add an explicit null check.
Suggested fix
private void OnUiRemoteAction(EntityUid uid, StationAiHeldComponent component, AiRemoteControllerComponent.RemoteDeviceActionMessage msg)
{
if (msg.RemoteAction == null)
return;
var target = GetEntity(msg.RemoteAction?.Target);
- if (!HasComp<AiRemoteControllerComponent>(target))
+ if (target == null || !HasComp<AiRemoteControllerComponent>(target))
return;
- switch (msg.RemoteAction?.ActionType)
+ switch (msg.RemoteAction.ActionType)
{
case RemoteDeviceActionEvent.RemoteDeviceActionType.MoveToDevice:
if (!_stationAiSystem.TryGetCore(uid, out var stationAiCore)
|| stationAiCore.Comp?.RemoteEntity == null)
return;
- _xformSystem.SetCoordinates(stationAiCore.Comp.RemoteEntity.Value, Transform(target.Value).Coordinates);
+ _xformSystem.SetCoordinates(stationAiCore.Comp.RemoteEntity.Value, Transform(target.Value).Coordinates);
break;
case RemoteDeviceActionEvent.RemoteDeviceActionType.TakeControl:
- AiTakeControl(uid, target.Value);
+ AiTakeControl(uid, target.Value);
break;
}
}📝 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
| private void OnUiRemoteAction(EntityUid uid, StationAiHeldComponent component, AiRemoteControllerComponent.RemoteDeviceActionMessage msg) | |
| { | |
| if (msg.RemoteAction == null) | |
| return; | |
| var target = GetEntity(msg.RemoteAction?.Target); | |
| if (!HasComp<AiRemoteControllerComponent>(target)) | |
| return; | |
| switch (msg.RemoteAction?.ActionType) | |
| { | |
| case RemoteDeviceActionEvent.RemoteDeviceActionType.MoveToDevice: | |
| if (!_stationAiSystem.TryGetCore(uid, out var stationAiCore) | |
| || stationAiCore.Comp?.RemoteEntity == null) | |
| return; | |
| _xformSystem.SetCoordinates(stationAiCore.Comp.RemoteEntity.Value, Transform(target.Value).Coordinates); | |
| break; | |
| case RemoteDeviceActionEvent.RemoteDeviceActionType.TakeControl: | |
| AiTakeControl(uid, target.Value); | |
| break; | |
| } | |
| } | |
| private void OnUiRemoteAction(EntityUid uid, StationAiHeldComponent component, AiRemoteControllerComponent.RemoteDeviceActionMessage msg) | |
| { | |
| if (msg.RemoteAction == null) | |
| return; | |
| var target = GetEntity(msg.RemoteAction?.Target); | |
| if (target == null || !HasComp<AiRemoteControllerComponent>(target)) | |
| return; | |
| switch (msg.RemoteAction.ActionType) | |
| { | |
| case RemoteDeviceActionEvent.RemoteDeviceActionType.MoveToDevice: | |
| if (!_stationAiSystem.TryGetCore(uid, out var stationAiCore) | |
| || stationAiCore.Comp?.RemoteEntity == null) | |
| return; | |
| _xformSystem.SetCoordinates(stationAiCore.Comp.RemoteEntity.Value, Transform(target.Value).Coordinates); | |
| break; | |
| case RemoteDeviceActionEvent.RemoteDeviceActionType.TakeControl: | |
| AiTakeControl(uid, target.Value); | |
| break; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Server/_CorvaxNext/Silicons/Borgs/AiRemoteControlSystem.cs` around
lines 159 - 182, The method OnUiRemoteAction uses GetEntity to populate local
variable target but never checks it for null before calling
HasComp<AiRemoteControllerComponent>(target) or using target.Value in
SetCoordinates and AiTakeControl; add an explicit null check after var target =
GetEntity(...) (e.g., if (target == null) return;) so HasComp,
Transform(target.Value) and AiTakeControl(uid, target.Value) only run when
target is non-null, and remove redundant null-conditional operators where
appropriate.
Comment on lines
+27
to
+31
| public readonly RemoteDeviceActionEvent? RemoteAction; | ||
| public RemoteDeviceActionMessage(RemoteDeviceActionEvent remoteDeviceAction) | ||
| { | ||
| RemoteAction = remoteDeviceAction; | ||
| } |
Contributor
There was a problem hiding this comment.
Make RemoteAction non-nullable in the message contract.
Line 27 allows null, but Line 28 requires an action payload. This mismatch weakens the protocol contract and can propagate nulls into handlers.
Proposed fix
- public readonly RemoteDeviceActionEvent? RemoteAction;
+ public readonly RemoteDeviceActionEvent RemoteAction;
public RemoteDeviceActionMessage(RemoteDeviceActionEvent remoteDeviceAction)
{
+ ArgumentNullException.ThrowIfNull(remoteDeviceAction);
RemoteAction = remoteDeviceAction;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Content.Shared/_CorvaxNext/Silicons/Borgs/Components/SharedAiRemoteControllerComponent.cs`
around lines 27 - 31, Change the RemoteAction field on RemoteDeviceActionMessage
to be non-nullable to match the constructor and strengthen the message contract:
update the declaration of RemoteAction (currently "RemoteDeviceActionEvent?
RemoteAction") to "RemoteDeviceActionEvent RemoteAction" and ensure the
RemoteDeviceActionMessage constructor still assigns the incoming
RemoteDeviceActionEvent to that field; adjust any callers or serializers if
needed to guarantee a non-null payload is always provided when instantiating
RemoteDeviceActionMessage.
Comment on lines
+44
to
+52
| stationAiHeldComp.CurrentConnectedEntity = null; | ||
|
|
||
| _mind.TransferTo(remoteComp.LinkedMind.Value, remoteComp.AiHolder); | ||
|
|
||
| _stationAiSystem.SwitchRemoteEntityMode(stationAiCore, true); | ||
| remoteComp.AiHolder = null; | ||
| remoteComp.LinkedMind = null; | ||
|
|
||
| _xformSystem.SetCoordinates(stationAiCore.Comp.RemoteEntity.Value, Transform(entity).Coordinates); |
Contributor
There was a problem hiding this comment.
Missing Dirty() calls after component state modifications.
When modifying networked component fields (stationAiHeldComp.CurrentConnectedEntity, remoteComp.AiHolder, remoteComp.LinkedMind), you should call Dirty() to ensure the changes are synchronized across the network. Without this, clients may not receive the updated state.
Suggested fix
stationAiHeldComp.CurrentConnectedEntity = null;
+ Dirty(remoteComp.AiHolder.Value, stationAiHeldComp);
_mind.TransferTo(remoteComp.LinkedMind.Value, remoteComp.AiHolder);
_stationAiSystem.SwitchRemoteEntityMode(stationAiCore, true);
remoteComp.AiHolder = null;
remoteComp.LinkedMind = null;
+ Dirty(entity, remoteComp);
_xformSystem.SetCoordinates(stationAiCore.Comp.RemoteEntity.Value, Transform(entity).Coordinates);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Shared/_CorvaxNext/Silicons/Borgs/SharedAiRemoteControlSystem.cs`
around lines 44 - 52, The code updates networked component fields
(stationAiHeldComp.CurrentConnectedEntity, remoteComp.AiHolder,
remoteComp.LinkedMind) but never marks those components dirty, so clients won't
receive the updates; after setting stationAiHeldComp.CurrentConnectedEntity =
null, call stationAiHeldComp.Dirty(), and after setting remoteComp.AiHolder =
null and remoteComp.LinkedMind = null call remoteComp.Dirty() (or the
appropriate component instance Dirty() method) so the network state is
synchronized; ensure you call Dirty() on the exact component instances you
modified (stationAiHeldComp and remoteComp) immediately after their field
assignments.
Comment on lines
+246
to
255
| if (ent.Comp.Slot.Item != null | ||
| && TryComp<StationAiHeldComponent>(ent.Comp.Slot.Item, out var stationAiHeldComp) | ||
| && stationAiHeldComp.CurrentConnectedEntity != null) | ||
| { | ||
| _remoteSystem.ReturnMindIntoAi(stationAiHeldComp.CurrentConnectedEntity.Value); | ||
| } | ||
|
|
||
| if (!_slots.TryInsert(args.Args.Target.Value, targetHolder.Slot, ent.Comp.Slot.Item!.Value, args.User, excludeUserAudio: true)) | ||
| { | ||
| return; |
Contributor
There was a problem hiding this comment.
Return remote mind only after a successful slot transfer.
Current ordering can disconnect remote control even when TryInsert fails. Move ReturnMindIntoAi(...) after successful insertion.
Proposed fix
- if (ent.Comp.Slot.Item != null
- && TryComp<StationAiHeldComponent>(ent.Comp.Slot.Item, out var stationAiHeldComp)
- && stationAiHeldComp.CurrentConnectedEntity != null)
- {
- _remoteSystem.ReturnMindIntoAi(stationAiHeldComp.CurrentConnectedEntity.Value);
- }
+ EntityUid? connectedEntity = null;
+ if (ent.Comp.Slot.Item != null &&
+ TryComp<StationAiHeldComponent>(ent.Comp.Slot.Item, out var stationAiHeldComp))
+ {
+ connectedEntity = stationAiHeldComp.CurrentConnectedEntity;
+ }
if (!_slots.TryInsert(args.Args.Target.Value, targetHolder.Slot, ent.Comp.Slot.Item!.Value, args.User, excludeUserAudio: true))
{
return;
}
+
+ if (connectedEntity != null)
+ _remoteSystem.ReturnMindIntoAi(connectedEntity.Value);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Shared/Silicons/StationAi/SharedStationAiSystem.cs` around lines 246
- 255, The current code calls
_remoteSystem.ReturnMindIntoAi(stationAiHeldComp.CurrentConnectedEntity.Value)
before verifying the slot transfer succeeded, which can prematurely disconnect
remote control; change the flow so you only call
_remoteSystem.ReturnMindIntoAi(...) after _slots.TryInsert(...) returns true.
Specifically, move the block that checks ent.Comp.Slot.Item, resolves
TryComp<StationAiHeldComponent> and verifies
stationAiHeldComp.CurrentConnectedEntity != null so that the call to
_remoteSystem.ReturnMindIntoAi(...) occurs only inside the success branch where
_slots.TryInsert(args.Args.Target.Value, targetHolder.Slot,
ent.Comp.Slot.Item!.Value, args.User, excludeUserAudio: true) returns true.
|
|
||
| # Ents | ||
| ent-AiRemoteBrain = модуль удалённого управления Б.О.Р.И.С | ||
| .desc = Карта управления киборгом Искусственный Интеллектом. |
Contributor
There was a problem hiding this comment.
Russian description has a grammar issue.
The phrase is ungrammatical and reads awkwardly in UI text.
✍️ Suggested text fix
- .desc = Карта управления киборгом Искусственный Интеллектом.
+ .desc = Карта управления киборгом искусственным интеллектом.📝 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
| .desc = Карта управления киборгом Искусственный Интеллектом. | |
| .desc = Карта управления киборгом искусственным интеллектом. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Resources/Locale/ru-RU/_corvaxnext/silicons/ai-remote.ftl` at line 14, The
Russian description string .desc in ai-remote.ftl is grammatically awkward;
update its value to a natural phrase such as "Карта управления киборгом с
искусственным интеллектом." by replacing the current text "Карта управления
киборгом Искусственный Интеллектом." in the .desc entry so the UI shows a
correct, fluent Russian description.
Collaborator
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
Content.Shared/_CorvaxNext/Silicons/Borgs/Components/SharedAiRemoteControllerComponent.cs (1)
28-32:⚠️ Potential issue | 🟡 MinorMake
RemoteActionnon-nullable in the message contract.Constructor requires an action payload, but the field allows
null, which weakens the UI message protocol.🛠️ Proposed fix
- public readonly RemoteDeviceActionEvent? RemoteAction; + public readonly RemoteDeviceActionEvent RemoteAction; public RemoteDeviceActionMessage(RemoteDeviceActionEvent remoteDeviceAction) { RemoteAction = remoteDeviceAction; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_CorvaxNext/Silicons/Borgs/Components/SharedAiRemoteControllerComponent.cs` around lines 28 - 32, The RemoteDeviceActionMessage declares RemoteAction as nullable but its constructor requires a RemoteDeviceActionEvent; change the RemoteAction field to be non-nullable to match the contract by removing the nullable marker and keeping the assignment in the RemoteDeviceActionMessage(RemoteDeviceActionEvent remoteDeviceAction) constructor so RemoteDeviceActionMessage.RemoteAction is always a valid RemoteDeviceActionEvent.Content.Server/_CorvaxNext/Silicons/Borgs/AiRemoteControlSystem.cs (1)
101-103:⚠️ Potential issue | 🟠 MajorGuard against taking control of an already-controlled remote entity.
AiTakeControlcan currently overwrite an existing controller/link, which can corrupt active remote-control state.🛡️ Proposed fix
if (!TryComp<AiRemoteControllerComponent>(entity, out var aiRemoteComp)) return; + + if (aiRemoteComp.AiHolder != null || aiRemoteComp.LinkedMind != null) + return;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Server/_CorvaxNext/Silicons/Borgs/AiRemoteControlSystem.cs` around lines 101 - 103, AiTakeControl is overwriting existing controllers because it doesn't check for an existing link on the remote; after retrieving AiRemoteControllerComponent (aiRemoteComp) you must guard against reassigning a controller by checking the component's current controller/link field (e.g., aiRemoteComp.Controller or aiRemoteComp.LinkedAi) and aborting (returning false or early-return) if it's already set. Modify AiTakeControl in AiRemoteControlSystem to first TryComp<AiRemoteControllerComponent>(entity, out var aiRemoteComp) (already present) then check the existing controller/link field and only proceed to create/assign a new controller when that field is null/unset; preserve current controller state and avoid overwriting the link. Ensure any callers handle the failure result or provide a clear rejection path.Content.Shared/_CorvaxNext/Silicons/Borgs/SharedAiRemoteControlSystem.cs (1)
45-46:⚠️ Potential issue | 🟠 MajorDirty the networked held-component after clearing connection state.
CurrentConnectedEntityis mutated but not marked dirty, so clients can retain stale remote-link state.🛠️ Proposed fix
stationAiHeldComp.CurrentConnectedEntity = null; + Dirty(remoteComp.AiHolder.Value, stationAiHeldComp); _mind.TransferTo(remoteComp.LinkedMind.Value, remoteComp.AiHolder);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Shared/_CorvaxNext/Silicons/Borgs/SharedAiRemoteControlSystem.cs` around lines 45 - 46, After clearing the connection you must mark the held-component dirty so clients receive the update; after the assignment stationAiHeldComp.CurrentConnectedEntity = null call the network dirty method (e.g., Dirty(stationAiHeldComp) or the project's equivalent entity/component dirty API) to notify clients of the mutation so they don't keep stale remote-link state.
🧹 Nitpick comments (3)
Content.Server/Silicons/Laws/SiliconLawSystem.cs (3)
294-304: Consider reducing duplication between SetLaws and SetLawsSilent.These methods share identical logic for law assignment.
SetLawscould delegate toSetLawsSilentto reduce duplication.♻️ Refactor suggestion
public void SetLaws(List<SiliconLaw> newLaws, EntityUid target, SoundSpecifier? cue = null) { - if (!TryComp<SiliconLawProviderComponent>(target, out var component)) + if (!SetLawsSilent(newLaws, target)) return; - - if (component.Lawset == null) - component.Lawset = new SiliconLawset(); - - component.Lawset.Laws = newLaws; NotifyLawsChanged(target, cue); } - public void SetLawsSilent(List<SiliconLaw> newLaws, EntityUid target, SoundSpecifier? cue = null) + public bool SetLawsSilent(List<SiliconLaw> newLaws, EntityUid target) { if (!TryComp<SiliconLawProviderComponent>(target, out var component)) - return; + return false; if (component.Lawset == null) component.Lawset = new SiliconLawset(); component.Lawset.Laws = newLaws; + return true; }Also applies to: 329-338
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Server/Silicons/Laws/SiliconLawSystem.cs` around lines 294 - 304, The SetLaws method duplicates logic in SetLawsSilent; refactor by having SetLawsSilent perform the core assignment (TryComp check, ensure component.Lawset, set component.Lawset.Laws) and then have SetLaws call SetLawsSilent(newLaws, target) and afterwards call NotifyLawsChanged(target, cue) so the cue/notification behavior remains in SetLaws; update signatures if needed to return success/failure from SetLawsSilent (or make it void) so SetLaws can early-return when TryComp fails.
69-71: Consider extracting the hardcoded tag to a constant.The tag string
"StationAi"is hardcoded. If this tag is used elsewhere or may change, extracting it to a shared constant would improve maintainability and reduce risk of typos.♻️ Suggested improvement
+ private const string StationAiTag = "StationAi"; + // In OnMindAdded: - if (HasComp<AiRemoteControllerComponent>(uid) || _tagSystem.HasTag(uid, "StationAi")) + if (HasComp<AiRemoteControllerComponent>(uid) || _tagSystem.HasTag(uid, StationAiTag))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Server/Silicons/Laws/SiliconLawSystem.cs` around lines 69 - 71, The code uses the literal tag "StationAi" in the SiliconLawSystem (in the HasComp<AiRemoteControllerComponent>(uid) || _tagSystem.HasTag(uid, "StationAi") check); extract this literal into a shared constant (e.g., private const string StationAiTag = "StationAi") in the SiliconLawSystem (or a central tag constants class if used elsewhere), replace the string literal with the constant in _tagSystem.HasTag(uid, StationAiTag), and update any other occurrences to use the same constant to avoid duplication/typos.
319-326: Verify robustness of connected entity reference.The logic correctly propagates laws to the connected Station AI. However,
CurrentConnectedEntitycould theoretically become invalid between the null check and usage if the entity is deleted in the same tick.Consider using
TryComppattern for additional safety:🛡️ More defensive approach
- if (TryComp<StationAiHeldComponent>(update, out var heldComp) - && heldComp.CurrentConnectedEntity != null - && HasComp<SiliconLawProviderComponent>(heldComp.CurrentConnectedEntity)) - { - SetLaws(lawset.Laws, heldComp.CurrentConnectedEntity.Value, provider.LawUploadSound); - } + if (TryComp<StationAiHeldComponent>(update, out var heldComp) + && heldComp.CurrentConnectedEntity is { } connectedEntity + && TryComp<SiliconLawProviderComponent>(connectedEntity, out _)) + { + SetLaws(lawset.Laws, connectedEntity, provider.LawUploadSound); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Content.Server/Silicons/Laws/SiliconLawSystem.cs` around lines 319 - 326, The null-check on heldComp.CurrentConnectedEntity is race-prone; capture the CurrentConnectedEntity into a local EntityUid after confirming it's non-null and then use TryComp (or HasComp -> TryComp) against that captured uid to ensure the entity still exists and has SiliconLawProviderComponent before calling SetLaws. Concretely, inside the TryComp<StationAiHeldComponent>(update, out var heldComp) block assign var target = heldComp.CurrentConnectedEntity.Value (after the null check) and then call TryComp<SiliconLawProviderComponent>(target, out var providerComp) (or HasComp+TryComp) — only call SetLaws(lawset.Laws, target, providerComp.LawUploadSound) if that TryComp succeeds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Content.Server/_CorvaxNext/Silicons/Borgs/AiRemoteControlSystem.cs`:
- Line 124: The CurrentConnectedEntity field on StationAiHeldComponent is
networked and must be marked dirty right after mutation; in
AiRemoteControlSystem where you set stationAiHeldComp.CurrentConnectedEntity =
entity, immediately mark the component dirty so the change replicates (e.g.,
call the component's Dirty method or use EntityManager.DirtyComponent / the
project's equivalent to dirty StationAiHeldComponent for that entity).
In `@Content.Server/Silicons/Laws/SiliconLawSystem.cs`:
- Around line 329-338: Remove the unused SoundSpecifier? cue parameter from the
SetLawsSilent method signature and its declaration (method
SetLawsSilent(List<SiliconLaw> newLaws, EntityUid target) in SiliconLawSystem),
and update all call sites to stop passing a cue; if any callers relied on
overloads, replace calls with the new two-argument form. Ensure the method body
remains unchanged (still checks TryComp<SiliconLawProviderComponent>,
initializes Lawset if null, and assigns component.Lawset.Laws = newLaws) and run
a compile to find and fix any remaining references or tests expecting the old
signature.
---
Duplicate comments:
In `@Content.Server/_CorvaxNext/Silicons/Borgs/AiRemoteControlSystem.cs`:
- Around line 101-103: AiTakeControl is overwriting existing controllers because
it doesn't check for an existing link on the remote; after retrieving
AiRemoteControllerComponent (aiRemoteComp) you must guard against reassigning a
controller by checking the component's current controller/link field (e.g.,
aiRemoteComp.Controller or aiRemoteComp.LinkedAi) and aborting (returning false
or early-return) if it's already set. Modify AiTakeControl in
AiRemoteControlSystem to first TryComp<AiRemoteControllerComponent>(entity, out
var aiRemoteComp) (already present) then check the existing controller/link
field and only proceed to create/assign a new controller when that field is
null/unset; preserve current controller state and avoid overwriting the link.
Ensure any callers handle the failure result or provide a clear rejection path.
In
`@Content.Shared/_CorvaxNext/Silicons/Borgs/Components/SharedAiRemoteControllerComponent.cs`:
- Around line 28-32: The RemoteDeviceActionMessage declares RemoteAction as
nullable but its constructor requires a RemoteDeviceActionEvent; change the
RemoteAction field to be non-nullable to match the contract by removing the
nullable marker and keeping the assignment in the
RemoteDeviceActionMessage(RemoteDeviceActionEvent remoteDeviceAction)
constructor so RemoteDeviceActionMessage.RemoteAction is always a valid
RemoteDeviceActionEvent.
In `@Content.Shared/_CorvaxNext/Silicons/Borgs/SharedAiRemoteControlSystem.cs`:
- Around line 45-46: After clearing the connection you must mark the
held-component dirty so clients receive the update; after the assignment
stationAiHeldComp.CurrentConnectedEntity = null call the network dirty method
(e.g., Dirty(stationAiHeldComp) or the project's equivalent entity/component
dirty API) to notify clients of the mutation so they don't keep stale
remote-link state.
---
Nitpick comments:
In `@Content.Server/Silicons/Laws/SiliconLawSystem.cs`:
- Around line 294-304: The SetLaws method duplicates logic in SetLawsSilent;
refactor by having SetLawsSilent perform the core assignment (TryComp check,
ensure component.Lawset, set component.Lawset.Laws) and then have SetLaws call
SetLawsSilent(newLaws, target) and afterwards call NotifyLawsChanged(target,
cue) so the cue/notification behavior remains in SetLaws; update signatures if
needed to return success/failure from SetLawsSilent (or make it void) so SetLaws
can early-return when TryComp fails.
- Around line 69-71: The code uses the literal tag "StationAi" in the
SiliconLawSystem (in the HasComp<AiRemoteControllerComponent>(uid) ||
_tagSystem.HasTag(uid, "StationAi") check); extract this literal into a shared
constant (e.g., private const string StationAiTag = "StationAi") in the
SiliconLawSystem (or a central tag constants class if used elsewhere), replace
the string literal with the constant in _tagSystem.HasTag(uid, StationAiTag),
and update any other occurrences to use the same constant to avoid
duplication/typos.
- Around line 319-326: The null-check on heldComp.CurrentConnectedEntity is
race-prone; capture the CurrentConnectedEntity into a local EntityUid after
confirming it's non-null and then use TryComp (or HasComp -> TryComp) against
that captured uid to ensure the entity still exists and has
SiliconLawProviderComponent before calling SetLaws. Concretely, inside the
TryComp<StationAiHeldComponent>(update, out var heldComp) block assign var
target = heldComp.CurrentConnectedEntity.Value (after the null check) and then
call TryComp<SiliconLawProviderComponent>(target, out var providerComp) (or
HasComp+TryComp) — only call SetLaws(lawset.Laws, target,
providerComp.LawUploadSound) if that TryComp succeeds.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
Content.Client/_CorvaxNext/Silicons/Borgs/AiRemoteControlSystem.csContent.Client/_CorvaxNext/Silicons/Laws/Ui/RemoteDeviceDisplay.xaml.csContent.Client/_CorvaxNext/Silicons/Laws/Ui/RemoteDevicesBoundUserInterface.csContent.Client/_CorvaxNext/Silicons/Laws/Ui/RemoteDevicesMenu.xaml.csContent.Server/Silicons/Borgs/BorgSwitchableTypeSystem.csContent.Server/Silicons/Borgs/BorgSystem.Transponder.csContent.Server/Silicons/Laws/SiliconLawSystem.csContent.Server/_CorvaxNext/Silicons/Borgs/AiRemoteControlSystem.csContent.Shared/Robotics/RoboticsConsoleUi.csContent.Shared/Silicons/Borgs/SharedBorgSystem.csContent.Shared/Silicons/Laws/SharedSiliconLawSystem.csContent.Shared/Silicons/StationAi/SharedStationAiSystem.csContent.Shared/Silicons/StationAi/StationAiHeldComponent.csContent.Shared/_CorvaxNext/Silicons/Borgs/Components/AiRemoteBrainComponent.csContent.Shared/_CorvaxNext/Silicons/Borgs/Components/SharedAiRemoteControllerComponent.csContent.Shared/_CorvaxNext/Silicons/Borgs/SharedAiRemoteControlSystem.cs
🚧 Files skipped from review as they are similar to previous changes (7)
- Content.Client/_CorvaxNext/Silicons/Laws/Ui/RemoteDevicesMenu.xaml.cs
- Content.Shared/Silicons/Laws/SharedSiliconLawSystem.cs
- Content.Shared/Robotics/RoboticsConsoleUi.cs
- Content.Shared/Silicons/StationAi/SharedStationAiSystem.cs
- Content.Shared/_CorvaxNext/Silicons/Borgs/Components/AiRemoteBrainComponent.cs
- Content.Shared/Silicons/Borgs/SharedBorgSystem.cs
- Content.Server/Silicons/Borgs/BorgSystem.Transponder.cs
| aiRemoteComp.AiHolder = ai; | ||
| aiRemoteComp.LinkedMind = mindId; | ||
|
|
||
| stationAiHeldComp.CurrentConnectedEntity = entity; |
Contributor
There was a problem hiding this comment.
Dirty StationAiHeldComponent after setting current connected entity.
CurrentConnectedEntity is networked state and should be dirtied immediately after mutation.
🛠️ Proposed fix
stationAiHeldComp.CurrentConnectedEntity = entity;
+ Dirty(ai, stationAiHeldComp);📝 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
| stationAiHeldComp.CurrentConnectedEntity = entity; | |
| stationAiHeldComp.CurrentConnectedEntity = entity; | |
| Dirty(ai, stationAiHeldComp); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Server/_CorvaxNext/Silicons/Borgs/AiRemoteControlSystem.cs` at line
124, The CurrentConnectedEntity field on StationAiHeldComponent is networked and
must be marked dirty right after mutation; in AiRemoteControlSystem where you
set stationAiHeldComp.CurrentConnectedEntity = entity, immediately mark the
component dirty so the change replicates (e.g., call the component's Dirty
method or use EntityManager.DirtyComponent / the project's equivalent to dirty
StationAiHeldComponent for that entity).
Comment on lines
+329
to
+338
| public void SetLawsSilent(List<SiliconLaw> newLaws, EntityUid target, SoundSpecifier? cue = null) | ||
| { | ||
| if (!TryComp<SiliconLawProviderComponent>(target, out var component)) | ||
| return; | ||
|
|
||
| if (component.Lawset == null) | ||
| component.Lawset = new SiliconLawset(); | ||
|
|
||
| component.Lawset.Laws = newLaws; | ||
| } |
Contributor
There was a problem hiding this comment.
Unused cue parameter in SetLawsSilent.
The cue parameter is never used in this method. Since the purpose is to set laws without notification, the parameter is misleading and should be removed.
🐛 Remove unused parameter
- public void SetLawsSilent(List<SiliconLaw> newLaws, EntityUid target, SoundSpecifier? cue = null)
+ public void SetLawsSilent(List<SiliconLaw> newLaws, EntityUid target)
{
if (!TryComp<SiliconLawProviderComponent>(target, out var component))
return;
if (component.Lawset == null)
component.Lawset = new SiliconLawset();
component.Lawset.Laws = newLaws;
}📝 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 SetLawsSilent(List<SiliconLaw> newLaws, EntityUid target, SoundSpecifier? cue = null) | |
| { | |
| if (!TryComp<SiliconLawProviderComponent>(target, out var component)) | |
| return; | |
| if (component.Lawset == null) | |
| component.Lawset = new SiliconLawset(); | |
| component.Lawset.Laws = newLaws; | |
| } | |
| public void SetLawsSilent(List<SiliconLaw> newLaws, EntityUid target) | |
| { | |
| if (!TryComp<SiliconLawProviderComponent>(target, out var component)) | |
| return; | |
| if (component.Lawset == null) | |
| component.Lawset = new SiliconLawset(); | |
| component.Lawset.Laws = newLaws; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Content.Server/Silicons/Laws/SiliconLawSystem.cs` around lines 329 - 338,
Remove the unused SoundSpecifier? cue parameter from the SetLawsSilent method
signature and its declaration (method SetLawsSilent(List<SiliconLaw> newLaws,
EntityUid target) in SiliconLawSystem), and update all call sites to stop
passing a cue; if any callers relied on overloads, replace calls with the new
two-argument form. Ensure the method body remains unchanged (still checks
TryComp<SiliconLawProviderComponent>, initializes Lawset if null, and assigns
component.Lawset.Laws = newLaws) and run a compile to find and fix any remaining
references or tests expecting the old signature.
ren0san
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Описание PR
Портирована механика удаленного управления боргами для ИИ (B.O.R.I.S) из PR #2745 в текущую кодовую базу WL.
Добавлены:
Дополнительно внесен follow-up фикс по action'ам удаленного управления.
Почему / Баланс
Изменение закрывает предложение, которое требует готовой реализации в коде, и позволяет полноценно тестировать механику в игре.
По балансу:
Технические детали
Content.Client/_CorvaxNext/Silicons/...Content.Server/_CorvaxNext/Silicons/...Content.Shared/_CorvaxNext/Silicons/...Content.Server/Silicons/Borgs/*Content.Server/Silicons/Laws/SiliconLawSystem.csContent.Shared/Silicons/*Content.Shared/Robotics/RoboticsConsoleUi.cs_CorvaxNext/Actions/station_ai.yml_CorvaxNext/Entities/.../airemote.ymlResources/Locale/en-US/_corvaxnext/silicons/ai-remote.ftlResources/Locale/ru-RU/_corvaxnext/silicons/ai-remote.ftl//WL-Changesдобавлены в затронутые C# файлы.Медиа
При необходимости приложу видео/скрин интерфейса удаленного управления в игре.
Требования
Согласие с условиями
Критические изменения
Критических API-breaking изменений (переименований публичных типов/прототипов с обязательной миграцией внешнего кода) в рамках этого PR не заявлено.
Список изменений
🆑