The cards a skill inflicts, which nothing read

src/NosCore.GameObject/Services/BattleService/HitQueue.cs

@@ -35,6 +35,7 @@ public sealed class HitQueue(
     IBattleStatsProvider statsProvider,
     IBuffService buffService,
     IRegenerationService regenerationService,
+    IInflictedCardService inflictedCardService,
     ILogger<HitQueue> logger) : IHitQueue, ISingletonService
 {
     private readonly ConcurrentDictionary<Entity, Channel<HitRequest>> _channels = new();
@@ -183,6 +184,19 @@ await ApplySpecialActionsAsync(request.Skill.BCards, request.Origin, target)
                 _ = buffService.ApplySkillBuffAsync(target, request.Skill.SkillVnum, request.Skill.Duration, request.Skill.BCards, request.Origin);
             }
 
+            // The cards the skill inflicts - the stun of Star Attack, the poison of a poisoned
+            // arrow. A different thing from the buff above: that one turns the skill's own BCards
+            // into a lasting effect, this one applies the Card the skill names by id.
+            //
+            // On the target and not the caster because we are on a blow that has landed: who
+            // receives it is a question the files do not answer, and here it does not arise.
+            if (!killed && request.Skill.BCards.Count > 0)
+            {
+                await inflictedCardService
+                    .InflictAsync(target, request.Origin, request.Skill.BCards)
+                    .ConfigureAwait(false);
+            }
+
             request.Completion.TrySetResult(new HitOutcome(HitStatus.Landed, damage.Damage, damage.HitMode, killed));
         }
         catch (Exception ex)

src/NosCore.GameObject/Services/BattleService/IInflictedCardService.cs

@@ -0,0 +1,29 @@
+//  __  _  __    __   ___ __  ___ ___
+// |  \| |/__\ /' _/ / _//__\| _ \ __|
+// | | ' | \/ |`._`.| \_| \/ | v / _|
+// |_|\__|\__/ |___/ \__/\__/|_|_\___|
+//
+
+using System.Collections.Generic;
+using System.Threading.Tasks;
+using NosCore.Data.StaticEntities;
+using NosCore.GameObject.Ecs.Interfaces;
+
+namespace NosCore.GameObject.Services.BattleService;
+
+/// <summary>
+/// The cards a skill inflicts on what it hits: the poison of a poisoned arrow, the stun of Star
+/// Attack, the rage of Hit of Rage.
+///
+/// A skill does not carry the effect - it carries a BCard of type 25 saying "with N% chance,
+/// apply Card number M". Card M is a real entry of <c>Card.dat</c>, with its own duration and its
+/// own BCards. Without the step back from M to the Card, that reference goes nowhere, and it is
+/// the most widespread effect in the game: 1344 of the skills declare one.
+/// </summary>
+public interface IInflictedCardService
+{
+    /// <summary>
+    /// Rolls each type 25 BCard the skill declares and applies, or removes, the card it names.
+    /// </summary>
+    Task InflictAsync(IAliveEntity target, IAliveEntity? caster, IReadOnlyList<BCardDto> skillBCards);
+}

src/NosCore.GameObject/Services/BattleService/InflictedCardService.cs

@@ -0,0 +1,76 @@
+//  __  _  __    __   ___ __  ___ ___
+// |  \| |/__\ /' _/ / _//__\| _ \ __|
+// | | ' | \/ |`._`.| \_| \/ | v / _|
+// |_|\__|\__/ |___/ \__/\__/|_|_\___|
+//
+
+using System.Collections.Generic;
+using System.Threading.Tasks;
+using NosCore.Data.Enumerations.Buff;
+using NosCore.Data.StaticEntities;
+using NosCore.GameObject.Ecs.Interfaces;
+using NosCore.GameObject.Infastructure;
+
+namespace NosCore.GameObject.Services.BattleService;
+
+// BCard type 25/11 and 25/12: FirstData is the percentage, SecondData the card id. The columns
+// tell them apart - 32 distinct values on one side against 780 on the other, and an id does not
+// repeat 717 times.
+//
+// Takes a target rather than a skill because the BCard does not say who receives the card; on a
+// blow that has landed the entity that took the damage is the one it goes on.
+public sealed class InflictedCardService(
+    ICardCatalog cardCatalog,
+    IBuffService buffService,
+    IRandomProvider randomProvider) : IInflictedCardService, ISingletonService
+{
+    public async Task InflictAsync(IAliveEntity target, IAliveEntity? caster,
+        IReadOnlyList<BCardDto> skillBCards)
+    {
+        for (var i = 0; i < skillBCards.Count; i++)
+        {
+            var bCard = skillBCards[i];
+            if ((BCardType.CardType)bCard.Type != BCardType.CardType.Buff)
+            {
+                continue;
+            }
+
+            var subType = (AdditionalTypes.Buff)bCard.SubType;
+            if (subType is not (AdditionalTypes.Buff.ChanceCausing or AdditionalTypes.Buff.ChanceRemoving))
+            {
+                continue;
+            }
+
+            if (!Rolls(bCard.FirstData))
+            {
+                continue;
+            }
+
+            var cardId = (short)bCard.SecondData;
+            if (subType == AdditionalTypes.Buff.ChanceRemoving)
+            {
+                await buffService.RemoveAsync(target, cardId).ConfigureAwait(false);
+                continue;
+            }
+
+            // One of the 1341 declarations names a card that is not in the file. It is skipped
+            // rather than thrown on: a single bad row in the client's data must not take a blow
+            // down with it.
+            var card = cardCatalog.GetCard(cardId);
+            if (card == null)
+            {
+                continue;
+            }
+
+            await buffService
+                .ApplyAsync(target, card, cardCatalog.GetCardBCards(cardId), caster)
+                .ConfigureAwait(false);
+        }
+    }
+
+    /// <summary>
+    /// The roll. <c>Next(0, 100)</c> yields 0 to 99, so "less than" makes 100 always succeed and 0
+    /// never - which is what the file means, and 717 of the 1341 declarations say 100.
+    /// </summary>
+    private bool Rolls(int percent) => percent > 0 && randomProvider.Next(0, 100) < percent;
+}

test/NosCore.GameObject.Tests/Services/BattleService/HitQueueTests.cs

@@ -97,7 +97,8 @@ public async Task LandedHitAppliesSkillBuffsWhenSkillHasDuration()
             var stats = new Mock<IBattleStatsProvider>();
             stats.Setup(s => s.GetStats(It.IsAny<IAliveEntity>())).Returns(new CombatStats());
 
-            var queue = new HitQueue(calc.Object, stats.Object, buffs.Object, new Mock<IRegenerationService>().Object, new Mock<ILogger<HitQueue>>().Object);
+            var cards = new Mock<IInflictedCardService>();
+            var queue = new HitQueue(calc.Object, stats.Object, buffs.Object, new Mock<IRegenerationService>().Object, cards.Object, new Mock<ILogger<HitQueue>>().Object);
             var skill = MakeSkill() with
             {
                 SkillVnum = 7,
@@ -109,6 +110,74 @@ public async Task LandedHitAppliesSkillBuffsWhenSkillHasDuration()
             await queue.EnqueueAsync(request);
 
             buffs.Verify(b => b.ApplySkillBuffAsync(target, (short)7, (short)100, skill.BCards, attacker), Times.Once);
+
+            // The other half, and a different thing: the buff above turns the skill's own BCards
+            // into a lasting effect, this inflicts the Card those BCards name by id. On the
+            // entity that took the blow, which is the only reason this can run here at all.
+            cards.Verify(c => c.InflictAsync(target, attacker, skill.BCards), Times.Once);
+        }
+
+        // The blow does not report itself finished until the card it carries has been applied.
+        //
+        // The Verify above cannot see this: it passes just as well if the call is fired and
+        // forgotten, because Moq answers with an already-completed Task either way. And
+        // fire-and-forget is exactly what this used to be - TryApplyHit was made async for this
+        // one property, so something has to hold on to it.
+        //
+        // The wait can only fail in the safe direction: if the call is awaited the hit can never
+        // complete, so the delay always wins; a loaded machine can make this pass when it should
+        // not, never fail when it should not.
+        [TestMethod]
+        public async Task ALandedHitDoesNotFinishBeforeTheCardIsApplied()
+        {
+            var target = new FakeBattleEntity { Hp = 100, MaxHp = 100 };
+            var attacker = new FakeBattleEntity();
+            var calc = new Mock<IDamageCalculator>();
+            calc.Setup(c => c.Calculate(It.IsAny<CombatStats>(), It.IsAny<CombatStats>(), It.IsAny<SkillInfo>()))
+                .Returns(new DamageResult(10, SuPacketHitMode.SuccessAttack));
+            var stats = new Mock<IBattleStatsProvider>();
+            stats.Setup(s => s.GetStats(It.IsAny<IAliveEntity>())).Returns(new CombatStats());
+
+            // Two signals, not one. `entered` says the worker has actually reached the call -
+            // without it the test would also pass on a machine slow enough that the worker never
+            // got there, which is a pass for the wrong reason. `applying` is the gate the call
+            // waits on.
+            var entered = new TaskCompletionSource();
+            var applying = new TaskCompletionSource();
+            var cards = new Mock<IInflictedCardService>();
+            cards.Setup(c => c.InflictAsync(It.IsAny<IAliveEntity>(), It.IsAny<IAliveEntity>(),
+                    It.IsAny<System.Collections.Generic.IReadOnlyList<BCardDto>>()))
+                .Returns(() =>
+                {
+                    entered.TrySetResult();
+                    return applying.Task;
+                });
+
+            var queue = new HitQueue(calc.Object, stats.Object, new Mock<IBuffService>().Object,
+                new Mock<IRegenerationService>().Object, cards.Object, new Mock<ILogger<HitQueue>>().Object);
+            var skill = MakeSkill() with { BCards = new[] { new BCardDto { Type = 3 } } };
+
+            var hit = queue.EnqueueAsync(Request(attacker, target) with { Skill = skill });
+
+            try
+            {
+                Assert.AreSame(entered.Task, await Task.WhenAny(entered.Task, Task.Delay(5000)),
+                    "the worker never reached the card application");
+
+                // Now that the call is in flight, the hit must not be finished. The wait can only
+                // fail in the safe direction: if the call is awaited the hit can never complete,
+                // so the delay always wins.
+                Assert.AreNotSame(hit, await Task.WhenAny(hit, Task.Delay(200)),
+                    "the hit finished while the card was still being applied");
+            }
+            finally
+            {
+                // In a finally, or a failed assertion above would leave the queue's worker parked
+                // on a gate nobody opens for the rest of the run.
+                applying.TrySetResult();
+            }
+
+            await hit;
         }
 
         [TestMethod]
@@ -123,12 +192,18 @@ public async Task KillingHitSkipsBuffApplication()
             var stats = new Mock<IBattleStatsProvider>();
             stats.Setup(s => s.GetStats(It.IsAny<IAliveEntity>())).Returns(new CombatStats());
 
-            var queue = new HitQueue(calc.Object, stats.Object, buffs.Object, new Mock<IRegenerationService>().Object, new Mock<ILogger<HitQueue>>().Object);
+            var cards = new Mock<IInflictedCardService>();
+            var queue = new HitQueue(calc.Object, stats.Object, buffs.Object, new Mock<IRegenerationService>().Object, cards.Object, new Mock<ILogger<HitQueue>>().Object);
             var skill = MakeSkill() with { Duration = 100, BCards = new[] { new BCardDto { Type = 3 } } };
 
             await queue.EnqueueAsync(Request(attacker, target) with { Skill = skill });
 
             buffs.Verify(b => b.ApplySkillBuffAsync(It.IsAny<IAliveEntity>(), It.IsAny<short>(), It.IsAny<short>(), It.IsAny<System.Collections.Generic.IReadOnlyList<BCardDto>>(), It.IsAny<IAliveEntity>()), Times.Never);
+
+            // Nor a card on a corpse: poisoning something already dead costs a packet and a buff
+            // icon on an entity that is about to stop existing.
+            cards.Verify(c => c.InflictAsync(It.IsAny<IAliveEntity>(), It.IsAny<IAliveEntity>(),
+                It.IsAny<System.Collections.Generic.IReadOnlyList<BCardDto>>()), Times.Never);
         }
 
         [TestMethod]
@@ -167,7 +242,8 @@ private static HitQueue QueueDealing(int ordinaryDamage)
             var stats = new Mock<IBattleStatsProvider>();
             stats.Setup(s => s.GetStats(It.IsAny<IAliveEntity>())).Returns(new CombatStats());
             return new HitQueue(calc.Object, stats.Object, new Mock<IBuffService>().Object,
-                new Mock<IRegenerationService>().Object, new Mock<ILogger<HitQueue>>().Object);
+                new Mock<IRegenerationService>().Object, new Mock<IInflictedCardService>().Object,
+                new Mock<ILogger<HitQueue>>().Object);
         }
 
         [TestMethod]
@@ -273,7 +349,7 @@ private static HitQueue BuildQueue(Action<MutableDamage> configure)
             var stats = new Mock<IBattleStatsProvider>();
             stats.Setup(s => s.GetStats(It.IsAny<IAliveEntity>())).Returns(new CombatStats());
 
-            return new HitQueue(calc.Object, stats.Object, new Mock<IBuffService>().Object, new Mock<IRegenerationService>().Object, new Mock<ILogger<HitQueue>>().Object);
+            return new HitQueue(calc.Object, stats.Object, new Mock<IBuffService>().Object, new Mock<IRegenerationService>().Object, new Mock<IInflictedCardService>().Object, new Mock<ILogger<HitQueue>>().Object);
         }
 
         private class MutableDamage