refactor: encapsule message builders for Cyclopedia, Bosstiary and others #3721
Conversation
…hers Introduces new message builder classes for Cyclopedia Character, House, Outfit, Imbuement, Prey, and Bosstiary features in the network protocol. Updates CMakeLists.txt to include these new source files. Refactors Player class to expose marriage spouse ID and item type count methods for protocol use.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors network protocol code by introducing new message builder classes for Cyclopedia Character, House, Outfit, Imbuement, Prey, and Bosstiary features. The refactoring improves code organization and maintainability by extracting large amounts of inline message construction code into dedicated builder classes.
Key changes:
- Extracts complex message building logic from ProtocolGame class into specialized builder classes
- Adds new CMakeLists.txt entries for the new message builder source files
- Exposes Player class methods (marriage spouse ID and item type count) for protocol use
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/server/network/protocol/protocolgame.hpp | Removes imbuement method declaration and adds forward declaration for CyclopediaCharacterMessageBuilder |
| src/server/network/protocol/protocolgame.cpp | Replaces inline message building with builder class calls and removes large message construction functions |
| src/server/network/protocol/builders/*.hpp | New header files defining message builder class interfaces |
| src/server/network/protocol/builders/*.cpp | New implementation files containing extracted message building logic |
| src/server/CMakeLists.txt | Adds new message builder source files to build |
| src/creatures/players/player.hpp | Moves getItemTypeCount to public section and adds getMarriageSpouseId method |
| src/creatures/players/player.cpp | Implements new getMarriageSpouseId method |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| std::string name = toStartCaseWithSpace(magic_enum::enum_name(bless).data()); | ||
| msg.addString(name); | ||
| auto blessValue = enumToValue(bless); | ||
| msg.addByte(player->hasBlessing(blessValue) ? player->getBlessingCount(blessValue) : 0x00); |
There was a problem hiding this comment.
The code calls player->getBlessingCount(blessValue) but this method doesn't exist in the diff. The original code used player->blessings[blessValue - 1]. This will cause a compilation error.
| msg.addByte(player->hasBlessing(blessValue) ? player->getBlessingCount(blessValue) : 0x00); | |
| msg.addByte(player->hasBlessing(blessValue) ? player->blessings[blessValue - 1] : 0x00); |
| if (const auto spouseId = player->getMarriageSpouseId(); spouseId > 0) { | ||
| if (const auto &spouse = g_game().getPlayerByID(spouseId, true); spouse) { | ||
| playerDescriptionSize++; | ||
| msg.addString("Married to"); | ||
| msg.addString(spouse->getName()); | ||
| } |
There was a problem hiding this comment.
The method getMarriageSpouseId() is called but the original code used getMarriageSpouse(). This inconsistency will cause a compilation error since the old method was removed.
| if (const auto spouseId = player->getMarriageSpouseId(); spouseId > 0) { | |
| if (const auto &spouse = g_game().getPlayerByID(spouseId, true); spouse) { | |
| playerDescriptionSize++; | |
| msg.addString("Married to"); | |
| msg.addString(spouse->getName()); | |
| } | |
| if (const auto spouse = player->getMarriageSpouse(); spouse) { | |
| playerDescriptionSize++; | |
| msg.addString("Married to"); | |
| msg.addString(spouse->getName()); |
| continue; | ||
| } | ||
|
|
||
| if (imbuementInfo.duration == 0) { |
There was a problem hiding this comment.
The original code checked 'imbuementInfo.duration > 0' but this checks for '== 0' and continues. This inverts the logic and will cause imbuements with no duration to be processed instead of skipped.
| if (imbuementInfo.duration == 0) { | |
| if (imbuementInfo.duration <= 0) { |
|
|
This PR is stale because it has been open 45 days with no activity. |
Introduces new message builder classes for Cyclopedia Character, House, Outfit, Imbuement, Prey, and Bosstiary features in the network protocol. Updates CMakeLists.txt to include these new source files. Refactors Player class to expose marriage spouse ID and item type count methods for protocol use.