docs(protocol): P_AttackActor detail page (combat math + damage broadcast)#288
Merged
Conversation
…cast) Third per-packet detail page (after P_InventoryUpdate #285 and P_StandardUpdate #286). Documents the combat packet -- one of the most security-sensitive surfaces because every PvP outcome flows through its validation chain. ## What the doc covers * **C->S field layout**: 2-byte payload (target RuntimeID), no sub-code. The packet shape is the second-shortest in the protocol (P_ActorGone is the shortest). * **S->C three sub-codes** with per-recipient field layouts: - `"H"` to attacker -- `1B sub + 2B Victim\RuntimeID + 2B Damage+1 + 1B DamageType` - `"Y"` to victim -- `1B sub + 2B Attacker\RuntimeID + 2B Damage+1 + 1B DamageType` - `"O"` to observers -- `1B sub + 2B Attacker\RuntimeID + 2B Victim\RuntimeID` (NO damage payload) * **The Damage+1 encoding trick**: 0 on wire means "miss" (parry animation); client subtracts 1 to recover the actual damage. Lets the engine use a signed 2-byte field to signal "miss" explicitly without burning a separate flag byte. * **DamageType client-side bound** (0..19): protects against wild values out of the malformed-packet space. * **Six C->S validation gates**: sender validity, packet length, CombatDelay anti-spam, mount-blocks-attack, same-area (PR #276), PvP-or-NPC permission. * **ActorAttack damage engine** with the additional engine-level checks (already-dead target guard, Aggressiveness=3 block, faction rating, range check) and the four CombatFormula variants (1-3 fixed formulas + #4 attack-script delegation). * **Three-channel broadcast pattern**: attacker / victim / observers. Observer "O" broadcast does NOT carry damage (re-sync via P_StatUpdate) -- a design choice worth documenting because the current ClientNet "Else" branch reads damage as 0 (uninitialised local) which makes the no-damage decision implicit rather than explicit. ## Historical PRs surfaced * #276 -- same-area gate * #282 -- O(1) sender resolution * #283 -- per-area FirstInZone chain in observer broadcast * #287 -- per-leader FirstSlave chain in pet-aggro recruitment * Two-attackers-same-tick fix (pre-PR-numbered) -- the already-dead guard * AInstance Null defensive check in the broadcast loop ## Verification * `compile.bat -t` clean. * `test.bat` -- 26/26 pass. * `gen_packet_index.sh` regenerated; Detail column links the new page. * Spot-check 3 field layouts against the actual source: H/Y damage emission at GameServer.bb:563-568 + 271-276, observer "O" emission at GameServer.bb:573 + 328, client receive at ClientNet.bb:1117 / 1145. ## Deferred 52 packet detail pages remain. Next recommended candidates: * P_SpellUpdate -- multi-sub-code spell lifecycle (cast / charge / free) * P_ChatMessage -- multi-prefix dispatch (Chr$(252/253/254) channels) * P_VerifyAccount -- the auth state machine (PRs #264-#268) * P_StatUpdate -- HP/stat broadcast (P_AttackActor's observer re-sync channel) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…cit-var framing PR #288 reviewer optional suggestions, addressed before merge: 1. The C->S handler section's lead said "Five gates" but the numbered list had six items (the lead was written before adding the same-area gate from PR #276). Bumped to "Six". 2. The S->C "O" sub-code description claimed observers "read Damage as 0 (uninitialised local)". More precise: `Damage` is an *implicit function-scope* variable in non-Strict `UpdateNetwork()`. It zero-initialises on first entry, but persists across `Select Case` iterations within one call -- so the second P_AttackActor "O" in the same UpdateNetwork tick reads the PRIOR H/Y packet's Damage, not 0. Benign because P_StatUpdate re-syncs HP authoritatively, but the precise framing better captures the non-Strict dispatcher property. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23aa0e71af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | `"Y"` | Victim (`A2\RNID`) | `1B sub + 2B Attacker\RuntimeID + 2B Damage+1 + 1B DamageType` | "They hit me" — for the victim's HUD (incoming damage, parry/hit animation). | | ||
| | `"O"` | All other players in the same area | `1B sub + 2B Attacker\RuntimeID + 2B Victim\RuntimeID` (no damage payload) | Observer swing animation. Observers re-sync HP via the next `P_StatUpdate`, so the damage isn't replicated on this channel. | | ||
|
|
||
| **Damage+1 encoding:** The 2-byte `Damage` field carries `Damage + 1` so a value of 0 on the wire means "miss" (rendered as a parry animation). The wire field can be negative (signed 2-byte = -32768..32767), which lets the server signal a miss explicitly. Client subtracts 1 to recover the actual damage at [ClientNet.bb:1117](../../../src/Modules/ClientNet.bb#L1117) / [:1145](../../../src/Modules/ClientNet.bb#L1145). |
There was a problem hiding this comment.
This sentence documents the 2-byte damage payload as signed and says negatives are used to signal misses, but the actual codec path writes Damage + 1 with RCE_StrFromInt(..., 2) and reads it with RCE_IntFromStr after zeroing upper bytes, so the on-wire value is effectively 0..65535 and miss is encoded as 0 (client then subtracts 1). Keeping the signed-short claim here can mislead alternate client/server implementations into decoding values >= 32768 incorrectly and diverging from server combat outcomes.
Useful? React with 👍 / 👎.
| | `1` (Normal) | 90% | `weapon.Damage ± strength-rolled bonus`, critical 1/10 (×2). Armour subtracts `GetArmourLevel + Resistances[DamageType] - 100 + ToughnessStat / 8`. | | ||
| | `2` (No strength bonus) | 90% | `weapon.Damage` flat. Critical 1/10 (×2). Same armour formula. | | ||
| | `3` (Multiplied) | 90% | `weapon.Damage × Strength`. Critical 1/10. | | ||
| | `4` (Attack script) | N/A | Delegates to a `ThreadScript("Attack", "Main", attacker, victim)` — content authors implement the formula in `.rsl`. No range/damage check server-side. | |
There was a problem hiding this comment.
The table states that formula 4 has no server-side range check, but that is only true for the ranged-weapon scripted branch; melee scripted attacks still pass through the shared distance gate before the CombatFormula = 4 dispatch. This overgeneralization weakens the protocol doc’s accuracy for anti-cheat analysis by implying melee script combat bypasses a guard that is actually enforced.
Useful? React with 👍 / 👎.
This was referenced May 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Third per-packet detail page after #285 (P_InventoryUpdate) and #286 (P_StandardUpdate). Documents the combat packet — one of the most security-sensitive surfaces in the protocol since every PvP outcome flows through its validation chain.
Non-technical
Combat is the highest-stakes wire surface for cheating prevention. This doc captures every gate the server applies between "client says I'm hitting X" and "X loses HP", plus the four formula variants and the three-channel broadcast pattern (attacker / victim / observers).
What's documented
Damage+1encoding trick: 0 on wire = miss; client subtracts 1CombatFormulavariants: Normal / NoStrength / Multiplied / Attack-script-delegatedAcceptance criteria
ActorAttackengine gates documented.bbsource changescompile.bat -tcleantest.bat— 26/26 passTest plan
gen_packet_index.sh --checkidempotentcompile.bat -t+test.bat— passTrade-offs / deferred
Damageas 0 because the wire payload doesn't carry damage for observers — it's an uninitialized local. Documented but not changed (current behavior is correct: animations play, HP re-syncs via P_StatUpdate).🤖 Generated with Claude Code