Try fix invalid entities in device networks (#22845)

* Try fix invalid entities in device networks

* more fixes

* a

* fix device merging

Content.Client/NetworkConfigurator/Systems/NetworkConfiguratorSystem.cs

@@ -102,17 +102,6 @@ public sealed class NetworkConfiguratorSystem : SharedNetworkConfiguratorSystem
         _overlay.RemoveOverlay(overlay);
     }
 
-    // hacky solution related to mapping
-    public void SetActiveDeviceList(EntityUid tool, EntityUid list, NetworkConfiguratorComponent? component = null)
-    {
-        if (!Resolve(tool, ref component))
-        {
-            return;
-        }
-
-        component.ActiveDeviceList = list;
-    }
-
     private sealed class StatusControl : Control
     {
         private readonly RichTextLabel _label;

Content.Server/DeviceNetwork/Components/DeviceNetworkComponent.cs

@@ -103,11 +103,17 @@ namespace Content.Server.DeviceNetwork.Components
         public bool SendBroadcastAttemptEvent = false;
 
         /// <summary>
-        ///     A list of entities that get sent the <see cref="DeviceShutDownEvent"/> when this entity gets deleted.<br/><br/>
+        ///     A list of device-lists that this device is on.
-        ///     When a device subscribes to the deletion of another device the entity id of the device being subscribed
-        ///     to also gets saved on the subscribing device.
         /// </summary>
-        [DataField("ShutdownSubscribers")]
+        [DataField]
-        public HashSet<EntityUid> ShutdownSubscribers = new();
+        [Access(typeof(DeviceListSystem))]
+        public HashSet<EntityUid> DeviceLists = new();
+
+        /// <summary>
+        ///     A list of configurators that this device is on.
+        /// </summary>
+        [DataField]
+        [Access(typeof(NetworkConfiguratorSystem))]
+        public HashSet<EntityUid> Configurators = new();
     }
 }

Content.Server/DeviceNetwork/Systems/DeviceListSystem.cs

@@ -14,22 +14,32 @@ public sealed class DeviceListSystem : SharedDeviceListSystem
 {
     private ISawmill _sawmill = default!;
 
-    [Dependency] private DeviceNetworkSystem _deviceNetworkSystem = null!;
+    [Dependency] private readonly NetworkConfiguratorSystem _configurator = default!;
 
     public override void Initialize()
     {
         base.Initialize();
-        SubscribeLocalEvent<DeviceListComponent, ComponentInit>(OnInit);
+        SubscribeLocalEvent<DeviceListComponent, ComponentShutdown>(OnShutdown);
         SubscribeLocalEvent<DeviceListComponent, BeforeBroadcastAttemptEvent>(OnBeforeBroadcast);
         SubscribeLocalEvent<DeviceListComponent, BeforePacketSentEvent>(OnBeforePacketSent);
-        SubscribeLocalEvent<DeviceListComponent, DeviceShutDownEvent>(OnDeviceShutdown);
         SubscribeLocalEvent<BeforeSaveEvent>(OnMapSave);
         _sawmill = Logger.GetSawmill("devicelist");
     }
 
-    public void OnInit(EntityUid uid, DeviceListComponent component, ComponentInit args)
+    private void OnShutdown(EntityUid uid, DeviceListComponent component, ComponentShutdown args)
     {
-        Dirty(component);
+        foreach (var conf in component.Configurators)
+        {
+            _configurator.OnDeviceListShutdown(conf, (uid, component));
+        }
+
+        var query = GetEntityQuery<DeviceNetworkComponent>();
+        foreach (var device in component.Devices)
+        {
+            if (query.TryGetComponent(device, out var comp))
+                comp.DeviceLists.Remove(uid);
+        }
+        component.Devices.Clear();
     }
 
     /// <summary>
@@ -74,20 +84,6 @@ public sealed class DeviceListSystem : SharedDeviceListSystem
         return addresses.Contains(address);
     }
 
-    protected override void UpdateShutdownSubscription(EntityUid uid, List<EntityUid> newDevices, List<EntityUid> oldDevices)
-    {
-        foreach (var device in newDevices)
-        {
-            _deviceNetworkSystem.SubscribeToDeviceShutdown(uid, device);
-        }
-
-        var removedDevices = oldDevices.Except(newDevices);
-        foreach (var device in removedDevices)
-        {
-            _deviceNetworkSystem.UnsubscribeFromDeviceShutdown(uid, device);
-        }
-    }
-
     /// <summary>
     /// Filters the broadcasts recipient list against the device list as either an allow or deny list depending on the components IsAllowList field
     /// </summary>
@@ -119,10 +115,14 @@ public sealed class DeviceListSystem : SharedDeviceListSystem
             args.Cancel();
     }
 
-    private void OnDeviceShutdown(EntityUid uid, DeviceListComponent component, ref DeviceShutDownEvent args)
+    public void OnDeviceShutdown(Entity<DeviceListComponent?> list, Entity<DeviceNetworkComponent> device)
     {
-        component.Devices.Remove(args.ShutDownEntityUid);
+        device.Comp.DeviceLists.Remove(list.Owner);
-        Dirty(component);
+        if (!Resolve(list.Owner, ref list.Comp))
+            return;
+
+        list.Comp.Devices.Remove(device);
+        Dirty(list);
     }
 
     private void OnMapSave(BeforeSaveEvent ev)
@@ -166,4 +166,57 @@ public sealed class DeviceListSystem : SharedDeviceListSystem
             toRemove.Clear();
         }
     }
+
+    /// <summary>
+    ///     Updates the device list stored on this entity.
+    /// </summary>
+    /// <param name="uid">The entity to update.</param>
+    /// <param name="devices">The devices to store.</param>
+    /// <param name="merge">Whether to merge or replace the devices stored.</param>
+    /// <param name="deviceList">Device list component</param>
+    public DeviceListUpdateResult UpdateDeviceList(EntityUid uid, IEnumerable<EntityUid> devices, bool merge = false, DeviceListComponent? deviceList = null)
+    {
+        if (!Resolve(uid, ref deviceList))
+            return DeviceListUpdateResult.NoComponent;
+
+        var list = devices.ToList();
+        var newDevices = new HashSet<EntityUid>(list);
+
+        if (merge)
+            newDevices.UnionWith(deviceList.Devices);
+
+        if (newDevices.Count > deviceList.DeviceLimit)
+        {
+            return DeviceListUpdateResult.TooManyDevices;
+        }
+
+        var query = GetEntityQuery<DeviceNetworkComponent>();
+        var oldDevices = deviceList.Devices.ToList();
+        foreach (var device in oldDevices)
+        {
+            if (newDevices.Contains(device))
+                continue;
+
+            deviceList.Devices.Remove(device);
+            if (query.TryGetComponent(device, out var comp))
+                comp.DeviceLists.Remove(uid);
+        }
+
+        foreach (var device in newDevices)
+        {
+            if (!query.TryGetComponent(device, out var comp))
+                continue;
+
+            if (!deviceList.Devices.Add(device))
+                continue;
+
+            comp.DeviceLists.Add(uid);
+        }
+
+        RaiseLocalEvent(uid, new DeviceListUpdateEvent(oldDevices, list));
+
+        Dirty(uid, deviceList);
+
+        return DeviceListUpdateResult.UpdateOk;
+    }
 }

Content.Server/DeviceNetwork/Systems/DeviceNetworkSystem.cs

@@ -6,6 +6,7 @@ using Robust.Shared.Random;
 using System.Buffers;
 using System.Diagnostics.CodeAnalysis;
 using System.Numerics;
+using Content.Shared.DeviceNetwork.Components;
 using Content.Shared.Examine;
 
 namespace Content.Server.DeviceNetwork.Systems
@@ -20,6 +21,8 @@ namespace Content.Server.DeviceNetwork.Systems
         [Dependency] private readonly IRobustRandom _random = default!;
         [Dependency] private readonly IPrototypeManager _protoMan = default!;
         [Dependency] private readonly SharedTransformSystem _transformSystem = default!;
+        [Dependency] private readonly DeviceListSystem _deviceLists = default!;
+        [Dependency] private readonly NetworkConfiguratorSystem _configurator = default!;
 
         private readonly Dictionary<int, DeviceNet> _networks = new(4);
         private readonly Queue<DeviceNetworkPacketEvent> _queueA = new();
@@ -143,15 +146,14 @@ namespace Content.Server.DeviceNetwork.Systems
         /// </summary>
         private void OnNetworkShutdown(EntityUid uid, DeviceNetworkComponent component, ComponentShutdown args)
         {
-            var eventArgs = new DeviceShutDownEvent(uid);
+            foreach (var list in component.DeviceLists)
-
-            foreach (var shutdownSubscriberId in component.ShutdownSubscribers)
             {
-                RaiseLocalEvent(shutdownSubscriberId, ref eventArgs);
+                _deviceLists.OnDeviceShutdown(list, (uid, component));
+            }
 
-                DeviceNetworkComponent? device = null!;
+            foreach (var list in component.Configurators)
-                if (Resolve(shutdownSubscriberId, ref device))
+            {
-                    device.ShutdownSubscribers.Remove(uid);
+                _configurator.OnDeviceShutdown(list, (uid, component));
             }
 
             GetNetwork(component.DeviceNetId).Remove(component);
@@ -267,36 +269,6 @@ namespace Content.Server.DeviceNetwork.Systems
             deviceNet.Add(device);
         }
 
-        public void SubscribeToDeviceShutdown(
-            EntityUid subscriberId, EntityUid targetId,
-            DeviceNetworkComponent? subscribingDevice = null,
-            DeviceNetworkComponent? targetDevice = null)
-        {
-            if (subscriberId == targetId)
-                return;
-
-            if (!Resolve(subscriberId, ref subscribingDevice) || !Resolve(targetId, ref targetDevice))
-                return;
-
-            targetDevice.ShutdownSubscribers.Add(subscriberId);
-            subscribingDevice.ShutdownSubscribers.Add(targetId);
-        }
-
-        public void UnsubscribeFromDeviceShutdown(
-            EntityUid subscriberId, EntityUid targetId,
-            DeviceNetworkComponent? subscribingDevice = null,
-            DeviceNetworkComponent? targetDevice = null)
-        {
-            if (subscriberId == targetId)
-                return;
-
-            if (!Resolve(subscriberId, ref subscribingDevice) || !Resolve(targetId, ref targetDevice))
-                return;
-
-            targetDevice.ShutdownSubscribers.Remove(subscriberId);
-            subscribingDevice.ShutdownSubscribers.Remove(targetId);
-        }
-
         /// <summary>
         ///     Try to find a device on a network using its address.
         /// </summary>
@@ -481,11 +453,4 @@ namespace Content.Server.DeviceNetwork.Systems
             Data = data;
         }
     }
-
-    /// <summary>
-    /// Gets raised on entities that subscribed to shutdown event of the shut down entity
-    /// </summary>
-    /// <param name="ShutDownEntityUid">The entity that was shut down</param>
-    [ByRefEvent]
-    public readonly record struct DeviceShutDownEvent(EntityUid ShutDownEntityUid);
 }

Content.Server/DeviceNetwork/Systems/NetworkConfiguratorSystem.cs

@@ -43,6 +43,7 @@ public sealed class NetworkConfiguratorSystem : SharedNetworkConfiguratorSystem
         base.Initialize();
 
         SubscribeLocalEvent<NetworkConfiguratorComponent, MapInitEvent>(OnMapInit);
+        SubscribeLocalEvent<NetworkConfiguratorComponent, ComponentShutdown>(OnShutdown);
 
         //Interaction
         SubscribeLocalEvent<NetworkConfiguratorComponent, AfterInteractEvent>(AfterInteract); //TODO: Replace with utility verb?
@@ -66,6 +67,15 @@ public sealed class NetworkConfiguratorSystem : SharedNetworkConfiguratorSystem
         SubscribeLocalEvent<DeviceListComponent, ComponentRemove>(OnComponentRemoved);
     }
 
+    private void OnShutdown(EntityUid uid, NetworkConfiguratorComponent component, ComponentShutdown args)
+    {
+        ClearDevices(uid, component);
+
+        if (TryComp(component.ActiveDeviceList, out DeviceListComponent? list))
+            list.Configurators.Remove(uid);
+        component.ActiveDeviceList = null;
+    }
+
     public override void Update(float frameTime)
     {
         base.Update(frameTime);
@@ -85,7 +95,6 @@ public sealed class NetworkConfiguratorSystem : SharedNetworkConfiguratorSystem
 
     private void OnMapInit(EntityUid uid, NetworkConfiguratorComponent component, MapInitEvent args)
     {
-        component.Devices.Clear();
         UpdateListUiState(uid, component);
     }
 
@@ -131,6 +140,7 @@ public sealed class NetworkConfiguratorSystem : SharedNetworkConfiguratorSystem
             return;
         }
 
+        device.Configurators.Add(configuratorUid);
         configurator.Devices.Add(address, targetUid.Value);
         _popupSystem.PopupCursor(Loc.GetString("network-configurator-device-saved", ("address", device.Address), ("device", targetUid)),
             userUid, PopupType.Medium);
@@ -462,14 +472,21 @@ public sealed class NetworkConfiguratorSystem : SharedNetworkConfiguratorSystem
     /// </summary>
     private void OpenDeviceListUi(EntityUid configuratorUid, EntityUid? targetUid, EntityUid userUid, NetworkConfiguratorComponent configurator)
     {
+        if (configurator.ActiveDeviceLink == targetUid)
+            return;
+
         if (Delay(configurator))
             return;
 
         if (!targetUid.HasValue || !TryComp(userUid, out ActorComponent? actor) || !AccessCheck(targetUid.Value, userUid, configurator))
             return;
 
+        if (!TryComp(targetUid, out DeviceListComponent? list))
+            return;
+
+        list.Configurators.Add(configuratorUid);
         configurator.ActiveDeviceList = targetUid;
-        Dirty(configurator);
+        Dirty(configuratorUid, configurator);
 
         if (!_uiSystem.TryGetUi(configuratorUid, NetworkConfiguratorUiKey.Configure, out var bui))
             return;
@@ -516,6 +533,10 @@ public sealed class NetworkConfiguratorSystem : SharedNetworkConfiguratorSystem
     private void OnUiClosed(EntityUid uid, NetworkConfiguratorComponent component, BoundUIClosedEvent args)
     {
         component.ActiveDeviceList = null;
+        if (TryComp(component.ActiveDeviceList, out DeviceListComponent? list))
+        {
+            list.Configurators.Remove(uid);
+        }
 
         if (args.UiKey is NetworkConfiguratorUiKey.Link)
         {
@@ -524,15 +545,28 @@ public sealed class NetworkConfiguratorSystem : SharedNetworkConfiguratorSystem
         }
     }
 
+    public void OnDeviceListShutdown(Entity<NetworkConfiguratorComponent?> conf, Entity<DeviceListComponent> list)
+    {
+        list.Comp.Configurators.Remove(conf.Owner);
+        if (Resolve(conf.Owner, ref conf.Comp))
+            conf.Comp.ActiveDeviceList = null;
+    }
+
     /// <summary>
     /// Removes a device from the saved devices list
     /// </summary>
     private void OnRemoveDevice(EntityUid uid, NetworkConfiguratorComponent component, NetworkConfiguratorRemoveDeviceMessage args)
     {
         if (component.Devices.TryGetValue(args.Address, out var removedDevice) && args.Session.AttachedEntity != null)
+        {
             _adminLogger.Add(LogType.DeviceLinking, LogImpact.Low,
                 $"{ToPrettyString(args.Session.AttachedEntity.Value):actor} removed buffered device {ToPrettyString(removedDevice):subject} from {ToPrettyString(uid):tool}");
+        }
+
         component.Devices.Remove(args.Address);
+        if (TryComp(removedDevice, out DeviceNetworkComponent? device))
+            device.Configurators.Remove(uid);
+
         UpdateListUiState(uid, component);
     }
 
@@ -544,10 +578,24 @@ public sealed class NetworkConfiguratorSystem : SharedNetworkConfiguratorSystem
         if (args.Session.AttachedEntity != null)
             _adminLogger.Add(LogType.DeviceLinking, LogImpact.Low,
                 $"{ToPrettyString(args.Session.AttachedEntity.Value):actor} cleared buffered devices from {ToPrettyString(uid):tool}");
-        component.Devices.Clear();
+
+
+        ClearDevices(uid, component);
         UpdateListUiState(uid, component);
     }
 
+    private void ClearDevices(EntityUid uid, NetworkConfiguratorComponent component)
+    {
+        var query = GetEntityQuery<DeviceNetworkComponent>();
+        foreach (var device in component.Devices.Values)
+        {
+            if (query.TryGetComponent(device, out var comp))
+                comp.Configurators.Remove(uid);
+        }
+
+        component.Devices.Clear();
+    }
+
     private void OnClearLinks(EntityUid uid, NetworkConfiguratorComponent configurator, NetworkConfiguratorClearLinksMessage args)
     {
         if (!configurator.ActiveDeviceLink.HasValue || !configurator.DeviceLinkTarget.HasValue)
@@ -702,7 +750,18 @@ public sealed class NetworkConfiguratorSystem : SharedNetworkConfiguratorSystem
                 if (args.Session.AttachedEntity != null)
                     _adminLogger.Add(LogType.DeviceLinking, LogImpact.Low,
                         $"{ToPrettyString(args.Session.AttachedEntity.Value):actor} copied devices from {ToPrettyString(component.ActiveDeviceList.Value):subject} to {ToPrettyString(uid):tool}");
-                component.Devices = _deviceListSystem.GetDeviceList(component.ActiveDeviceList.Value);
+
+                ClearDevices(uid, component);
+
+                var query = GetEntityQuery<DeviceNetworkComponent>();
+                foreach (var (addr, device) in _deviceListSystem.GetDeviceList(component.ActiveDeviceList.Value))
+                {
+                    if (query.TryGetComponent(device, out var comp))
+                    {
+                        component.Devices[addr] = device;
+                        comp.Configurators.Add(uid);
+                    }
+                }
                 UpdateListUiState(uid, component);
                 return;
             case NetworkConfiguratorButtonKey.Show:
@@ -725,6 +784,21 @@ public sealed class NetworkConfiguratorSystem : SharedNetworkConfiguratorSystem
                     .Select(v => (v.Key, MetaData(v.Value).EntityName)).ToHashSet()));
     }
 
+    public void OnDeviceShutdown(Entity<NetworkConfiguratorComponent?> conf, Entity<DeviceNetworkComponent> device)
+    {
+        device.Comp.Configurators.Remove(conf.Owner);
+        if (!Resolve(conf.Owner, ref conf.Comp))
+            return;
+
+        foreach (var (addr, dev) in conf.Comp.Devices)
+        {
+            if (device.Owner == dev)
+                conf.Comp.Devices.Remove(addr);
+        }
+
+        UpdateListUiState(conf, conf.Comp);
+    }
+
     private void OnUiOpenAttempt(EntityUid uid, NetworkConfiguratorComponent configurator, ActivatableUIOpenAttemptEvent args)
     {
         if (configurator.LinkModeActive)

Content.Shared/DeviceNetwork/Components/DeviceListComponent.cs

@@ -33,4 +33,7 @@ public sealed partial class DeviceListComponent : Component
     [ViewVariables(VVAccess.ReadWrite)]
     [DataField, AutoNetworkedField]
     public bool HandleIncomingPackets;
+
+    [DataField, Access(typeof(SharedNetworkConfiguratorSystem))]
+    public HashSet<EntityUid> Configurators = new();
 }

Content.Shared/DeviceNetwork/Components/NetworkConfiguratorComponent.cs

@@ -10,6 +10,7 @@ namespace Content.Shared.DeviceNetwork.Components;
 [Access(typeof(SharedNetworkConfiguratorSystem))]
 public sealed partial class NetworkConfiguratorComponent : Component
 {
+    // AAAAA ALL OF THESE FAA
     /// <summary>
     /// Determines whether the configurator is in linking mode or list mode
     /// </summary>
@@ -21,19 +22,19 @@ public sealed partial class NetworkConfiguratorComponent : Component
     /// The entity containing a <see cref="DeviceListComponent"/> this configurator is currently interacting with
     /// </summary>
     [DataField, AutoNetworkedField]
-    public EntityUid? ActiveDeviceList;
+    public EntityUid? ActiveDeviceList { get; set; }
 
     /// <summary>
     /// The entity containing a <see cref="DeviceLinkSourceComponent"/> or <see cref="DeviceLinkSinkComponent"/> this configurator is currently interacting with.<br/>
     /// If this is set the configurator is in linking mode.
     /// </summary>
-    [DataField]
+    // TODO handle device deletion
     public EntityUid? ActiveDeviceLink;
 
     /// <summary>
     /// The target device this configurator is currently linking with the <see cref="ActiveDeviceLink"/>
     /// </summary>
-    [DataField]
+    // TODO handle device deletion
     public EntityUid? DeviceLinkTarget;
 
     /// <summary>

Content.Shared/DeviceNetwork/Systems/SharedDeviceListSystem.cs

@@ -5,39 +5,6 @@ namespace Content.Shared.DeviceNetwork.Systems;
 
 public abstract class SharedDeviceListSystem : EntitySystem
 {
-    /// <summary>
-    ///     Updates the device list stored on this entity.
-    /// </summary>
-    /// <param name="uid">The entity to update.</param>
-    /// <param name="devices">The devices to store.</param>
-    /// <param name="merge">Whether to merge or replace the devices stored.</param>
-    /// <param name="deviceList">Device list component</param>
-    public DeviceListUpdateResult UpdateDeviceList(EntityUid uid, IEnumerable<EntityUid> devices, bool merge = false, DeviceListComponent? deviceList = null)
-    {
-        if (!Resolve(uid, ref deviceList))
-            return DeviceListUpdateResult.NoComponent;
-
-        var oldDevices = deviceList.Devices.ToList();
-        var newDevices = merge ? new HashSet<EntityUid>(deviceList.Devices) : new();
-        var devicesList = devices.ToList();
-
-        newDevices.UnionWith(devicesList);
-        if (newDevices.Count > deviceList.DeviceLimit)
-        {
-            return DeviceListUpdateResult.TooManyDevices;
-        }
-
-        deviceList.Devices = newDevices;
-
-        UpdateShutdownSubscription(uid, devicesList, oldDevices);
-
-        RaiseLocalEvent(uid, new DeviceListUpdateEvent(oldDevices, devicesList));
-
-        Dirty(deviceList);
-
-        return DeviceListUpdateResult.UpdateOk;
-    }
-
     public IEnumerable<EntityUid> GetAllDevices(EntityUid uid, DeviceListComponent? component = null)
     {
         if (!Resolve(uid, ref component))
@@ -46,10 +13,6 @@ public abstract class SharedDeviceListSystem : EntitySystem
         }
         return component.Devices;
     }
-
-    protected virtual void UpdateShutdownSubscription(EntityUid uid, List<EntityUid> devicesList, List<EntityUid> oldDevices)
-    {
-    }
 }
 
 public sealed class DeviceListUpdateEvent : EntityEventArgs