refactor(Packet): Add ServerPacket structure by 0blu · Pull Request #3361 · vmangos/core

0blu · 2026-04-11T16:54:32Z

Converts some of the manually built WorldPacket (which were sent by the server) to nicely structured ServerPackets.

Due to the large amount of changes required this will be split into multiple PRs.

src/game/Handlers/PetitionsHandler.cpp

src/game/Handlers/QueryHandler.cpp

ratkosrb · 2026-04-13T08:57:14Z

src/game/Server/Packets/Spell.h

+    // Simplified SMSG_CAST_RESULT for a basic spell failure (status=2) with no extra data.
+    // For complex failures with additional payload (e.g. cooldown time, required area),
+    // use the full Spell::SendCastResult() path in Spell.cpp instead.
+    class CastResultSimpleFailure final : public ServerPacket


Why don't we pass the Spell* to the packet so it can build the whole structure?

I fear .reload

It will not invalidate the pointers. We read the new data into the existing spell entry.
https://github.com/vmangos/core/blob/development/src/game/Spells/SpellMgr.cpp#L3776

src/game/Server/Packets/Spell.cpp

src/game/Handlers/SkillHandler.cpp

src/game/Handlers/TaxiHandler.cpp

src/game/Server/Packets/Channel.cpp

src/game/Server/Packets/GmTicket.h

ratkosrb · 2026-04-13T09:02:53Z

src/game/Server/Packets/Group.h

+    // --- Server Packets ---
+
+    static constexpr uint8 PARTY_MAX_POSITIVE_AURAS = 32;  // MAX_POSITIVE_AURAS
+    static constexpr uint8 PARTY_MAX_NEGATIVE_AURAS = 16;  // MAX_AURAS - MAX_POSITIVE_AURAS


why do we hardcode this here instead of using the actual defines

Woops these changes were not intended for this PR.
I have a larger PR where I cherry picked some changes.

I would need to import SpellAuraDefines.h which then gets indirectly imported in a lot of places.
(because WorldSession.h has all the packet definitions imported)

src/game/Server/Packets/Petition.cpp

ratkosrb · 2026-04-13T09:05:53Z

src/game/Server/Packets/Petition.cpp

+    buffer << charterEntry;
+    buffer << charterDisplayId;
+    buffer << charterCost;
+    buffer << unknown;


have you tried taking a peak at how this is used to determine it's possible meaning?

Must be & 1 to show the UI. 🤷

src/game/Battlegrounds/BattleGroundMgr.h

src/game/Server/Packets/Misc.h

ratkosrb · 2026-04-13T09:10:13Z

For cases where the client assigns the same handler to multiple packets, reading the same structure, we should use the same packet class for them as well and just pass opcode in constructor, rather than creating duplicate structures.

0blu · 2026-04-13T23:15:19Z

For cases where the client assigns the same handler to multiple packets, reading the same structure, we should use the same packet class for them as well and just pass opcode in constructor, rather than creating duplicate structures.

I would like to prevent mistakes in the code.
If the developer can freely choose an opcode, the struct might be invalid.

I've used NullClientPacket for empty client packets, which I might want to change too.
Just so every packet is uniquely typed.

ratkosrb · 2026-04-13T23:51:10Z

If the developer can freely choose an opcode, the struct might be invalid.

That doesn't prevent us from having invalid structs. I don't like this duplication of code. There are a million movement packets with the same structure.

0blu force-pushed the nice-packets-5-only-some-static branch from 0e94066 to 896c5b3 Compare April 11, 2026 17:16