fix(Core/Packets): Correct data source for attack stop packet, rewrite to modern class. (#25915)

Co-authored-by: Shauren <shauren.trinity@gmail.com>
Co-authored-by: Ludwig <sudlud@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

src/server/game/Entities/Unit/Unit.cpp

@@ -28,6 +28,7 @@
 #include "Chat.h"
 #include "ChatPackets.h"
 #include "ChatTextBuilder.h"
+#include "CombatPackets.h"
 #include "Common.h"
 #include "ConditionMgr.h"
 #include "Creature.h"
@@ -3236,21 +3237,18 @@ void Unit::SendMeleeAttackStart(Unit* victim, Player* sendTo)
  * @brief Send to the client SMSG_ATTACKSTOP but doesn't clear UNIT_STATE_MELEE_ATTACKING on server side
  * or interrupt spells. Unless you know exactly what you're doing, use AttackStop() or RemoveAllAttackers() instead
  */
-void Unit::SendMeleeAttackStop(Unit* victim)
+void Unit::SendMeleeAttackStop(Unit const* victim) const
 {
     // pussywizard: calling SendMeleeAttackStop without clearing UNIT_STATE_MELEE_ATTACKING and then AttackStart the same player may spoil npc rotating!
     // pussywizard: this happens in some boss scripts, just add clearing here
     // ClearUnitState(UNIT_STATE_MELEE_ATTACKING); // commented out for now
 
-    WorldPacket data(SMSG_ATTACKSTOP, (8 + 8 + 4));
-    data << GetPackGUID();
+    WorldPackets::Combat::SAttackStop attackStop;
+    attackStop.Attacker = GetGUID();
+    attackStop.Victim = Object::GetGUID(victim);
+    attackStop.NowDead = !IsAlive();
 
-    if (victim)
-    {
-        data << victim->GetPackGUID();
-        data << (uint32)victim->isDead();
-    }
-    SendMessageToSet(&data, true);
+    SendMessageToSet(attackStop.Write(), true);
     LOG_DEBUG("entities.unit", "WORLD: Sent SMSG_ATTACKSTOP");
 
     if (victim)

src/server/game/Entities/Unit/Unit.h

@@ -909,7 +909,7 @@ public:
     void StopAttackingInvalidTarget();
     Unit* SelectNearbyTarget(Unit* exclude = nullptr, float dist = NOMINAL_MELEE_RANGE) const;
     Unit* SelectNearbyNoTotemTarget(Unit* exclude = nullptr, float dist = NOMINAL_MELEE_RANGE) const;
-    void SendMeleeAttackStop(Unit* victim = nullptr);
+    void SendMeleeAttackStop(Unit const* victim = nullptr) const;
     void SendMeleeAttackStart(Unit* victim, Player* sendTo = nullptr);
 
     [[nodiscard]] uint32 GetAttackTime(WeaponAttackType att) const

src/server/game/Handlers/CombatHandler.cpp

@@ -37,14 +37,14 @@ void WorldSession::HandleAttackSwingOpcode(WorldPacket& recvData)
     if (!pEnemy)
     {
         // stop attack state at client
-        SendAttackStop(nullptr);
+        _player->SendMeleeAttackStop(nullptr);
         return;
     }
 
     if (!_player->IsValidAttackTarget(pEnemy))
     {
         // stop attack state at client
-        SendAttackStop(pEnemy);
+        _player->SendMeleeAttackStop(pEnemy);
         return;
     }
 
@@ -57,7 +57,7 @@ void WorldSession::HandleAttackSwingOpcode(WorldPacket& recvData)
         ASSERT(seat);
         if (!(seat->m_flags & VEHICLE_SEAT_FLAG_CAN_ATTACK))
         {
-            SendAttackStop(pEnemy);
+            _player->SendMeleeAttackStop(pEnemy);
             return;
         }
     }
@@ -80,16 +80,3 @@ void WorldSession::HandleSetSheathedOpcode(WorldPackets::Combat::SetSheathed& pa
 
     _player->SetSheath(SheathState(packet.CurrentSheathState));
 }
-
-void WorldSession::SendAttackStop(Unit const* enemy)
-{
-    WorldPacket data(SMSG_ATTACKSTOP, (8 + 8 + 4)); // we guess size
-    data << GetPlayer()->GetPackGUID();
-
-    if (enemy)
-    {
-        data << enemy->GetPackGUID();               // must be packed guid
-        data << (uint32)enemy->isDead();
-    }
-    SendPacket(&data);
-}

src/server/game/Server/Packets/CombatPackets.cpp

@@ -21,3 +21,14 @@ void WorldPackets::Combat::SetSheathed::Read()
 {
     _worldPacket >> CurrentSheathState;
 }
+
+WorldPacket const* WorldPackets::Combat::SAttackStop::Write()
+{
+    // Packed in place because ObjectGuid::WriteAsPacked() returns a PackedGuid by value, and each one
+    // heap-allocates its buffer. Use it again once TC's non-owning PackedGuidWriter is ported.
+    _worldPacket.appendPackGUID(Attacker.GetRawValue());
+    _worldPacket.appendPackGUID(Victim.GetRawValue());
+    _worldPacket << uint32(NowDead);
+
+    return &_worldPacket;
+}

src/server/game/Server/Packets/CombatPackets.h

@@ -34,6 +34,18 @@ namespace WorldPackets
 
             uint32 CurrentSheathState = 0;
         };
+
+        class SAttackStop final : public ServerPacket
+        {
+        public:
+            SAttackStop() : ServerPacket(SMSG_ATTACKSTOP, 8 + 8 + 4) { }
+
+            WorldPacket const* Write() override;
+
+            ObjectGuid Attacker;
+            ObjectGuid Victim;
+            bool NowDead = false;
+        };
     }
 }
 

src/server/game/Server/WorldSession.h

@@ -553,8 +553,6 @@ public:
     void SendSpiritResurrect();
     void SendBindPoint(Creature* npc);
 
-    void SendAttackStop(Unit const* enemy);
-
     void SendBattleGroundList(ObjectGuid guid, BattlegroundTypeId bgTypeId = BATTLEGROUND_RB);
 
     void SendTradeStatus(TradeStatusInfo const& info);