Branch: feature/wotlk-classic-v3.4.3
Commit: 884517f496f95466c9c87a0a4d6b504eff605add
fix(build): stop clobbering TRACE on Windows, disarm remaining aborts (#116)
HermesProxy.csproj assigned DefineConstants instead of appending to it:
<PropertyGroup Condition="$([MSBuild]::IsOSPlatform('Windows'))">
<DefineConstants>_WINDOWS</DefineConstants>
</PropertyGroup>
That discarded the SDK-provided TRACE define. Measured with
dotnet msbuild -getProperty:DefineConstants, Windows resolved to
_WINDOWS;DEBUG / _WINDOWS;RELEASE while macOS got TRACE;DEBUG /
TRACE;RELEASE. Trace.Assert is [Conditional("TRACE")], so every assertion
in the codebase was a no-op on Windows and live, process-fatal, everywhere
else.
That divergence is what hid #111: the same commit against the same backend
sent an opcode-0 packet and played on under Windows, but aborted from a
socket callback on macOS. Every Windows contributor has been running with
assertions disabled for as long as the line has existed.
Append instead, and add a VerifyTraceDefine target that fails the build if
TRACE ever goes missing again — verified to fail a simulated clobber and to
pass a normal build.
Restoring TRACE arms assertions on Windows, so the asserts #111 left behind
are converted to non-fatal handling in the same commit. A network daemon
should not abort from a socket callback on untrusted input:
- Packet.cs: WorldPacket(uint, byte[]) — both callers are legacy receive
paths (SMSG_COMPRESSED_MOVES reads the opcode off the wire, Inflate()
copies it from the parent), so a zero opcode is corrupt input, not a
programming error. Log it and let dispatch drop it as unknown. - GameData.cs: a Battlegrounds.csv row with no map IDs is a data problem —
log the battleground id and skip the row instead of aborting at startup. - WorldSocket.cs: SendAccountDataTimes on a non-realm socket logs and
returns. - LegacyWorldCrypt.cs: an empty session key throws ArgumentException, which
the handler guards added in #111 can catch and log.
Stacked on #111 deliberately: fixing the build define first would have armed
assertions while Packet.cs and EmptyClientPacket still aborted, turning the
silent Windows behaviour into the exact crash that was reported.
709/709 tests pass with assertions armed on Windows.
Claude-Session: https://claude.ai/code/session_01MCnS9FPB2hzTZqmf14qbKB
Co-authored-by: Claude Opus 5 (1M context) noreply@anthropic.com