Core/Packets: fixed crash on SMSG_ATTACKSTOP and fixed init of ItemInstance

8 files changed, 35 insertions, 48 deletions

diff --git a/src/server/game/Entities/Unit/Unit.cpp b/src/server/game/Entities/Unit/Unit.cpp
index 76ff57361ea..057b3b984ef 100644
--- a/src/server/game/Entities/Unit/Unit.cpp
+++ b/src/server/game/Entities/Unit/Unit.cpp
@@ -2108,20 +2108,11 @@ void Unit::SendMeleeAttackStart(Unit* victim)
     packet.Attacker = GetGUID();
     packet.Victim = victim->GetGUID();
     SendMessageToSet(packet.Write(), true);
-    TC_LOG_DEBUG("entities.unit", "WORLD: Sent SMSG_ATTACKSTART");
 }
 
 void Unit::SendMeleeAttackStop(Unit* victim)
 {
-    WorldPackets::Combat::SAttackStop packet;
-    packet.Attacker = GetGUID();
-    if (victim)
-    {
-        packet.Victim = victim->GetGUID();
-        packet.NowDead = victim->isDead();
-    }
-
-    SendMessageToSet(packet.Write(), true);
+    SendMessageToSet(WorldPackets::Combat::SAttackStop(this, victim).Write(), true);
 
     if (victim)
         TC_LOG_INFO("entities.unit", "%s stopped attacking %s", GetGUID().ToString().c_str(), victim->GetGUID().ToString().c_str());
@@ -11440,17 +11431,9 @@ void Unit::SetPower(Powers power, int32 val)
     if (IsInWorld())
     {
         WorldPackets::Combat::PowerUpdate packet;
-        WorldPackets::Combat::PowerUpdatePower power;
         packet.Guid = GetGUID();
         /// @todo: Support multiple counts ?
-        /*for (uint8 i = 0; i < 1; i++)
-        {
-            _power.Power = val;
-            _power.PowerType = powerIndex;
-        }*/
-        power.Power = val;
-        power.PowerType = powerIndex;
-        packet.Powers.push_back(power);
+        packet.Powers.emplace_back(val, powerIndex);
         SendMessageToSet(packet.Write(), GetTypeId() == TYPEID_PLAYER);
     }
 
diff --git a/src/server/game/Handlers/CombatHandler.cpp b/src/server/game/Handlers/CombatHandler.cpp
index 36f37403f32..9b9a32b2c2a 100644
--- a/src/server/game/Handlers/CombatHandler.cpp
+++ b/src/server/game/Handlers/CombatHandler.cpp
@@ -31,19 +31,19 @@
 
 void WorldSession::HandleAttackSwingOpcode(WorldPackets::Combat::AttackSwing& packet)
 {
-    Unit* pEnemy = ObjectAccessor::GetUnit(*_player, packet.Victim);
+    Unit* enemy = ObjectAccessor::GetUnit(*_player, packet.Victim);
 
-    if (!pEnemy)
+    if (!enemy)
     {
         // stop attack state at client
-        SendAttackStop(NULL);
+        SendAttackStop(nullptr);
         return;
     }
 
-    if (!_player->IsValidAttackTarget(pEnemy))
+    if (!_player->IsValidAttackTarget(enemy))
     {
         // stop attack state at client
-        SendAttackStop(pEnemy);
+        SendAttackStop(enemy);
         return;
     }
 
@@ -56,12 +56,12 @@ void WorldSession::HandleAttackSwingOpcode(WorldPackets::Combat::AttackSwing& pa
         ASSERT(seat);
         if (!(seat->Flags & VEHICLE_SEAT_FLAG_CAN_ATTACK))
         {
-            SendAttackStop(pEnemy);
+            SendAttackStop(enemy);
             return;
         }
     }
 
-    _player->Attack(pEnemy, true);
+    _player->Attack(enemy, true);
 }
 
 void WorldSession::HandleAttackStopOpcode(WorldPackets::Combat::AttackStop& /*recvData*/)
@@ -82,6 +82,5 @@ void WorldSession::HandleSetSheathedOpcode(WorldPackets::Combat::SetSheathed& pa
 
 void WorldSession::SendAttackStop(Unit const* enemy)
 {
-    WorldPackets::Combat::SAttackStop packet(GetPlayer()->GetGUID(), enemy->GetGUID(), enemy->isDead());
-    SendPacket(packet.Write());
+    SendPacket(WorldPackets::Combat::SAttackStop(GetPlayer(), enemy).Write());
 }
diff --git a/src/server/game/Loot/LootMgr.h b/src/server/game/Loot/LootMgr.h
index 75ea54a9dc5..a14bcb50107 100644
--- a/src/server/game/Loot/LootMgr.h
+++ b/src/server/game/Loot/LootMgr.h
@@ -35,11 +35,6 @@ namespace WorldPackets
     {
         class LootResponse;
     }
-
-    namespace Item
-    {
-        struct ItemInstance;
-    }
 }
 
 enum RollType
@@ -155,8 +150,7 @@ struct LootStoreItem
          { }
 
     bool Roll(bool rate) const;                             // Checks if the entry takes it's chance (at loot generation)
-    bool IsValid(LootStore const& store, uint32 entry) const;
-                                                            // Checks correctness of values
+    bool IsValid(LootStore const& store, uint32 entry) const; // Checks correctness of values
 };
 
 struct LootItem
diff --git a/src/server/game/Reputation/ReputationMgr.cpp b/src/server/game/Reputation/ReputationMgr.cpp
index 92a41e8c6a6..adc4768501c 100644
--- a/src/server/game/Reputation/ReputationMgr.cpp
+++ b/src/server/game/Reputation/ReputationMgr.cpp
@@ -49,7 +49,7 @@ bool ReputationMgr::IsAtWar(uint32 faction_id) const
     if (!factionEntry)
     {
         TC_LOG_ERROR("misc", "ReputationMgr::IsAtWar: Can't get AtWar flag of %s for unknown faction (faction id) #%u.", _player->GetName().c_str(), faction_id);
-        return 0;
+        return false;
     }
 
     return IsAtWar(factionEntry);
diff --git a/src/server/game/Server/Packets/CombatPackets.cpp b/src/server/game/Server/Packets/CombatPackets.cpp
index 5a23729f7bc..69718cb932f 100644
--- a/src/server/game/Server/Packets/CombatPackets.cpp
+++ b/src/server/game/Server/Packets/CombatPackets.cpp
@@ -31,6 +31,16 @@ WorldPacket const* WorldPackets::Combat::AttackStart::Write()
     return &_worldPacket;
 }
 
+WorldPackets::Combat::SAttackStop::SAttackStop(Unit const* attacker, Unit const* victim) : ServerPacket(SMSG_ATTACKSTOP, 16 + 16 + 1)
+{
+    Attacker = attacker->GetGUID();
+    if (victim)
+    {
+        Victim = victim->GetGUID();
+        NowDead = victim->isDead();
+    }
+}
+
 WorldPacket const* WorldPackets::Combat::SAttackStop::Write()
 {
     _worldPacket << Attacker;
@@ -152,7 +162,7 @@ WorldPacket const* WorldPackets::Combat::PowerUpdate::Write()
 {
     _worldPacket << Guid;
     _worldPacket << uint32(Powers.size());
-    for (WorldPackets::Combat::PowerUpdatePower const& power : Powers)
+    for (PowerUpdatePower const& power : Powers)
     {
         _worldPacket << power.Power;
         _worldPacket << power.PowerType;
diff --git a/src/server/game/Server/Packets/CombatPackets.h b/src/server/game/Server/Packets/CombatPackets.h
index 4e2aa3c34b2..4eeb61d3523 100644
--- a/src/server/game/Server/Packets/CombatPackets.h
+++ b/src/server/game/Server/Packets/CombatPackets.h
@@ -70,7 +70,7 @@ namespace WorldPackets
         {
         public:
             SAttackStop() : ServerPacket(SMSG_ATTACKSTOP, 16 + 16 + 1) { }
-            SAttackStop(ObjectGuid attacker, ObjectGuid victim, bool nowDead) : ServerPacket(SMSG_ATTACKSTOP, 16 + 16 + 1), Attacker(attacker), Victim(victim), NowDead(nowDead) { }
+            SAttackStop(Unit const* attacker, Unit const* victim);
 
             WorldPacket const* Write() override;
 
@@ -162,7 +162,7 @@ namespace WorldPackets
 
             WorldPacket const* Write() override;
 
-            Optional<WorldPackets::Spells::SpellCastLogData> LogData;
+            Optional<Spells::SpellCastLogData> LogData;
             uint32 HitInfo          = 0; // Flags
             ObjectGuid AttackerGUID;
             ObjectGuid VictimGUID;
@@ -188,6 +188,8 @@ namespace WorldPackets
 
         struct PowerUpdatePower
         {
+            PowerUpdatePower(int32 power, uint8 powerType) : Power(power), PowerType(powerType) { }
+
             int32 Power = 0;
             uint8 PowerType = 0;
         };
@@ -196,7 +198,7 @@ namespace WorldPackets
         {
         public:
             PowerUpdate() : ServerPacket(SMSG_POWER_UPDATE, 16 + 4 + 1) { }
-            
+
             WorldPacket const* Write() override;
 
             ObjectGuid Guid;
diff --git a/src/server/game/Server/Packets/ItemPackets.cpp b/src/server/game/Server/Packets/ItemPackets.cpp
index dbc7ba2615a..9277e0b73d5 100644
--- a/src/server/game/Server/Packets/ItemPackets.cpp
+++ b/src/server/game/Server/Packets/ItemPackets.cpp
@@ -175,16 +175,16 @@ void WorldPackets::Item::ItemInstance::Initalize(::Item const* item)
     {
         ItemBonus.HasValue = true;
         ItemBonus.Value.BonusListIDs.insert(ItemBonus.Value.BonusListIDs.end(), bonusListIds.begin(), bonusListIds.end());
-        ItemBonus.Value.Context = 0; /// @todo
+        ItemBonus.Value.Context = item->GetUInt32Value(ITEM_FIELD_CONTEXT);
     }
 
-    for (uint8 i = 1; i < MAX_ITEM_MODIFIERS; ++i)
+    uint32 mask = item->GetUInt32Value(ITEM_FIELD_MODIFIERS_MASK);
+    Modifications.HasValue = mask != 0;
+
+    for (size_t i = 0; mask != 0; mask >>= 1, ++i)
     {
-        if (int32 mod = item->GetModifier(ItemModifier(i)))
-        {
-            Modifications.HasValue = true;
-            Modifications.Value.Insert(i - 1, mod);
-        }
+        if ((mask & 1) != 0)
+            Modifications.Value.Insert(i, item->GetModifier(ItemModifier(i)));
     }
 }
 
diff --git a/src/server/game/Server/Packets/NPCPackets.cpp b/src/server/game/Server/Packets/NPCPackets.cpp
index 4dc3d66a852..7bf96b03bab 100644
--- a/src/server/game/Server/Packets/NPCPackets.cpp
+++ b/src/server/game/Server/Packets/NPCPackets.cpp
@@ -16,7 +16,6 @@
  */
 
 #include "NPCPackets.h"
-#include "ItemPackets.h"
 
 void WorldPackets::NPC::Hello::Read()
 {