MoP 5.4.8.18414 opcode campaign: 22 packet waves, quest-log projection, quest-NPC query - #24
Merged
Merged
Conversation
Mechanical implementation of the binary-proven 18414 SMSG_COOLDOWN_EVENT wire format so the MoP client receives the cooldown event instead of a stale Cata body. This adds MopSpellPackets::BuildCooldownEvent, replaces the inherited SpellCooldownMgr body, registers the opcode, admits it through IsEnterWorldConverted, and updates Opcodes_reference.h. Byte-exact fixtures and hostile source mutations are included. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Accepting a quest updated server state but never reached the client's quest log: MopUpdateObject projected only the inventory, skill and buyback self ranges, so the quest-log fields were never sent and QuestLogFrame had nothing to render. Unlike every other self range, this one cannot be shifted by a constant. Four stores fifty five-word quest slots at 166..415 (MAX_QUEST_OFFSET 5), while 18414's CGPlayerData::questLog is fifty FIFTEEN-word slots at 171..920 -- 750 fields, confirmed both by the client descriptor initializers and by the retained 5.4.8 reference table. Only the leading five words of each client slot carry Four's id/state/counts/timer; the remaining ten are MoP objective storage Four does not populate. A flat range copy would therefore land legacy slot 1 on index 176, inside client slot 0, and scramble the log. TranslateSelfQuestLogIndex re-strides per slot instead. Three static_asserts tie the projection to the legacy PLAYER_QUEST_LOG_* constants and to MAX_QUEST_OFFSET so the two layouts cannot silently drift apart. The feed loop is ordered ahead of the visible-item feed because AppendSelfPlayerValuesBlock requires ascending legacy indices. Byte-exact test covers both ends of the block and asserts the flat-copy targets (176, 420) are absent; verified to fail when the translation is replaced with a flat copy. Adds a questlog_feed source mutation. ctest -R ^mop_(updateobject|self_values): 9/9 passed Release mangosd: built successfully Runtime validation remains pending: accept a quest from Marshal McBride (creature 197, Northshire), confirm it renders in the client quest log, and confirm it survives a relog.
mop_world_quest_interaction_packets_source pinned the BroadcastText-aware world-database requirement to 23.1.2, but revision_data.h.in has declared 23.2.31 since the database revision alignment. The check therefore asserted an obsolete tuple and failed on a clean tree; it predates the current upstream merge rather than being caused by it. Moves the pin, and the paired npc_db_content mutation, onto the declared tuple. The requirement is unchanged -- the database must be BroadcastText aware -- but the pin has to move with revision_data.h.in or the check silently guards the wrong version. ctest: 1076/1076 passed
Points the Eluna submodule at dc5059b, which restores WorldSession::SendAuctionHello routing in the Mists arm of SendAuctionMenu. Eluna master advanced to 8eaa22b along a line that never contained fd0b208a3, the commit this repository previously referenced, so the Four specific routing was lost rather than superseded. 8eaa22b also switched the inline packet to SMSG_AUCTION_HELLO, which emits the pre-18414 body under the 18414 opcode and is malformed to a real client. mop_auction_packets_source asserts the routing and fails without this. Upstream review is MangosServer/Eluna#9. ctest: 1076/1076 passed
MangosServer/Eluna#9 was squash merged as e5b40f0, so the branch commit dc5059b this repository referenced is no longer an ancestor of Eluna master and its branch has been deleted upstream. The pointer would not have resolved from a fresh clone. Re-points at e5b40f0. Tree content is identical: git diff dc5059b e5b40f0 is empty. ctest: 1076/1076 passed
Replaces the inherited legacy packed-GUID body for SMSG_PRE_RESURRECT 0x19C0
with the directly proved 18414 masked GUID layout in the owning
Player::BuildPlayerRepop path; registers and admits the packet.
Wire grammar, proved twice over. Dispatcher sub_659694 case 696 constructs
the message via parser sub_709F6B, which reads one packed GUID and nothing
else. Two independent readers agree on the order: the constructor helper
sub_6E7875 and the class virtual deserialize slot sub_6D6EF4.
mask bits [1, 7, 5, 2, 6, 0, 3, 4]
bytes XOR [5, 1, 7, 0, 6, 4, 2, 3]
Opcode identity is derived, not asserted. IDA labels the block calling
sub_709F6B as jumptable case 696; the same number falls out of the selector
arithmetic independently, since 0x19C0 & 0x125C == 0x1040 selects the +608
family and the free bits ~0x125C & 0x1FFF compact to 88. The model reproduces
every neighbouring case in that table exactly (0x0CBF->319, 0x1E0E->578,
0x1E2E->586), and sub_709F6B inverts to exactly one opcode, so there is no
shared-handler ambiguity. The NAME remains reference-consensus, not binary.
KNOWN EVIDENCE GAP: the semantic terminal is unresolved. The consumer is
reachable only through sub_6D1F55, a per-message Arxan guard trampoline whose
target is assembled at runtime as (dword_1096FB4 - (v2 ^ v3)). This is not a
shared stub that could be resolved once -- there are 881 distinct trampolines
across 881 accepted opcodes, this one used exactly once. The GUID therefore
carries the role Four already gave it rather than a binary-proved role; this
wave changes the encoding, not the semantics. The inherited body could not
have been read by an 18414 client at all, so this cannot regress behaviour.
RED: build failed only because MopCompactPackets::BuildPreResurrect was absent
GREEN: dense and sparse byte fixtures passed, including the shortened body
when zero GUID bytes clear their mask bits
ctest full suite: 1082/1082 passed (6 new hostile source mutations)
Release mangosd: built successfully
reference overlay: ACTIVE=397, DOC=438, DORMANT=685, idempotent
git diff --check: clean
canonical Wow.exe.i64 sha256 unchanged across all three IDA passes
Runtime validation pending: die and release spirit, retain 0x19C0, and confirm
the client reaches the ghost/pre-resurrect state with no malformed-packet
disconnect.
The 18414 client sends CMSG_QUEST_NPC_QUERY 0x1DAE on every quest accept,
once per quest per session. The opcode was never promoted out of
Opcodes_reference.h, so the server logged it as unhandled and answered
nothing, even though the reply half SMSG_QUEST_NPC_QUERY_RESPONSE 0x036D was
already carried in Opcodes.h.
Request. Operator captures show a fixed 204-byte body of which only the
leading uint32 quest id is initialised; the remainder is uninitialised client
stack memory, byte-identical across captures within one run and containing
x64 image pointers on the 64-bit client. The parser reads the one defined
field and deliberately discards the rest rather than validating a length the
client never populates. Three captures confirm the client queries one quest
at a time: the body did not grow when a second quest was held.
Response grammar, from client parser sub_6B8B3B -> sub_6B8A06:
bits21 questCount
x questCount: bits22 npcCount
(flush)
x questCount: uint32 questId, then npcCount x uint32 npcId
Widths are read from the composing shifts: sub_6A29A8 shift 13 gives 21 bits,
sub_695EBC shift 14 gives 22. Confirmed against real 18414 retail captures --
every SMSG_QUEST_NPC_QUERY_RESPONSE in the sniff corpus (1,296 quest entries)
decodes under this reader leaving no residual byte, and the builder now
reproduces one of those replies byte-for-byte at 90 bytes.
NPC selection is ENDERS, established from retail rather than assumed. Quest
28508 is a chain hand-off with disjoint sets -- giver 42898, ender 44452 --
and retail returned 44452. Givers-only and deduplicated-union both fail that
case outright; enders-only matches all fifteen corpus quests for which the
world database has ender data. Answering by quest needs an inverse of
m_CreatureQuestInvolvedRelations, built from the same rows at load so it
cannot drift and adds no query or table-name coupling.
Retail batches several quests into one reply although the client asks one at
a time, so the builder takes a vector and a single-quest answer is the
degenerate case of the same grammar, not a special form.
RED: build failed only because ParseQuestNpcQueryRequest and
BuildQuestNpcQueryResponse were absent
GREEN: dense, empty-ender, no-quest and retail-shape fixtures passed
ctest full suite: 1083/1083 passed
Release mangosd: built successfully
reference overlay: ACTIVE=399, DOC=437, DORMANT=684, idempotent
git diff --check: clean
Known limits, recorded rather than papered over. The corpus is entirely
actor 0, so whether gameobject questgivers belong in this response is
untested and deliberately not implemented. Quests whose ender rows are
missing or wrong in the converted world database will return an empty list;
33147 is a known instance and the server already flags its giver row at
startup. That is a content gap, not a protocol one.
Runtime validation pending: accept quest 29080 (giver 197 Marshal McBride,
ender 823) and confirm the reply carries 823; quest 29083 inverts the pair as
a second confirmation.
![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: 8a702f9990
ℹ️ 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".
Comment on lines
+622
to
+624
| for (uint16 i = MopUpdateObject::SelfQuestLogSourceStart; | ||
| i < MopUpdateObject::SelfQuestLogSourceStart + | ||
| MopUpdateObject::SelfQuestLogFieldCount; ++i) |
There was a problem hiding this comment.
When a player logs in with active quests, this changed-value loop never sends their quest IDs: Player::AddToWorld() clears m_changedValues, and Map::SendInitSelf() seeds visible-item, inventory, and skill fields but not SelfQuestLogSourceStart..SelfQuestLogFieldCount; SendInitialQuestSetup() only sends completed-quest bits. Consequently, persisted active quests remain absent from the client quest log after every relog until their IDs happen to change, so the login self-values packet must also include the stored quest-log fields.
Useful? React with 👍 / 👎.
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.
Integrates the MoP 5.4.8.18414 opcode campaign branch into
master.What is in this
22 packet-conversion waves landed previously on this branch, converting inherited pre-18414 packet bodies to the wire grammar proved against the 32-bit
Wow.exe18414 client — combat feedback, cooldowns, duels, threat, runes, channel updates, dismount, combo points and others. Each is recorded per wave in the external campaign ledger with its binary evidence.This session added:
a61d113e7f8a77ea41SMSG_PRE_RESURRECT 0x19C018414 encoding306dba043CMSG_QUEST_NPC_QUERY 0x1DAEhandler +0x036Dresponse7ffa8c380,170559ff78509800b5Plus merges bringing in the
WITH_TESTSgate, realmd as a submodule, the dep/Eluna/SD3 advance, and the Map spawn-difficulty fix.Notable fixes
Quest log never reached the client.
MopUpdateObjectprojected only the inventory, skill and buyback self ranges, so an accepted quest was never sent. This range cannot be shifted by a constant like the others: Four stores fifty five-word slots at 166..415 while 18414 uses fifty fifteen-word slots at 171..920, so it re-strides per slot. A flat copy would land legacy slot 1 inside client slot 0. Confirmed live:GetNumQuestLogEntries()correct,CMSG_QUESTLOG_REMOVE_QUEST slot=0agreeing with the projection, quest running through to COMPLETE.Eluna regression. Advancing the submodule to upstream
mastersilently dropped a Four-specific fix, leavingSendAuctionMenuemitting the pre-18414 body under the 18414 opcode — malformed to a real client. Fixed upstream in MangosServer/Eluna#9 and re-pointed here.CMSG_QUEST_NPC_QUERYwas never answered. The client sends it on every quest accept. Its response grammar was recovered from the client parser and then confirmed against real 18414 retail captures: every occurrence in the sniff corpus decodes with zero residual bytes, and the builder reproduces a retail reply byte-for-byte. NPC selection is enders-only, established from a retail chain-hand-off quest with disjoint giver/ender sets rather than assumed.Verification
ctest: 1083/1083 passing against the merged treemangosd: builds cleangit diff --check: cleanMerge note
Please merge with a merge commit, not squash. The campaign ledger, runtime-validation matrix and handoff documents cite the individual wave commits by hash as evidence anchors; squashing invalidates all of them. This is the same reason the branch was merged rather than rebased when picking up upstream.
Known limits, recorded rather than hidden
SMSG_PRE_RESURRECT: semantic terminal unresolved — the consumer sits behind a per-message Arxan guard trampoline resolved at runtime. The wave changes encoding, not semantics, and the inherited body could not have been read by an 18414 client at all.CMSG_QUEST_NPC_QUERY: gameobject questgivers untested; the retail corpus is entirely creature-actor, so they are deliberately not implemented.🤖 Generated with Claude Code
This change is