Skip to content

feat: update current client 15.25 compatibility#4020

Open
dudantas wants to merge 19 commits into
mainfrom
dudantas/protocol-1525-wire-sync
Open

feat: update current client 15.25 compatibility#4020
dudantas wants to merge 19 commits into
mainfrom
dudantas/protocol-1525-wire-sync

Conversation

@dudantas

@dudantas dudantas commented Jul 3, 2026

Copy link
Copy Markdown
Member

Summary

  • Updates the current client runtime profile to the 15.25 byte contract.
  • Gates confirmed current-client payload differences behind runtime feature flags.
  • Adds minimal byte-compatible module shims for new current-client windows and side dialogs.
  • Aligns current-client payloads for resource balances, graphical effects, vocation-specific data, skill wheel, weapon proficiency, game events, and related request parsers.
  • Documents the compatibility scope and versioned payload flags.

Scope

This PR is a compatibility update. It sends and consumes the bytes expected by the current client so sessions can log in and keep running without message-boundary debugs.

It intentionally does not implement the full gameplay systems behind the new windows, dialogs, rewards, shops, progression, or balancing. Those can be added later on top of the stable message shape.

Validation

  • Tested current-client login and key UI surfaces locally while fixing reported client debug cases.
  • Confirmed fixes for skill wheel, character stats, store summary, defense stats, game events, graphical effects, and related current-client payload boundaries.
  • Not run: full compile/build validation.

Summary by CodeRabbit

  • New Features
    • Added client 15.25 byte-contract compatibility gating and expanded official payload support.
    • Introduced minimal official packet shims for Taskboard and Soul Seals, plus expanded GameStore parsing (offer descriptions/events).
    • Added an in-game /protocolprobe tool for protocol/message testing.
  • Bug Fixes
    • Improved packet parsing robustness with stricter trailing-byte handling and updated quest tracker validation/logging.
    • Refined wheel gem reveal and client-specific quest bonus payload formatting.
  • Gameplay/Balance Updates
    • Scaled weapon proficiency perk values/versions and added new proficiency bonus types (including elemental pierce).
  • Documentation
    • Updated systems docs and added guidance for the 15.25 compatibility update and multiprotocol feature notes.

Greptile Summary

This PR updates the server's network protocol layer to be byte-compatible with the Tibia 15.25 client, adding six new ProtocolFeature flags to gate payload differences and introducing minimal shim handlers for Taskboard, SoulSeals, and several previously-unhandled opcodes.

  • Six new ProtocolFeature flags gate 15.25-specific payloads (skill wheel layout, weapon proficiency detail list, graphical effect source byte, vocation-specific data, taskboard/soulseals packet families); the current profile mask is updated to include all of them.
  • New Lua modules for Taskboard (0x5F) and SoulSeals (0xBA) respond with structurally valid but empty windows, gated by the new feature flags; ~15 previously-default: break C++ opcodes now have explicit parsers.
  • A god-level /protocolprobe talkaction is added for live protocol testing, plus fixes for the levelPercent cast precision, a getForgeSkillStat argument placement bug, and the shared-global DEFAULT_VALUE_ENTRIES_PER_PAGE state mutation.

Confidence Score: 3/5

The change is mostly additive and well-guarded, but the questtrack trailing-bytes regression silently breaks quest-tracker sync for any client profile still sending the old 3-byte footer.

The questtrack module is registered globally and runs for all protocol profiles. Lowering the required-bytes gate from +3 to +2 while adding a strict trailing-bytes rejection means every client sending the previously-accepted extra byte has its quest-tracker updates silently discarded with no player-visible error. This affects tibia1100-profile connections using the old footer format. The rest of the dispatch, feature-flag, and shim work is well-structured, but this unguarded regression in a globally-active module requires caution before merging.

data/modules/scripts/questtrack/questtrack.lua (trailing-bytes rejection affects all protocol profiles) and src/server/network/protocol/protocolgame.cpp (parseContainerAction and sendMonkData switch).

Important Files Changed

Filename Overview
src/server/network/protocol/protocolgame.cpp Core protocol dispatch and packet handlers updated for 15.25 byte contract; new parsers added for ~15 previously-unhandled opcodes, several with missing profile guards; critical fixes to levelPercent cast, getForgeSkillStat args, and global store state mutation.
src/server/network/protocol/protocol_profile.hpp Six new ProtocolFeature flags added (OfficialTaskboardPackets, OfficialVocationSpecificPlayerData, OfficialWeaponProficiencyPayload, GraphicalEffectSourceByte, OfficialSoulSealsPackets, OfficialSkillWheelPayload) for fine-grained 15.25 gating.
data/modules/scripts/questtrack/questtrack.lua Required-bytes gate lowered from +3 to +2 and a strict trailing-bytes guard added; silently rejects legacy clients still sending the old 3-byte footer, breaking quest-tracker sync for tibia1100-profile connections.
data/libs/gamestore/parsers.lua Global DEFAULT_VALUE_ENTRIES_PER_PAGE mutation removed (good fix); new offer-description and store-event handlers added; parseBuyStoreOffer refactored to read configuredString/configuredByte before the offer-type switch.
src/creatures/players/components/wheel/player_wheel.cpp OfficialSkillWheelPayload gates new quest-bonus u16 and gem-list layout; sendGemAtelierGemRevealed called after revealGem stores the UUID; all gem action handlers already call sendOpenWheelWindow internally.
data/modules/scripts/taskboard/taskboard.lua New official-client shim for 15.25 Taskboard (0x5F/0x5B); sends structurally valid but empty Bounty/Weekly/Shop windows; action payload consumed before trailing-bytes validation.
data/modules/scripts/soulseals/soulseals.lua New official-client shim for 15.25 SoulSeals (0xBA); reads raceId, validates trailing bytes, responds with empty raceId list placeholder.
src/lua/modules/modules.cpp executeOnRecvbyte refactored to return bool and accept a shared_ptr directly; linear scan over all modules replaced with direct map lookup via getEventByRecvbyte; clean improvement.
data/scripts/talkactions/god/protocol_probe.lua New god-only /protocolprobe talkaction for live protocol testing; includes a hand-rolled JSON decoder, built-in probes, and file-driven local probes; restricted to god group type.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Client packet arrives] --> B[parsePacket reads recvbyte]
    B --> C{recvbyte == 0xD3?}
    C -- yes --> D[parseSetOutfit direct module dispatch]
    C -- no --> E{shouldDispatchRecvbyte\nModuleForProfile?}
    E -- 0x5F + OfficialTaskboardPackets --> F[Lua taskboard.lua shim]
    E -- 0xBA + OfficialSoulSealsPackets --> G[Lua soulseals.lua shim]
    E -- 0xD0 all profiles --> H[Lua questtrack.lua]
    E -- other --> K{Module found?}
    K -- yes, returns true --> L[return: module owns byte]
    K -- no --> M[parsePacketFromDispatcher]
    F --> L
    G --> L
    H --> L
    M --> N{opcode switch}
    N -- 0x5F legacy --> O[parseTaskHuntingAction]
    N -- 0xBA legacy --> P[parseSoulSealsFightMonster]
    N -- 0xC4 --> Q{OfficialWeaponProficiency\nPayload?}
    Q -- yes --> R[sendWeaponProficiencyWindow + detail list]
    Q -- no --> S[sendWeaponProficiencyWindow no detail list]
    N -- 0xED --> T{CurrentPayload?}
    T -- yes --> U[read leading byte then parseSendResourceBalance]
    T -- no --> V[parseSendResourceBalance no extra byte]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Client packet arrives] --> B[parsePacket reads recvbyte]
    B --> C{recvbyte == 0xD3?}
    C -- yes --> D[parseSetOutfit direct module dispatch]
    C -- no --> E{shouldDispatchRecvbyte\nModuleForProfile?}
    E -- 0x5F + OfficialTaskboardPackets --> F[Lua taskboard.lua shim]
    E -- 0xBA + OfficialSoulSealsPackets --> G[Lua soulseals.lua shim]
    E -- 0xD0 all profiles --> H[Lua questtrack.lua]
    E -- other --> K{Module found?}
    K -- yes, returns true --> L[return: module owns byte]
    K -- no --> M[parsePacketFromDispatcher]
    F --> L
    G --> L
    H --> L
    M --> N{opcode switch}
    N -- 0x5F legacy --> O[parseTaskHuntingAction]
    N -- 0xBA legacy --> P[parseSoulSealsFightMonster]
    N -- 0xC4 --> Q{OfficialWeaponProficiency\nPayload?}
    Q -- yes --> R[sendWeaponProficiencyWindow + detail list]
    Q -- no --> S[sendWeaponProficiencyWindow no detail list]
    N -- 0xED --> T{CurrentPayload?}
    T -- yes --> U[read leading byte then parseSendResourceBalance]
    T -- no --> V[parseSendResourceBalance no extra byte]
Loading

Reviews (5): Last reviewed commit: "chore: merge main into protocol 15.25 br..." | Re-trigger Greptile

dudantas added 4 commits July 2, 2026 11:16
Adds new weapon proficiencies for the Crypt set (ProficiencyId 457-463) with full level perk tables, and updates item appearance data to match. Rebalances multiple existing proficiencies by increasing key Type 27 values (including Sanguine, Cobra, Lion, Falcon, Avenger, and Phantasmal lines), adjusts specific augment penalties (e.g., -150 to -30), and bumps affected proficiency versions.
Copilot AI review requested due to automatic review settings July 3, 2026 00:05
@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR updates client 15.25 compatibility across protocol features, packet routing, and Lua shims, while also rebasing weapon proficiency item data and related enum/parser support.

Changes

Protocol and packet handling update

Layer / File(s) Summary
Client version and feature flags
src/core.hpp, src/server/server_definitions.hpp, src/server/network/protocol/protocol_profile.hpp, src/server/network/protocol/protocol_profile.cpp, src/server/network/protocol/protocolgame.hpp
CLIENT_VERSION is updated to 1525, resource ids change at the 0x56/0x57 boundary, new ProtocolFeature bits are added, currentProfile enables the new flags, and ProtocolGame declarations gain the new parsing/sending entry points plus clientVersionString.
Modules recvbyte execution
src/lua/modules/modules.cpp, src/lua/modules/modules.hpp
Modules::getEventByRecvbyte becomes const, and recvbyte execution now uses a player-aware helper with module/type/permission checks.
Recvbyte routing and CurrentPayload handlers
src/server/network/protocol/protocolgame.cpp
parsePacket/parsePacketFromDispatcher add new recvbyte routes and payload-aware handlers for client options, object info, transaction details, weapon proficiency, cyclopedia, wheel, screenshots, and other protocol flows.
Player-side feature gating
src/creatures/players/player.cpp, src/creatures/players/components/wheel/player_wheel.cpp
Task hunting, gem reveal notifications, and the Way of the Monk wheel bonus payload are gated or encoded based on protocol feature support.
Taskboard, Soul Seals, and probe shims
data/modules/scripts/taskboard/taskboard.lua, data/modules/scripts/soulseals/soulseals.lua, data/modules/modules.xml, data/modules/scripts/questtrack/questtrack.lua, data/libs/gamestore/constants.lua, data/libs/gamestore/parsers.lua, data/modules/scripts/gamestore/init.lua, docs/systems/*, .gitignore, data/scripts/talkactions/god/protocol_probe.lua, data/scripts/talkactions/god/protocol_probe.example.json
Adds the taskboard and soul seals recvbyte scripts, registers them, extends gamestore parsing, adjusts quest-tracker byte handling, adds compatibility docs, and adds the protocol probe tool plus its local ignore rule.
Compatibility docs
docs/systems/README.md, docs/systems/client-15-25-compatibility-update.md, docs/systems/multiprotocol.md
Adds the 15.25 compatibility guide and updates the systems docs index plus versioned payload feature notes.

Weapon Proficiency Data Rebalancing

Layer / File(s) Summary
Sanguine perk value updates
data/items/proficiencies.json
Increases perk Value from 5.0 to 10.0 with Version increments across Sanguine and Grand Sanguine entries.
Cobra/Lion/Falcon scaling updates
data/items/proficiencies.json
Increases perk Value (0.1→0.2, 0.14→0.28) with Version bumps.
Negative perk value adjustments
data/items/proficiencies.json
Adjusts large negative perk Value from -150.0 to -30.0 with Version increments.
New Crypt entries and related enum support
data/items/proficiencies.json, src/enums/weapon_proficiency.hpp, src/creatures/players/components/weapon_proficiency.cpp, data-otservbr-global/monster/quests/the_secret_library/bosses/grand_master_oberon.lua
Adds new Crypt weapon proficiency objects, extends weapon proficiency bonus types and parsing, and switches a boss loot entry to reference an item id.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers: majestyotbr, phacUFPE

Poem

A hop, a byte, a version raised so high,
New flags unfurl beneath the client sky,
Task boards and soul seals thump along the trail,
Sanguine blades now gleam, and Crypts prevail,
This bunny twitches whiskers, pleased and bright. 🐇✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: bringing the current client runtime up to 15.25 compatibility.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dudantas/protocol-1525-wire-sync

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@dudantas dudantas changed the title feat: protocol 15.25 feat: update current client 15.25 compatibility Jul 3, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates Canary’s “current” protocol profile and message handlers to align with Tibia client 15.25 byte contracts, focusing on login/gameplay compatibility (opcode shapes, field widths, and feature-gated payload variations) rather than full gameplay system implementation.

Changes:

  • Bumps advertised client version to 1525 and expands the current runtime protocol profile with new ProtocolFeature flags for 15.21–15.25 payload differences.
  • Extends ProtocolGame parsing/sending to match 15.25 packet layouts (resource balances, wheel payload, graphical effects source byte, vocation-specific data, etc.), including minimal Lua recvbyte shims for new client modules.
  • Adds/updates documentation describing versioned payload feature gating and the intended “byte-compatibility first” scope.

Reviewed changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/server/server_definitions.hpp Adds new resource IDs for 15.25 resource balance types.
src/server/network/protocol/protocolgame.hpp Declares new 15.25-related parse/send handlers and stores a client build string.
src/server/network/protocol/protocolgame.cpp Implements 15.25 opcode parsing/writing, feature gating, and module dispatch adjustments.
src/server/network/protocol/protocol_profile.hpp Adds new ProtocolFeature flags and clarifies versioned feature documentation expectations.
src/server/network/protocol/protocol_profile.cpp Enables new 15.25-related protocol features on the “current” profile.
src/creatures/players/player.cpp Avoids sending legacy task hunting base data when official taskboard packets are enabled.
src/creatures/players/components/wheel/player_wheel.cpp Adjusts wheel payload layout for official 15.25 skill wheel contract and adds a gem reveal notification.
src/core.hpp Updates CLIENT_VERSION constant to 1525.
docs/systems/README.md Adds a new system doc link and tweaks wording (“message handlers”).
docs/systems/multiprotocol.md Documents “versioned payload features” guidance and records known feature/version mappings.
docs/systems/client-15-25-compatibility-update.md New documentation describing scope, non-goals, and validation checklist for the 15.25 compatibility work.
docs/lua-api/lua_api.md Updates generated Lua API docs for House:startTrade.
docs/lua-api/lua_api.json Updates generated Lua API JSON for House:startTrade parameter list.
docs/lua-api/lua_api.d.lua Updates Lua API stub for House:startTrade.
data/modules/scripts/taskboard/taskboard.lua Adds minimal official-client taskboard recvbyte shim (0x5F → 0x5B).
data/modules/scripts/soulseals/soulseals.lua Adds minimal official-client soul seals recvbyte shim for 0xBA.
data/modules/scripts/questtrack/questtrack.lua Adjusts quest tracker recvbyte parsing to match updated trailing fields.
data/modules/modules.xml Registers new recvbyte modules for taskboard (0x5F) and soul seals (0xBA).
data/libs/gamestore/parsers.lua Ensures unread bytes are consumed after parsing gamestore packets.
data/items/proficiencies.json Updates weapon proficiency data/versions and appends new entries to match updated client data expectations.

Comment thread src/server/network/protocol/protocolgame.cpp Outdated
Comment thread src/server/network/protocol/protocolgame.cpp
Comment thread src/server/network/protocol/protocolgame.cpp
Comment thread src/server/network/protocol/protocolgame.cpp
Comment thread src/server/network/protocol/protocolgame.cpp
Comment thread src/server/network/protocol/protocolgame.cpp Outdated
Comment thread src/server/network/protocol/protocolgame.cpp Outdated
@gemini-code-assist

Copy link
Copy Markdown

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
src/server/network/protocol/protocolgame.cpp (2)

1975-1977: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Case 0xDB breaks ascending opcode ordering.

The new case 0xDB is inserted between case 0xD6 and case 0xD7, out of the otherwise ascending numeric order used throughout this switch. Purely cosmetic (compiles fine), but worth moving for readability.

♻️ Suggested reordering
 		case 0xD6:
 			parseClearImbuement(msg);
 			break;
-		case 0xDB:
-			parseCyclopediaMapAction(msg);
-			break;
 		case 0xD7:
 			parseCloseImbuementWindow(msg);
 			break;
+		case 0xDB:
+			parseCyclopediaMapAction(msg);
+			break;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/network/protocol/protocolgame.cpp` around lines 1975 - 1977, The
new 0xDB branch in protocolgame.cpp breaks the ascending opcode order in the
switch. Move the parseCyclopediaMapAction(msg) case within the protocol dispatch
switch so it sits in numeric order with the surrounding opcode cases (around the
existing 0xD6 and 0xD7 handlers), keeping the ordering consistent with the rest
of the switch.

3137-3259: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add focused tests for the new feature-gated packet handlers.

This cohort introduces a large batch of new parse/send handlers gated by ProtocolFeature flags (weapon proficiency detail list, wheel gem reveal, resource balance u32 encoding, object info, monk data encoding, etc.) without accompanying tests. Given the protocol-desync risk if any of these byte-layouts are wrong, focused unit/integration tests around the new feature-gated branches would materially reduce regression risk.

As per coding guidelines, "For C++ changes, prefer focused tests or the smallest practical build/check that validates the touched code."

Also applies to: 4209-4247, 11484-11523

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/network/protocol/protocolgame.cpp` around lines 3137 - 3259, Add
focused tests for the new ProtocolGame packet parsers so the feature-gated byte
layouts are validated end-to-end. Cover the new branches in
ProtocolGame::parseSetClientOptions, parseCharacterTradeConfigurationAction,
parseBossDifficultySelection, parseInspectPlayer, parseCyclopediaMapAction, and
parseSetHirelingName by asserting they consume the expected fields only when the
relevant action/ProtocolFeature conditions are met. Prefer the smallest
practical unit or protocol integration tests that exercise both enabled and
disabled feature paths to catch desync regressions.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@data/items/proficiencies.json`:
- Around line 26280-27088: The new Crypt proficiencies are using invalid Type:
31 perk entries, which are rejected by addStat() because
WeaponProficiencyBonus_t only supports 0–27. Update the Crypt 2H Sword, Crypt 2H
Axe, Crypt 2H Club, Crypt 2H Fist, Crypt 2H Bow, Crypt 1H Wand, and Crypt 1H Rod
entries so the elemental bonuses are expressed with the correct supported type
used elsewhere in these records, and keep ElementId/Type: 13 mappings unchanged
where they already work.

In `@docs/lua-api/lua_api.d.lua`:
- Around line 1179-1180: Restore the missing player and tradePartner parameters
in House:startTrade’s Lua annotation so the API matches the native binding and
script usage. Update the function signature in House:startTrade to accept those
two arguments, keeping it aligned with the argument handling in
house_functions.cpp and the call site in sell_house.lua. Ensure the documented
return stays unchanged.

---

Nitpick comments:
In `@src/server/network/protocol/protocolgame.cpp`:
- Around line 1975-1977: The new 0xDB branch in protocolgame.cpp breaks the
ascending opcode order in the switch. Move the parseCyclopediaMapAction(msg)
case within the protocol dispatch switch so it sits in numeric order with the
surrounding opcode cases (around the existing 0xD6 and 0xD7 handlers), keeping
the ordering consistent with the rest of the switch.
- Around line 3137-3259: Add focused tests for the new ProtocolGame packet
parsers so the feature-gated byte layouts are validated end-to-end. Cover the
new branches in ProtocolGame::parseSetClientOptions,
parseCharacterTradeConfigurationAction, parseBossDifficultySelection,
parseInspectPlayer, parseCyclopediaMapAction, and parseSetHirelingName by
asserting they consume the expected fields only when the relevant
action/ProtocolFeature conditions are met. Prefer the smallest practical unit or
protocol integration tests that exercise both enabled and disabled feature paths
to catch desync regressions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8d403d98-a14f-45bd-bbf1-f76cee50f319

📥 Commits

Reviewing files that changed from the base of the PR and between 82b4778 and 6bd5080.

⛔ Files ignored due to path filters (1)
  • data/items/appearances.dat is excluded by !**/*.dat
📒 Files selected for processing (20)
  • data/items/proficiencies.json
  • data/libs/gamestore/parsers.lua
  • data/modules/modules.xml
  • data/modules/scripts/questtrack/questtrack.lua
  • data/modules/scripts/soulseals/soulseals.lua
  • data/modules/scripts/taskboard/taskboard.lua
  • docs/lua-api/lua_api.d.lua
  • docs/lua-api/lua_api.json
  • docs/lua-api/lua_api.md
  • docs/systems/README.md
  • docs/systems/client-15-25-compatibility-update.md
  • docs/systems/multiprotocol.md
  • src/core.hpp
  • src/creatures/players/components/wheel/player_wheel.cpp
  • src/creatures/players/player.cpp
  • src/server/network/protocol/protocol_profile.cpp
  • src/server/network/protocol/protocol_profile.hpp
  • src/server/network/protocol/protocolgame.cpp
  • src/server/network/protocol/protocolgame.hpp
  • src/server/server_definitions.hpp

Comment thread data/items/proficiencies.json
Comment thread docs/lua-api/lua_api.d.lua Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/lua/modules/modules.cpp (1)

83-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant zero-id check after key-based lookup.

modulePtr is fetched via getEventByRecvbyte(byte, false), and since registerEvent always stores modules keyed by modulePtr->getRecvbyte(), modulePtr->getRecvbyte() == 0 can only be true when byte == 0. The check works, but it's a roundabout way to say "reject byte 0" — a plain if (byte == 0) { ...; return false; } before the lookup would be clearer and avoid an unnecessary map lookup for byte 0. Not a functional issue.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lua/modules/modules.cpp` around lines 83 - 104, The zero-id guard in
Modules::executeOnRecvbyte is redundant after the key-based lookup and should be
handled directly. Add an explicit early check for byte == 0 at the start of
executeOnRecvbyte, log the same invalid-module message there, and return false
before calling getEventByRecvbyte; then remove the modulePtr->getRecvbyte() == 0
branch and keep the rest of the MODULE_TYPE_RECVBYTE and canRunModule flow
unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/server/network/protocol/protocolgame.cpp`:
- Around line 4664-4665: The level percent sent from protocolgame’s player stats
block is truncating before scaling because the cast is applied too early. Update
the `player->getLevelPercent()` calculation in the same `protocolGame` method so
it multiplies by 100 first and only then clamps/writes the value, matching the
existing `player->getMagicLevelPercent()` and `player->getSkillPercent()`
handling.

---

Nitpick comments:
In `@src/lua/modules/modules.cpp`:
- Around line 83-104: The zero-id guard in Modules::executeOnRecvbyte is
redundant after the key-based lookup and should be handled directly. Add an
explicit early check for byte == 0 at the start of executeOnRecvbyte, log the
same invalid-module message there, and return false before calling
getEventByRecvbyte; then remove the modulePtr->getRecvbyte() == 0 branch and
keep the rest of the MODULE_TYPE_RECVBYTE and canRunModule flow unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cedc2fb2-d425-4e2e-b814-91143004e221

📥 Commits

Reviewing files that changed from the base of the PR and between 6bd5080 and ded57a5.

📒 Files selected for processing (7)
  • data-otservbr-global/monster/quests/the_secret_library/bosses/grand_master_oberon.lua
  • src/creatures/players/components/weapon_proficiency.cpp
  • src/enums/weapon_proficiency.hpp
  • src/lua/modules/modules.cpp
  • src/lua/modules/modules.hpp
  • src/server/network/protocol/protocolgame.cpp
  • src/server/network/protocol/protocolgame.hpp
💤 Files with no reviewable changes (1)
  • src/server/network/protocol/protocolgame.hpp

Comment thread src/server/network/protocol/protocolgame.cpp Outdated
@dudantas dudantas force-pushed the dudantas/protocol-1525-wire-sync branch from 5837622 to ded57a5 Compare July 3, 2026 01:46
Comment thread src/server/network/protocol/protocolgame.cpp
Comment thread src/server/network/protocol/protocolgame.cpp
dudantas added 3 commits July 2, 2026 23:12
Adds `/protocolprobe` and `/probeopcode` for sending built-in or JSON-defined protocol payloads, with help/list output and local-file loading on each command. Also adds an example probe file and ignores the local probe override file in git.
Adjust game protocol parsing and serialization for CurrentPayload clients. This skips optional channel trailer data, accepts 32-bit imbuement IDs safely, consumes client-check payload bytes when present, and omits the legacy PvP mode byte on outgoing fight mode packets.
Update gamestore packet handling to match newer client payloads. This adds explicit handlers for offer description and store event packets, consumes extra fields in store-offer requests (including protocol tail bytes), and reads configurable purchase data before processing name/hireling-related offers. It also reads the entries-per-page byte for transaction history and sends initial category offers directly on store open, replacing the previous unread-byte skip fallback with targeted parsing.
@zoelner

zoelner commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

@dudantas How to test this version? You have a client to evaluate this PR?

@dudantas

dudantas commented Jul 3, 2026

Copy link
Copy Markdown
Member Author

@dudantas How to test this version? You have a client to evaluate this PR?

The same repo as the others' client: https://github.com/dudantas/tibia-client/releases/tag/15.25.0a00a0

dudantas and others added 3 commits July 2, 2026 23:48
Updates taskboard action decoding to treat ShopBuy, UnlockPreferenceSlot, ClearPreferred, and ClearUnwanted as U16 payload actions instead of one-byte payloads. In protocol handling, replaces hardcoded resource IDs (0x3C/0x50) with named Resource_t entries, wires them into the valid current-resource switch, and adds the corresponding enum constants. Also includes small protocol cleanups: clearer client-check/news field comments, a more descriptive gem index parameter name, and explicit Virtue payload count handling in monk data serialization.
@opentibiabr opentibiabr deleted a comment from sonarqubecloud Bot Jul 3, 2026
Comment thread src/server/network/protocol/protocolgame.cpp
Comment thread data/libs/gamestore/parsers.lua Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
src/server/network/protocol/protocolgame.cpp (1)

2367-2377: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Make the discarded payload field explicit.

These get<uint16_t>() calls intentionally advance the cursor, but Sonar flags the ignored return. Cast the result to void or use a skip/discard helper if one exists.

Proposed cleanup
 	if (hasProtocolFeature(protocolProfile, ProtocolFeature::CurrentPayload) && getUnreadBytes(msg) >= sizeof(uint16_t)) {
-		msg.get<uint16_t>();
+		static_cast<void>(msg.get<uint16_t>());
 	}
@@
 	if (hasProtocolFeature(protocolProfile, ProtocolFeature::CurrentPayload) && getUnreadBytes(msg) >= sizeof(uint16_t)) {
-		msg.get<uint16_t>();
+		static_cast<void>(msg.get<uint16_t>());
 	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/network/protocol/protocolgame.cpp` around lines 2367 - 2377, The
`ProtocolGame::parseChannelInvite` and `ProtocolGame::parseChannelExclude`
payload-skipping reads intentionally discard a `uint16_t`, but the ignored
return value is flagged. Update these `msg.get<uint16_t>()` calls to make the
discard explicit by casting the result to void, or replace them with an existing
skip/discard helper if `NetworkMessage` provides one, while keeping the cursor
advance behavior unchanged.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@data/libs/gamestore/parsers.lua`:
- Around line 421-422: The transaction-history request in
sendStoreTransactionHistory handling is mutating
GameStore.DefaultValues.DEFAULT_VALUE_ENTRIES_PER_PAGE from the client-provided
byte, which makes one player’s request affect all future requests. Keep the
requested page size local to the current request in the parsers.lua flow, and
pass that local value into sendStoreTransactionHistory instead of writing it
into GameStore.DefaultValues.
- Around line 249-256: Move the configured-field reads in the purchase parsing
flow into the same guarded block used for the purchase handling so truncated
packets are caught by the existing recovery path. In the parser logic around the
configuredString/configuredByte handling in the purchase branch, defer
msg:getString() and msg:getByte() until inside the pcall-protected section and
keep the productType checks there so any malformed configured-purchase packet is
handled consistently.

In `@data/scripts/talkactions/god/protocol_probe.lua`:
- Around line 411-440: The u64 and i64 branches in protocol_probe.lua are still
validating against 32-bit limits, so larger values are rejected before reaching
NetworkMessage:addU64 and NetworkMessage:add64. Update the requireInteger bounds
in the fieldType == "u64" and fieldType == "i64" cases to use full 64-bit
ranges, keeping the existing error handling and addU64/add64 calls unchanged.

---

Nitpick comments:
In `@src/server/network/protocol/protocolgame.cpp`:
- Around line 2367-2377: The `ProtocolGame::parseChannelInvite` and
`ProtocolGame::parseChannelExclude` payload-skipping reads intentionally discard
a `uint16_t`, but the ignored return value is flagged. Update these
`msg.get<uint16_t>()` calls to make the discard explicit by casting the result
to void, or replace them with an existing skip/discard helper if
`NetworkMessage` provides one, while keeping the cursor advance behavior
unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 015be454-c7f5-4d85-b531-b363e970adf1

📥 Commits

Reviewing files that changed from the base of the PR and between ded57a5 and 93aa74e.

📒 Files selected for processing (10)
  • .gitignore
  • data/libs/gamestore/constants.lua
  • data/libs/gamestore/parsers.lua
  • data/modules/scripts/gamestore/init.lua
  • data/modules/scripts/taskboard/taskboard.lua
  • data/scripts/talkactions/god/protocol_probe.example.json
  • data/scripts/talkactions/god/protocol_probe.lua
  • src/server/network/protocol/protocolgame.cpp
  • src/server/network/protocol/protocolgame.hpp
  • src/server/server_definitions.hpp
✅ Files skipped from review due to trivial changes (2)
  • data/scripts/talkactions/god/protocol_probe.example.json
  • .gitignore
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/server/server_definitions.hpp
  • data/modules/scripts/taskboard/taskboard.lua
  • src/server/network/protocol/protocolgame.hpp

Comment thread data/libs/gamestore/parsers.lua Outdated
Comment thread data/libs/gamestore/parsers.lua Outdated
Comment thread data/scripts/talkactions/god/protocol_probe.lua
dudantas added 2 commits July 3, 2026 00:13
Move payload-size parsing behind the CurrentPayload feature gate so unsupported clients are ignored without consuming bytes. Also update inline comments to document the confirmed wire format and clarify that the sent byte is a raw client-check probe.
Add strict trailing-byte validation in questtrack, soulseals, and taskboard packet handlers. Each handler now logs and ignores malformed client packets that contain unexpected unread bytes, preventing partial/ambiguous payloads from being processed. In soulseals, this also removes the old behavior that silently skipped extra bytes.
Comment thread src/server/network/protocol/protocolgame.cpp
Comment thread data/libs/gamestore/parsers.lua
@sonarqubecloud

sonarqubecloud Bot commented Jul 3, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants