Fix Hands Crash (#122)

* Fixed sprite issues with construction system (Thanks PJB!).

* Storage and Hands Systems now subscribe to Transform Parent changes, and will keep their containers in sync.

* Add check in Interaction System to prevent processing client-side entities on the server.

Content.Client/GameObjects/Components/Construction/ConstructorComponent.cs

@@ -1,4 +1,4 @@
-using System.Collections.Generic;
+using System.Collections.Generic;
 using Content.Client.Construction;
 using Content.Shared.Construction;
 using Content.Shared.GameObjects.Components.Construction;
@@ -68,10 +68,10 @@ namespace Content.Client.GameObjects.Components.Construction
             comp.Prototype = prototype;
             comp.Master = this;
             comp.GhostID = nextId++;
-            var transform = ghost.GetComponent<ITransformComponent>().LocalRotation = dir.ToAngle();
+            ghost.GetComponent<ITransformComponent>().LocalRotation = dir.ToAngle();
             var sprite = ghost.GetComponent<SpriteComponent>();
             sprite.LayerSetSprite(0, prototype.Icon);
-            sprite.LayerSetShader(0, "unshaded");
+            sprite.LayerSetVisible(0, true);
 
             Ghosts.Add(comp.GhostID, comp);
         }

Content.Server/GameObjects/Components/GUI/InventoryComponent.cs

@@ -111,7 +111,7 @@ namespace Content.Server.GameObjects
                 return false;
             }
 
-            clothing.EquippedToSlot(inventorySlot);
+            clothing.EquippedToSlot();
 
             Dirty();
             return true;

Content.Server/GameObjects/Components/GUI/ServerHandsComponent.cs

@@ -71,6 +71,21 @@ namespace Content.Server.GameObjects
             }
         }
 
+        /// <inheritdoc />
+        public void RemoveHandEntity(IEntity entity)
+        {
+            if(entity == null)
+                return;
+
+            foreach (var slot in hands.Values)
+            {
+                if (slot.ContainedEntity == entity)
+                {
+                    slot.Remove(entity);
+                }
+            }
+        }
+
         public ItemComponent GetHand(string index)
         {
             var slot = hands[index];

Content.Server/GameObjects/Components/Items/Storage/ItemComponent.cs

@@ -18,7 +18,7 @@ namespace Content.Server.GameObjects
             }
         }
 
-        public void EquippedToSlot(ContainerSlot slot)
+        public void EquippedToSlot()
         {
             foreach (var component in Owner.GetAllComponents<ISpriteRenderableComponent>())
             {

Content.Server/GameObjects/Components/Items/Storage/ServerStorageComponent.cs

@@ -14,6 +14,7 @@ using SS14.Shared.IoC;
 using SS14.Shared.Log;
 using SS14.Shared.Serialization;
 using System.Collections.Generic;
+using SS14.Shared.GameObjects.EntitySystemMessages;
 using SS14.Shared.ViewVariables;
 
 namespace Content.Server.GameObjects
@@ -70,7 +71,7 @@ namespace Content.Server.GameObjects
         /// </summary>
         /// <param name="toremove"></param>
         /// <returns></returns>
-        bool Remove(IEntity toremove)
+        public bool Remove(IEntity toremove)
         {
             _ensureInitialCalculated();
             if (storage.Remove(toremove))
@@ -88,7 +89,7 @@ namespace Content.Server.GameObjects
         /// </summary>
         /// <param name="toinsert"></param>
         /// <returns></returns>
-        bool Insert(IEntity toinsert)
+        public bool Insert(IEntity toinsert)
         {
             if (CanInsert(toinsert) && storage.Insert(toinsert))
             {
@@ -105,7 +106,7 @@ namespace Content.Server.GameObjects
         /// </summary>
         /// <param name="toinsert"></param>
         /// <returns></returns>
-        bool CanInsert(IEntity toinsert)
+        public bool CanInsert(IEntity toinsert)
         {
             _ensureInitialCalculated();
             if (toinsert.TryGetComponent(out StoreableComponent store))

Content.Server/GameObjects/EntitySystems/Click/InteractionSystem.cs

@@ -139,7 +139,13 @@ namespace Content.Server.GameObjects.EntitySystems
             // client sanitization
             if(!coords.IsValidLocation())
             {
-                Logger.InfoS("interaction", $"Invalid Coordinates: client={session}, coords={coords}");
+                Logger.InfoS("system.interaction", $"Invalid Coordinates: client={session}, coords={coords}");
+                return;
+            }
+
+            if (uid.IsClientSide())
+            {
+                Logger.WarningS("system.interaction", $"Client sent interaction with client-side entity. Session={session}, Uid={uid}");
                 return;
             }
 
@@ -169,8 +175,8 @@ namespace Content.Server.GameObjects.EntitySystems
             {
                 return;
             }
-            var item = hands.GetActiveHand?.Owner;
 
+            var item = hands.GetActiveHand?.Owner;
 
             if (!MobCanInteract(player))
                 return;

Content.Server/GameObjects/EntitySystems/HandsSystem.cs

@@ -1,7 +1,7 @@
 using System;
 using Content.Server.GameObjects.Components;
-using Content.Server.GameObjects.Components.Projectiles;
 using Content.Server.GameObjects.Components.Stack;
+using Content.Server.Interfaces.GameObjects;
 using Content.Shared.Input;
 using Content.Shared.Physics;
 using SS14.Server.GameObjects;
@@ -11,7 +11,6 @@ using SS14.Shared.GameObjects;
 using SS14.Shared.GameObjects.EntitySystemMessages;
 using SS14.Shared.GameObjects.Systems;
 using SS14.Shared.Input;
-using SS14.Shared.Interfaces.GameObjects;
 using SS14.Shared.Interfaces.GameObjects.Components;
 using SS14.Shared.Interfaces.Timing;
 using SS14.Shared.IoC;
@@ -61,11 +60,18 @@ namespace Content.Server.GameObjects.EntitySystems
         {
             var msg = (EntParentChangedMessage) args;
 
+            // entity is no longer a child of OldParent, therefore it cannot be in the hand of the parent
+            if (msg.OldParent != null && msg.OldParent.IsValid() && msg.OldParent.TryGetComponent(out IHandsComponent handsComp))
+            {
+                handsComp.RemoveHandEntity(msg.Entity);
+            }
+
+            // deleted entities will not pass this test
             if (!msg.Entity.TryGetComponent(out ITransformComponent transform))
                 return;
 
             // if item is in a container
-            if(transform.IsMapTransform)
+            if (transform.IsMapTransform)
                 return;
 
             if(!msg.Entity.TryGetComponent(out PhysicsComponent physics))

Content.Server/GameObjects/EntitySystems/StorageSystem.cs

@@ -1,7 +1,9 @@
 using System.Collections.Generic;
 using SS14.Server.Interfaces.Player;
 using SS14.Shared.GameObjects;
+using SS14.Shared.GameObjects.EntitySystemMessages;
 using SS14.Shared.GameObjects.Systems;
+using SS14.Shared.Interfaces.GameObjects;
 
 namespace Content.Server.GameObjects.EntitySystems
 {
@@ -15,42 +17,72 @@ namespace Content.Server.GameObjects.EntitySystems
             EntityQuery = new TypeEntityQuery(typeof(ServerStorageComponent));
         }
 
+        /// <inheritdoc />
+        public override void SubscribeEvents()
+        {
+            base.SubscribeEvents();
+
+            SubscribeEvent<EntParentChangedMessage>(HandleParentChanged);
+        }
+
         /// <inheritdoc />
         public override void Update(float frameTime)
         {
             foreach (var entity in RelevantEntities)
             {
-                var storageComp = entity.GetComponent<ServerStorageComponent>();
+                CheckSubscribedEntities(entity);
+            }
+        }
 
-                // We have to cache the set of sessions because Unsubscribe modifies the original.
+        private static void HandleParentChanged(object sender, EntitySystemMessage message)
-                _sessionCache.Clear();
+        {
-                _sessionCache.AddRange(storageComp.SubscribedSessions);
+            if(!(sender is IEntity childEntity))
+                return;
 
-                if (_sessionCache.Count == 0)
+            if(!(message is EntParentChangedMessage msg))
+                return;
+
+            var oldParentEntity = msg.OldParent;
+
+            if(oldParentEntity == null || !oldParentEntity.IsValid())
+                return;
+
+            if (oldParentEntity.TryGetComponent(out ServerStorageComponent storageComp))
+            {
+                storageComp.Remove(childEntity);
+            }
+        }
+
+        private void CheckSubscribedEntities(IEntity entity)
+        {
+            var storageComp = entity.GetComponent<ServerStorageComponent>();
+
+            // We have to cache the set of sessions because Unsubscribe modifies the original.
+            _sessionCache.Clear();
+            _sessionCache.AddRange(storageComp.SubscribedSessions);
+
+            if (_sessionCache.Count == 0)
+                return;
+
+            var storagePos = entity.Transform.WorldPosition;
+            var storageMap = entity.Transform.MapID;
+
+            foreach (var session in _sessionCache)
+            {
+                var attachedEntity = session.AttachedEntity;
+
+                // The component manages the set of sessions, so this invalid session should be removed soon.
+                if (attachedEntity == null || !attachedEntity.IsValid())
                     continue;
 
-                var storagePos = entity.Transform.WorldPosition;
+                if (storageMap != attachedEntity.Transform.MapID)
-                var storageMap = entity.Transform.MapID;
+                    continue;
 
-                foreach (var session in _sessionCache)
+                var distanceSquared = (storagePos - attachedEntity.Transform.WorldPosition).LengthSquared;
+                if (distanceSquared > InteractionSystem.INTERACTION_RANGE_SQUARED)
                 {
-                    var attachedEntity = session.AttachedEntity;
+                    storageComp.UnsubscribeSession(session);
-
-                    // The component manages the set of sessions, so this invalid session should be removed soon.
-                    if (attachedEntity == null || !attachedEntity.IsValid())
-                        continue;
-
-                    if(storageMap != attachedEntity.Transform.MapID)
-                        continue;
-
-                    var distanceSquared = (storagePos - attachedEntity.Transform.WorldPosition).LengthSquared;
-                    if (distanceSquared > InteractionSystem.INTERACTION_RANGE_SQUARED)
-                    {
-                        storageComp.UnsubscribeSession(session);
-                    }
                 }
-
-
             }
         }
     }

Content.Server/Interfaces/GameObjects/Components/Items/IHandsComponent.cs

@@ -24,6 +24,12 @@ namespace Content.Server.Interfaces.GameObjects
         /// <returns>The item in the held, null if no item is held</returns>
         ItemComponent GetHand(string index);
 
+        /// <summary>
+        ///     If any hands are holding this entity, immediately remove the entity without dropping it.
+        /// </summary>
+        /// <param name="entity">Entity to be removed.</param>
+        void RemoveHandEntity(IEntity entity);
+
         /// <summary>
         /// Gets item held by the current active hand
         /// </summary>

Resources/Prototypes/Construction/structures.yml

@@ -3,7 +3,9 @@
   id: wall
   category: Structures
   description: Keeps the air in and the greytide out.
-  icon: Tiles/wall_texture.png
+  icon:
+    sprite: Buildings/wall.rsi
+    state: full
   objecttype: Structure
   result: wall
   placementmode: SnapgridBorder

Resources/Prototypes/Entities/Construction.yml

@@ -4,6 +4,8 @@
   components:
   - type: Sprite
     color: '#3F38'
+    layers:
+    - shader: unshaded
   - type: ConstructionGhost
   - type: BoundingBox
   - type: Clickable