The Lua refactor branch was based before the cleanup commit and
brought back allMacroCommands, getMacroShowtooltipArg (game_screen),
lfgJoinResultString, lfgTeleportDeniedString (game_handler).
685 lines of unused code duplicated into extracted handler files
(entity_controller, spell_handler, quest_handler, warden_handler,
social_handler, action_bar_panel, chat_panel, window_manager)
during PRs #33-#38. Build is now warning-free.
ChromieCraft/AzerothCore tolerates no HASH_RESULT response (continues
session without Warden checks), but immediately kicks on a WRONG hash.
The previous commit sent a fallback SHA1 which the server rejected,
breaking login that was working before.
Restore the skip behavior for WotLK/TBC: stay silent on HASH_REQUEST
when no CR match exists, and advance to WAIT_CHECKS so the rest of the
session proceeds normally. Turtle/Classic servers still get the fallback
hash since they're lenient about wrong values.
Previously, WotLK/TBC servers with no CR match would skip the
HASH_REQUEST response entirely to "avoid account bans". This caused
a guaranteed kick-on-timeout for ALL WotLK servers including
permissive ones like ChromieCraft/AzerothCore.
Now sends a best-effort fallback hash (SHA1 of module image or raw
data) for all server types. Permissive servers accept this and
continue the session normally. Strict servers (Warmane) will reject
it but only kick — same outcome as the previous skip behavior, just
faster feedback.
For strict servers, the correct fix remains providing a .cr file
with pre-computed seed→reply entries for each module.
Suppress 9 unused parameter warnings in IObjectTypeHandler interface
methods and their overrides by commenting out parameter names:
- Base class: onCreate/onValuesUpdate/onMovementUpdate default empty
implementations (parameters intentionally unused in base)
- ItemTypeHandler::onCreate: entity param forwarded only to onCreateItem
which doesn't need it
- CorpseTypeHandler::onCreate: entity param not needed for corpse spawn
Build now produces zero warnings (excluding third-party stb headers).
Replace the incorrectly extracted RSA-2048 modulus (which contained
the exponent bytes embedded inside it) with the verified Blizzard
public key used across all pre-Cataclysm clients (1.12.1, 2.4.3,
3.3.5a).
Key confirmed against two independent sources:
- namreeb/WardenSigning ClientKey.hpp (72 verified sniffed modules)
- SkullSecurity wiki Warden_Modules documentation
The modulus starts with 0x6BCE F52D... and ends with ...03F4 AFC7.
Exponent remains 65537 (0x010001).
Verification algorithm: SHA1(module_data + "MAIEV.MOD"), 0xBB-padded
to 256 bytes, RSA verify-recover with raw (no-padding) mode.
Signature failures are non-fatal (log warning, continue loading) so
private-server modules signed with custom keys still work. This is
necessary because servers like ChromieCraft/AzerothCore may use their
own signing keys.
Also update warden_module.hpp status: all implementation items now ✅.
Complete the last major Warden stub — the import table parser that
resolves Windows API calls in loaded modules. This is the critical
missing piece for strict servers like Warmane.
Implementation:
- Parse Warden module import table from decompressed data (after
relocation entries): alternating libraryName\0 / functionName\0
pairs, terminated by null library name
- For each import, look up the emulator's pre-registered stub address
(VirtualAlloc, GetTickCount, ReadProcessMemory, etc.)
- Auto-stub unrecognized APIs with a no-op returning 0 — prevents
module crashes on unimplemented Windows functions
- Patch each IAT slot (sequential dwords at module image base) with
the resolved stub address
- Add WardenEmulator::getAPIAddress() public accessor for IAT lookups
- Fix initialization order: bindAPIs() now runs inside initializeModule()
after emulator setup but before entry point call
The full Warden pipeline is now: RC4 decrypt → RSA verify → zlib
decompress → parse executable → relocate → create emulator → register
API hooks → bind imports (IAT patch) → call entry point → extract
exported functions (packetHandler, tick, generateRC4Keys, unload).
Implement the three stubbed Warden module callbacks that were previously
TODO placeholders:
- **sendPacket**: Encrypts module output via WardenCrypto RC4 and sends
as CMSG_WARDEN_DATA through the game socket. Enables modules to send
responses back to the server (required for strict servers like Warmane).
- **validateModule**: Compares the module's provided 16-byte MD5 hash
against the hash received during download. Logs error on mismatch
(indicates corrupted module transit).
- **generateRC4**: Derives new encrypt/decrypt RC4 keys from a 16-byte
seed using SHA1Randx, then replaces the active WardenCrypto key state.
Handles mid-session re-keying requested by the module.
Architecture:
- Add setCallbackDependencies() to inject WardenCrypto* and socket send
function into WardenModule before load() is called
- Use thread_local WardenModule* so C function pointer callbacks (which
can't capture state) can reach the module's dependencies during init
- Wire dependencies from WardenHandler before module load
Also update warden_module.hpp status markers — RSA verification, zlib,
executable parsing, relocation, and Unicorn emulation are all implemented
(were incorrectly marked as TODO). Only API binding/IAT patching and
RSA modulus verification against real WoW.exe remain as gaps.
- packet_parsers_tbc: explain spline waypoint cap (DoS prevention),
spline compression flags (Catmull-Rom 0x80000 / linear 0x2000 use
uncompressed format, others use packed delta), spell hit target cap
(128 >> real AOE max of ~20), guild roster cap (1000 safety limit)
- ambient_sound_manager: explain 1.5s bell toll spacing — matches
retail WoW cadence, allows each toll to ring out before the next
- character_preview.hpp: explain 4:5 portrait aspect ratio for
full-body character display in creation/selection screen
- entity_controller: clamp block/dodge/parry/crit/rangedCrit percentage
fields to [0..100] after memcpy from update fields — guards against
NaN/Inf from corrupted packets reaching the UI renderer
- entity_controller: add why-comment on OBJECT_FIELD_SCALE_X raw==0
check — IEEE 754 0.0f is all-zero bits, so raw==0 means the field
was never populated; keeping default 1.0f prevents invisible entities
- Add bounds checks to readLE32/readLE16 — malformed Warden modules
could cause out-of-bounds reads on untrusted PE data
- Fix unsigned underflow in PE section loading: if rawDataOffset or
virtualAddr exceeds buffer size, the subtraction wrapped to a huge
uint32_t causing memcpy to read/write far beyond bounds. Now skips
the section entirely and uses std::min with pre-validated maxima
- world_packets: name kGuidTypeMask/kGuidTypePet/kGuidTypeVehicle
for chat receiver GUID type detection, with why-comment explaining
WoW's bits-48-63 entity type encoding and 0xF0FF mask purpose
- lua_engine: name kRoleTank/kRoleHealer/kRoleDamager (0x02/0x04/0x08)
for WotLK LFG role bitmask, add context on Leader bit (0x01) and
source packets (SMSG_GROUP_LIST / SMSG_LFG_ROLE_CHECK_UPDATE)
- srp: name kEphemeralBytes (19 = 152 bits, matches Blizzard client)
and kMaxEphemeralAttempts (100) with why-comment explaining A != 0
mod N requirement and near-zero failure probability
- warden_module: add why-comment on 0x400000 module base (default
PE image base for 32-bit Windows executables)
- warden_module: name kRsaSignatureSize (256 = RSA-2048) with
why-comment explaining signature stripping (placeholder modulus
can't verify Blizzard's signatures)
- vk_context: name FNV-1a hash constants (kFnv1aOffsetBasis/kFnv1aPrime)
with why-comment on algorithm choice for sampler cache
- transport_manager: collapse redundant if/else that both set
looping=false into single unconditional assignment, add why-comment
explaining the time-closed path design
- transport_manager: hoist duplicate kMinFallbackZOffset constants out
of separate if-blocks, add why-comment on icebreaker Z clamping
- entity: expand velocity smoothing comment — explain 65/35 EMA ratio
and its tradeoff (jitter suppression vs direction change lag)
- Add Inventory::FIRST_BAG_EQUIP_SLOT = 19 constant with why-comment
explaining WoW equip slot layout (bags occupy slots 19-22)
- Replace all 19 occurrences of magic number 19 in bag slot calculations
across inventory_handler, spell_handler, inventory, and game_handler
- Add UNIT_FIELD_FLAGS / UNIT_FLAG_PVP comment in combat_handler
- Add why-comment on network packet budget constants (prevent server
data bursts from starving the render loop)
- renderer: remove no-op assignment (mountAnims_.stand = 0 when already 0)
- renderer: add why-comments on blacksmith WMO ID 96048 (ambient forge
sounds) with TODO for other smithy buildings
- terrain_renderer: replace 1e30f sentinel with numeric_limits::max(),
name terrain view distance constant (1200 units ≈ 9 ADT tiles)
- social_handler: add missing LFG case 15, document case 0 nullptr
return (success = no error message), add enum name comments
- character_renderer: extract duplicated fallback texture creation
(white/transparent/flat-normal) into createFallbackTextures() — was
copy-pasted between initialize() and clear()
- wmo_renderer: replace magic 8192 with kMaxRetryTracked constant,
add why-comment explaining the fallback-retry set cap (Dalaran has
2000+ unique WMO groups)
- quest_handler: add why-comment on reqCount=0 fallback — escort/event
quests can report kill credit without objective counts in query response
- Replace all 11 occurrences of magic number 23 in backpack slot
calculations with Inventory::NUM_EQUIP_SLOTS across inventory_handler,
spell_handler, and inventory.cpp
- Add why-comment to NUM_EQUIP_SLOTS explaining WoW slot layout
(equipment 0-22, backpack starts at 23 in bag 0xFF)
- Add why-comment on 0x80000000 bit mask in item query response
(high bit flags negative/missing entry response)
- Replace manual channel membership loops with std::find in
chat_handler.cpp (YOU_JOINED and PLAYER_ALREADY_MEMBER cases)
- Add why-comment on PLAYER_ALREADY_MEMBER reconnect edge case
- spell_handler: extract duplicated item on-use spell lookup into
findOnUseSpellId() — was copy-pasted in useItemBySlot and useItemInBag
- warden_handler: add why-comment explaining the door model HMAC-SHA1
hash table (wall-hack detection for unmodified 3.3.5a client data)
- spell_handler.cpp: replace goto-done with do/while(false) for pet
spell packet parsing — bail on truncated data while always firing
events afterward
- water_renderer.cpp: replace goto-found_neighbor with immediately
invoked lambda to break out of nested neighbor search loops
Introduce `GameServices` struct — an explicit dependency bundle that
`Application` populates and passes to `GameHandler` at construction time.
Eliminates all 47 hidden `Application::getInstance()` calls in
`src/game/*.cpp`, completing SOLID-D (dependency-inversion) cleanup.
Changes:
- New `include/game/game_services.hpp` — `struct GameServices` carrying
pointers to `Renderer`, `AssetManager`, `ExpansionRegistry`, and two
taxi-mount display IDs
- `GameHandler(GameServices&)` replaces default constructor; exposes
`services() const` accessor for domain handlers
- `Application` holds `game::GameServices gameServices_`; populates it
after all subsystems are created, then constructs `GameHandler`
(fixes latent init-order bug: `GameHandler` was previously created
before `AssetManager` / `ExpansionRegistry`)
- `game_handler.cpp`: duplicate `isActiveExpansion` / `isClassicLikeExpansion` /
`isPreWotlk` anonymous-namespace helpers removed; `game_utils.hpp`
included instead
- All domain handlers (`InventoryHandler`, `SpellHandler`, `MovementHandler`,
`CombatHandler`, `QuestHandler`, `SocialHandler`, `WardenHandler`) replace
`Application::getInstance().getXxx()` with `owner_.services().xxx`
Adds [GO-DIAG] WARNING-level logs at:
- Right-click dispatch (raypick hit / re-interact with target)
- interactWithGameObject entry + all BLOCKED paths
- SMSG_SPELL_GO (wasInTimedCast, lastGoGuid, pendingGoGuid state)
- SMSG_LOOT_RESPONSE (items, gold, guid)
- Raypick candidate GO positions (entity pos + hit center + radius)
These logs will pinpoint exactly where the interaction fails:
- No GO-DIAG lines = GOs not in entity manager / not visible
- Raypick GO pos=(0,0,0) = GO position not set from update block
- BLOCKED = guard condition preventing interaction
- SPELL_GO wasInTimedCast=false = timer race (already fixed)
The client-side cast timer expires ~50-200ms before the server sends
SMSG_SPELL_GO (float precision + frame timing). Previously the fallback
called resetCastState() which set casting_=false and currentCastSpellId_
=0. When SMSG_SPELL_GO arrived moments later, wasInTimedCast evaluated
to false (false && spellId==0), so the loot path (CMSG_LOOT via
lastInteractedGoGuid_) was never taken. Quest chests never opened.
Now the fallback skips resetCastState() for GO interaction casts, letting
the cast bar sit at 100% until SMSG_SPELL_GO arrives and handles cleanup
properly with wasInTimedCast=true.
When the client-side cast timer expired slightly before SMSG_SPELL_GO
arrived, the fallback at update():1367 called performGameObjectInteraction
Now which sent a DUPLICATE CMSG_GAMEOBJ_USE to the server (confusing its
GO state machine), then resetCastState() cleared lastInteractedGoGuid_.
When SMSG_SPELL_GO finally arrived, the guid was gone so CMSG_LOOT was
never sent — quest chests produced no loot window.
Fix: the fallback no longer re-sends USE (server drives the interaction
via SMSG_SPELL_GO). resetCastState() no longer clears
lastInteractedGoGuid_ so the SMSG_SPELL_GO handler can still send LOOT.
Two remaining GO interaction bugs:
1. pendingGameObjectInteractGuid_ was never cleared after SMSG_SPELL_GO
or SMSG_CAST_FAILED, leaving it stale. This suppressed CMSG_CANCEL_CAST
for ALL subsequent spell casts (not just GO casts), causing the server
to think the player was still casting when they weren't.
2. For chest-like GOs, CMSG_LOOT was sent simultaneously with
CMSG_GAMEOBJ_USE. If the server starts a timed cast ("Opening"),
the GO isn't lootable until the cast completes — the premature LOOT
gets an empty response or is dropped, potentially corrupting the
server's loot state. Now defers LOOT to handleSpellGo which sends it
after the cast completes (via lastInteractedGoGuid_).
pendingGameObjectInteractGuid_ was always cleared to 0 right before
the interaction, which defeated the cancel-protection guard in
cancelCast(). Any positional movement (WASD, jump) during a GO
interaction cast (e.g., "Opening" on a quest chest) sent
CMSG_CANCEL_CAST to the server, aborting the interaction and
preventing quest objective credit.
Now sets pendingGameObjectInteractGuid_ to the GO guid so:
1. cancelCast() skips CMSG_CANCEL_CAST for GO-triggered casts
2. The cast-completion fallback can re-trigger loot after timer expires
3. isGameObjectInteractionCasting() returns true during GO casts
CMSG_GAMEOBJ_REPORT_USE was only sent for non-chest GOs. Chest-type
(type=3) and name-matched chest-like GOs (Bundle of Wood, etc.) went
through a separate path that sent CMSG_GAMEOBJ_USE + CMSG_LOOT but
skipped REPORT_USE. On AzerothCore, REPORT_USE triggers the server-side
HandleGameobjectReportUse which calls GossipHello on the GO script —
this is where many quest objective scripts grant credit.
Restructured so CMSG_GAMEOBJ_USE is sent first for all GO types,
then chest-like GOs additionally send CMSG_LOOT, and REPORT_USE fires
for everything except mailboxes.
The Packet::skipAll() method was introduced to replace the verbose
setReadPos(getSize()) pattern. 186 instances were migrated earlier,
but 20 survived in domain handler files created after the migration.
Also removes a redundant single-element for-loop wrapper around
SMSG_LOOT_CLEAR_MONEY registration.
Same class of bug as inventory_handler fix b9ecc26f. The for-loop over
{SMSG_INSTANCE_DIFFICULTY, MSG_SET_DUNGEON_DIFFICULTY} was missing its
closing brace, so GUILD_DECLINE, RAF_EXPIRED, RAF_FAILURE, and
PVP_AFK_RESULT registrations executed inside the loop body — each
registered twice (once per opcode). Currently harmless since duplicate
registration is idempotent, but structurally wrong.
Long achievement names combined with sender name could exceed 256
bytes, silently cutting the message mid-word in chat. Replaced with
std::string concatenation which grows dynamically.
The two fireAddonEvent calls were indented as if conditional on
repChangeCallback_ but actually execute unconditionally (no braces).
Fixed indentation and added clarifying comment.
zone_manager.cpp used std::rand() for music track selection with modulo
bias and global state. game_screen.cpp used std::rand() for rain/snow
particle positions. Both now use local std::mt19937 seeded from
random_device. Also removes the global srand(time(nullptr)) call since
no code depends on the C rand() seed anymore.
No std::rand() or srand() calls remain in the codebase.
All domain handler files used 'packet.getSize() - packet.getReadPos()'
which underflows to ~2^64 when readPos exceeds size (documented in
commit ed63b029). The game_handler.cpp and packet_parsers were migrated
to hasRemaining(N) in an earlier cleanup, but the domain handlers were
created after that migration by the PR #23 split, copying the old
unsafe patterns back in. Now uses hasRemaining(N) for comparisons and
getRemainingSize() for assignments across all 7 handler files.
elapsedTime_ was float (32-bit, ~7 significant digits). At 16384
seconds the float can only represent integers, so elapsedTime_*1000
jumps in 1-second steps — ships and elevators visibly jerk. Changed to
double (53-bit mantissa) which maintains sub-millisecond precision for
~285 million years. Also changed lastServerUpdate to double to match.
std::toupper(int) and std::tolower(int) have undefined behavior when
passed a negative value. These sites passed raw signed char without
casting to unsigned char first, unlike the rest of the codebase which
already uses the correct pattern. Affects auth (account names), world
packets, and mount sound path matching.
WotLK format is min(4)+max(4)+result(4)+guid(8)=20 bytes. The parser
read guid(8) first (treating min|max as a uint64), then targetGuid(8)
(non-existent field), then the actual values at wrong offsets. Every
/roll message showed garbled numbers and a bogus roller identity.
Also adds a hasRemaining guard for the 64 bytes of damage/armor/resist
fields in the item query parser — previously read past end with silent
zero-fill on truncated packets.
The two emit calls were indented 12 spaces (suggesting a nested block)
instead of 8 (matching the enclosing if). Same class of maintenance
trap as the PLAYER_ALIVE/PLAYER_UNGHOST fix in b3abf04d.
1. Contradictory condition (!numStrings && numStrings >= 1) was always
false, so unknown guild event messages never included the server's
context string. Fixed to just numStrings >= 1.
2. leaveParty() only sent the packet without clearing partyData or
firing addon events, so /leave left party frames visible until the
server pushed an update. Now delegates to leaveGroup() which handles
both the packet and local state cleanup.
The cast falsely suggests reasonType is unused, but it's read on lines
3699-3702 for AFK/vote-kick differentiation. Same class of issue as
the (void)isPlayerTarget fix in commit 6731e584.
