Fix spray nozzle not cleaning reagents properly (#35950)

* init, god help us all

* further refining

* final round of bugfixes

* whoopsies

* To file scoped namespace

* first review

* oopsie

* oopsie woopsie

* pie is on my face

* persistence

* datafieldn't

* make PreviousTileRef nullable

* change component to file scoped namespace

* Minor tweaks:

- We clamp the reaction amount to a minimum value because when working with percentages and dividing, we approach numbers like 0.01 and never actually properly delete the entity (because we check for zero). This allows us to react with a minimum amount and cleans things up nicely.
- Minor clarification to comments.
- Rebalancing of the spray nozzle projectile.

* the scug lies!!!!

* undo file scoped namespace in system

* kid named warning

---------

Co-authored-by: ScarKy0 <106310278+ScarKy0@users.noreply.github.com>

Content.Server/Chemistry/Components/VaporComponent.cs

@@ -1,4 +1,4 @@
-using Content.Shared.FixedPoint;
+using Robust.Shared.Map;
 
 namespace Content.Server.Chemistry.Components
 {
@@ -7,11 +7,31 @@ namespace Content.Server.Chemistry.Components
     {
         public const string SolutionName = "vapor";
 
-        [DataField("transferAmount")]
-        public FixedPoint2 TransferAmount = FixedPoint2.New(0.5);
+        /// <summary>
+        /// Stores data on the previously reacted tile. We only want to do reaction checks once per tile.
+        /// </summary>
+        [DataField]
+        public TileRef? PreviousTileRef;
 
-        public float ReactTimer;
-        [DataField("active")]
+        /// <summary>
+        /// Percentage of the reagent that is reacted with the TileReaction.
+        /// <example>
+        /// 0.5 = 50% of the reagent is reacted.
+        /// </example>
+        /// </summary>
+        [DataField]
+        public float TransferAmountPercentage;
+
+        /// <summary>
+        /// The minimum amount of the reagent that will be reacted with the TileReaction.
+        /// We do this to prevent floating point issues. A reagent with a low percentage transfer amount will
+        /// transfer 0.01~ forever and never get deleted.
+        /// <remarks>Defaults to 0.05 if not defined, a good general value.</remarks>
+        /// </summary>
+        [DataField]
+        public float MinimumTransferAmount = 0.05f;
+
+        [DataField]
         public bool Active;
     }
 }

Content.Server/Chemistry/EntitySystems/VaporSystem.cs

@@ -30,8 +30,6 @@ namespace Content.Server.Chemistry.EntitySystems
         [Dependency] private readonly ReactiveSystem _reactive = default!;
         [Dependency] private readonly SharedTransformSystem _transformSystem = default!;
 
-        private const float ReactTime = 0.125f;
-
         public override void Initialize()
         {
             base.Initialize();
@@ -50,13 +48,19 @@ namespace Content.Server.Chemistry.EntitySystems
             }
 
             // Check for collision with a impassable object (e.g. wall) and stop
-            if ((args.OtherFixture.CollisionLayer & (int) CollisionGroup.Impassable) != 0 && args.OtherFixture.Hard)
+            if ((args.OtherFixture.CollisionLayer & (int)CollisionGroup.Impassable) != 0 && args.OtherFixture.Hard)
             {
                 EntityManager.QueueDeleteEntity(entity);
             }
         }
 
-        public void Start(Entity<VaporComponent> vapor, TransformComponent vaporXform, Vector2 dir, float speed, MapCoordinates target, float aliveTime, EntityUid? user = null)
+        public void Start(Entity<VaporComponent> vapor,
+            TransformComponent vaporXform,
+            Vector2 dir,
+            float speed,
+            MapCoordinates target,
+            float aliveTime,
+            EntityUid? user = null)
         {
             vapor.Comp.Active = true;
             var despawn = EnsureComp<TimedDespawnComponent>(vapor);
@@ -83,7 +87,9 @@ namespace Content.Server.Chemistry.EntitySystems
                 return false;
             }
 
-            if (!_solutionContainerSystem.TryGetSolution(vapor.Owner, VaporComponent.SolutionName, out var vaporSolution))
+            if (!_solutionContainerSystem.TryGetSolution(vapor.Owner,
+                    VaporComponent.SolutionName,
+                    out var vaporSolution))
             {
                 return false;
             }
@@ -93,53 +99,71 @@ namespace Content.Server.Chemistry.EntitySystems
 
         public override void Update(float frameTime)
         {
+            base.Update(frameTime);
+
+            // Enumerate over all VaporComponents
             var query = EntityQueryEnumerator<VaporComponent, SolutionContainerManagerComponent, TransformComponent>();
             while (query.MoveNext(out var uid, out var vaporComp, out var container, out var xform))
             {
-                foreach (var (_, soln) in _solutionContainerSystem.EnumerateSolutions((uid, container)))
+                // Return early if we're not active
+                if (!vaporComp.Active)
+                    continue;
+
+                // Get the current location of the vapor entity first
+                if (TryComp(xform.GridUid, out MapGridComponent? gridComp))
                 {
-                    Update(frameTime, (uid, vaporComp), soln, xform);
-                }
-            }
-        }
+                    var tile = _map.GetTileRef(xform.GridUid.Value, gridComp, xform.Coordinates);
 
-        private void Update(float frameTime, Entity<VaporComponent> ent, Entity<SolutionComponent> soln, TransformComponent xform)
-        {
-            var (entity, vapor) = ent;
-            if (!vapor.Active)
-                return;
+                    // Check if the tile is a tile we've reacted with previously. If so, skip it.
+                    // If we have no previous tile reference, we don't return so we can save one.
+                    if (vaporComp.PreviousTileRef != null && tile == vaporComp.PreviousTileRef)
+                        continue;
 
-            vapor.ReactTimer += frameTime;
-
-            var contents = soln.Comp.Solution;
-            if (vapor.ReactTimer >= ReactTime && TryComp(xform.GridUid, out MapGridComponent? gridComp))
-            {
-                vapor.ReactTimer = 0;
-
-                var tile = _map.GetTileRef(xform.GridUid.Value, gridComp, xform.Coordinates);
-                foreach (var reagentQuantity in contents.Contents.ToArray())
-                {
-                    if (reagentQuantity.Quantity == FixedPoint2.Zero) continue;
-                    var reagent = _protoManager.Index<ReagentPrototype>(reagentQuantity.Reagent.Prototype);
-
-                    var reaction =
-                        reagent.ReactionTile(tile, (reagentQuantity.Quantity / vapor.TransferAmount) * 0.25f, EntityManager, reagentQuantity.Reagent.Data);
-
-                    if (reaction > reagentQuantity.Quantity)
+                    // Enumerate over all the reagents in the vapor entity solution
+                    foreach (var (_, soln) in _solutionContainerSystem.EnumerateSolutions((uid, container)))
                     {
-                        Log.Error($"Tried to tile react more than we have for reagent {reagentQuantity}. Found {reaction} and we only have {reagentQuantity.Quantity}");
-                        reaction = reagentQuantity.Quantity;
+                        // Iterate over the reagents in the solution
+                        // Reason: Each reagent in our solution may have a unique TileReaction
+                        // In this instance, we check individually for each reagent's TileReaction
+                        // This is not doing chemical reactions!
+                        var contents = soln.Comp.Solution;
+                        foreach (var reagentQuantity in contents.Contents.ToArray())
+                        {
+                            // Check if the reagent is empty
+                            if (reagentQuantity.Quantity == FixedPoint2.Zero)
+                                continue;
+
+                            var reagent = _protoManager.Index<ReagentPrototype>(reagentQuantity.Reagent.Prototype);
+
+                            // Limit the reaction amount to a minimum value to ensure no floating point funnies.
+                            // Ex: A solution with a low percentage transfer amount will slowly approach 0.01... and never get deleted
+                            var clampedAmount = Math.Max(
+                                (float)reagentQuantity.Quantity * vaporComp.TransferAmountPercentage,
+                                vaporComp.MinimumTransferAmount);
+
+                            // Preform the reagent's TileReaction
+                            var reaction =
+                                reagent.ReactionTile(tile,
+                                    clampedAmount,
+                                    EntityManager,
+                                    reagentQuantity.Reagent.Data);
+
+                            if (reaction > reagentQuantity.Quantity)
+                                reaction = reagentQuantity.Quantity;
+
+                            _solutionContainerSystem.RemoveReagent(soln, reagentQuantity.Reagent, reaction);
+                        }
+
+                        // Delete the vapor entity if it has no contents
+                        if (contents.Volume == 0)
+                            EntityManager.QueueDeleteEntity(uid);
+
                     }
 
-                    _solutionContainerSystem.RemoveReagent(soln, reagentQuantity.Reagent, reaction);
+                    // Set the previous tile reference to the current tile
+                    vaporComp.PreviousTileRef = tile;
                 }
             }
-
-            if (contents.Volume == 0)
-            {
-                // Delete this
-                EntityManager.QueueDeleteEntity(entity);
-            }
         }
     }
 }

Resources/Prototypes/Entities/Objects/Specific/Janitorial/spray.yml

@@ -99,8 +99,8 @@
   - type: Tag
     tags:
       - Spray
-# Vapor
 
+# Vapor
 - type: entity
   id: Vapor
   name: "vapor"
@@ -111,6 +111,8 @@
       vapor:
         maxVol: 50
   - type: Vapor
+    active: true
+    transferAmountPercentage: 0.5
   - type: AnimationPlayer
   - type: Sprite
     sprite: Effects/chempuff.rsi

Resources/Prototypes/Entities/Objects/Weapons/Guns/Projectiles/projectiles.yml

@@ -936,6 +936,7 @@
         - BulletImpassable
   - type: Vapor
     active: true
+    transferAmountPercentage: 1
   - type: Appearance
   - type: VaporVisuals