Reimplement multiplayer command ownership and document network flow

docs/wiki.rst

@@ -33,7 +33,7 @@ The Game itself
 - AI Behavior
 - Graphics and Sound Systems
 - Modding Support
-- Multiplayer Architecture
+- :doc:`Multiplayer command ownership and replay boundaries <wiki/multiplayer-command-flow>`
 
 (Coming Soon!)
 

docs/wiki/multiplayer-command-flow.md

@@ -0,0 +1,98 @@
+# Multiplayer command ownership and replay boundaries
+
+This note records original-code evidence from Crusader 1.41, SHA-256
+`3bb0a8c1e72331b3a30a5aa93ed94beca0081b476b04c1960e26d5b45387ac5a`.
+Addresses refer to that executable. Generated names help navigation; the original
+instructions establish behavior. No multiplayer match was run for this analysis.
+
+## From transport to simulation
+
+1. `queueCommand` (`0x489100`) builds a command in a 200-entry ring. Each entry is
+   `0x4F8` bytes, with tick at offset 0, sender handle at 4, category at 8, state
+   at 9 and parameters at 10. The handler participates in both serialization
+   and execution; invoking it does not necessarily mean the command executed.
+2. `transmitCommand` (`0x487C50`) serializes the low **three bytes** of the tick
+   after the category byte. The generated `Packet` labels divide this into
+   `short time` and `byte unknown`, but the native copy at `0x46A830` writes all
+   three bytes of the timestamp. Payloads above 200 bytes normally use envelope
+   category 125; category 65 has a separate exception. Most commands use
+   guaranteed DirectPlay delivery; categories 12 and 117 have special handling.
+3. `receiveAllTransmittedCommands` (`0x490690`) unwraps category 125 and zeroes a
+   32-bit tick before copying those three bytes into it. DirectPlay system
+   messages, synchronization packets and ordinary commands take different paths.
+   Host migration and player removal are processed here, outside timed dispatch.
+4. `scheduleReceivedCommand` (`0x480210`) invokes the handler in receive/parse
+   mode. Timed payloads go into the ring. Commands with signed time <= 0 copy to
+   the fixed parameter area and execute immediately at `0x480425`, then clear
+   their temporary ring entry. They never reach `processWaitingCommands`.
+5. `getCommandIDFromCommandSelectionStuff` (`0x480440`) selects due, unprocessed
+   entries, with a batch cap of 100. It translates sender handles and stably
+   orders the selected entries by logical player slot. A recorder must observe
+   actual dispatch order; transport arrival order is insufficient.
+6. `processWaitingCommands` (`0x4892F0`) translates each sender again, sets the
+   execution action/parameter state, calls the handler and marks the entry
+   processed. The translator is `0x47EAF0`, reimplemented by this contribution.
+
+The timestamp is thus 24-bit on the wire and 32-bit in the ring. The examined
+receive routine does not reconstruct a higher epoch. Long-duration recording
+and replay must preserve that distinction rather than assuming identical wire
+and internal formats. This observation does not establish a reproduced wrap bug.
+
+## System messages bypass command dispatch
+
+After a successful `IDirectPlay4A::Receive`, the receiver compares the sender
+with zero (`DPID_SYSMSG`). Its system-type switch begins at `0x490735`
+(Extreme `0x490895`). Failed receives and ordinary nonzero senders branch away
+before this point. Values below also agree with the DirectX SDK `dplay.h`:
+
+- `DPSYS_DESTROYPLAYERORGROUP` (`5`) reads the handle at message offset 8 and
+  calls the identity translator at `0x490755`. The next call is
+  `removePlayerFromLobby` at `0x49075B`, without reloading ECX. This is another
+  concrete native caller requiring this reimplementation to retain ECX.
+- `DPSYS_HOST` (`0x101`) sets `isHost`, resets the hash countdown, assigns a new
+  `timeGetTime()` value to the autosave timer and clears both nine-entry player
+  timing arrays. Chat and out-of-match lobby ordering also change. A host-only
+  transition can therefore change native scheduling state without a changed
+  player roster or any timed command.
+- `DPSYS_CREATEPLAYERORGROUP` (`3`) and `DPSYS_SESSIONLOST` (`0x31`) fall through
+  to the next receive iteration without special handling in this switch. This
+  describes the original routine; it does not imply that a transport or replay
+  implementation may disregard the broader connection lifecycle.
+
+Polling roster and synchronization fields at simulation boundaries cannot
+establish that no system event occurred between them. Recorder diagnostics can
+observe the type-switch entry before mutation, but replaying those events still
+requires their semantics and timing. DirectPlay system structures can contain
+process pointers; copying their bytes is not a portable replay format.
+
+## Why one save and a timed-command log are insufficient
+
+The original save section table does not cover active mode or the full network
+handle array. It does cover a saved mode copy, AI-slot values and local player
+slot. The single-player load-dialog path (`0x4950B0`) clears network handles and
+recreates the local entry. Multiplayer playback therefore needs explicit roster
+restoration and command ownership, not merely a changed mode flag or slot zero.
+
+`queueSynchronizedAutosaveProtocol` (`0x48C660`) uses host wall-clock elapsed time,
+then queues a save command carrying the simulation tick and a unit checksum.
+`checkGameSync` (`0x48CB00`) compares peer evidence and starts the resync state
+machine. `recomputeHashesAndSendResync` (`0x48CC90`) hashes selected game arrays;
+`sendPendingResyncCommandsInBudget` (`0x48E680`) sends mismatching sections in
+bounded batches, retaining its category/item position between calls. These
+include units, buildings, player data and tile-map regions. Resync replaces
+simulation data; treating every immediate command as presentation-only is wrong.
+
+A useful recorder must account separately for timed command execution, immediate
+state-changing commands, roster/host transitions and resync transfers. Reliable
+transport delivery alone cannot restore missing replay state or make
+frame/audio-driven RNG calls deterministic. An extension's ordinary save
+integration also does not demonstrate integration with native resync transfers.
+
+## Validation scope
+
+The accompanying player-identity checker compiles and executes the actual C++
+function against the originals in both variants. It covers last-match semantics,
+sentinels, missing handles, relocated receivers and the retained ECX value.
+Other routines in this note were inspected in the named Ghidra project and
+original assembly; they are not newly reimplemented or live-validated here.
+See [reproduction instructions](../../tools/reimplementation-tests/PLAYER-IDENTITY.md).

src/OpenSHC/Synchrony/GameSynchronyState/translateMultiplayerIDsIntoPlayerIDs.cpp

@@ -0,0 +1,36 @@
+#include "../GameSynchronyState.func.hpp"
+
+namespace OpenSHC {
+namespace Synchrony {
+
+    // FUNCTION: STRONGHOLDCRUSADER 0x0047EAF0
+    uint GameSynchronyState::translateMultiplayerIDsIntoPlayerIDs(int playerHandle)
+    {
+        uint player = 0;
+        if (this->currentGameMode == Game::GM_SOLITARY || this->currentGameMode == Game::GM_SKIRMISH_SINGLE_PLAYER) {
+            return this->currentPlayerSlotID;
+        }
+
+        // Do not return on the first match: the native function lets the last
+        // matching slot win, including duplicate or sentinel-valued handles.
+        if (this->currentPlayerFullIDArray[1] == playerHandle)
+            player = 1;
+        if (this->currentPlayerFullIDArray[2] == playerHandle)
+            player = 2;
+        if (this->currentPlayerFullIDArray[3] == playerHandle)
+            player = 3;
+        if (this->currentPlayerFullIDArray[4] == playerHandle)
+            player = 4;
+        if (this->currentPlayerFullIDArray[5] == playerHandle)
+            player = 5;
+        if (this->currentPlayerFullIDArray[6] == playerHandle)
+            player = 6;
+        if (this->currentPlayerFullIDArray[7] == playerHandle)
+            player = 7;
+        if (this->currentPlayerFullIDArray[8] == playerHandle)
+            player = 8;
+        return player;
+    }
+
+} // namespace Synchrony
+} // namespace OpenSHC

status/addresses-SHC-3BB0A8C1.txt

@@ -22385,7 +22385,7 @@ SHC_3BB0A8C1_0x0047E8F0 | 0.0% | Pending
 
 SHC_3BB0A8C1_0x0047EA40 | 0.0% | Pending
 
-SHC_3BB0A8C1_0x0047EAF0 | 0.0% | Pending
+SHC_3BB0A8C1_0x0047EAF0 | 85.29% | Reimplemented
 
 SHC_3BB0A8C1_0x0047EB80 | 0.0% | Pending
 

tools/reimplementation-tests/PLAYER-IDENTITY.md

@@ -0,0 +1,55 @@
+# Player-handle translation
+
+`GameSynchronyState::translateMultiplayerIDsIntoPlayerIDs` resolves a transport
+handle to the logical player that owns a command. It is called both during
+selection/sorting and immediately before native dispatch. Replaying the raw
+handle through ordinary single-player dispatch therefore executes as the local
+player, regardless of the recorded actor.
+
+The source preserves the original behavior:
+
+- Modes 0 and 99 return the receiver's local slot without examining the handle.
+- Other modes scan slots 1 through 8; slot 0 is ignored.
+- A missing handle returns zero. Duplicate handles return the highest matching
+  slot, including sentinel values. This routine itself does not validate a roster.
+- It does not mutate game state and preserves ECX, which native callers reuse.
+
+The addresses are `0x47EAF0` in Crusader and `0x47ECC0` in Extreme. Original
+instructions use ECX-relative state, not the fixed globals suggested by Ghidra's
+decompiler. Both variants use mode offset `0x618` and handles at `0x6A8`; the
+local-slot offsets are `0x109E74` and `0x166304` respectively. The OpenSHC source
+uses its existing Crusader structure layout; the checker populates each native
+variant's own layout for comparison.
+
+## Reproduce
+
+From a checkout with the project's toolchain/dependencies initialized:
+
+```powershell
+$taskRoot = (Get-Location).Path
+$taskMsvc = Join-Path $taskRoot 'MSVC1400-SP1'
+$env:PATH = "$taskMsvc/Common7/IDE;$taskMsvc/VC/bin;$env:PATH"
+$env:INCLUDE = "$taskMsvc/VC/include;$taskMsvc/VC/PlatformSDK/Include"
+New-Item -ItemType Directory -Force tmp | Out-Null
+& "$taskMsvc/VC/bin/cl.exe" /nologo /c /O2 /EHsc /DOPEN_SHC_DLL `
+  /I "$taskRoot/src" /I "$taskRoot/dependencies/ucp3/include" `
+  /I "$taskRoot/dependencies/lua/include" /I "$taskRoot/dependencies/DXSDK_Aug2007/include" `
+  /FI "$taskRoot/src/precomp/pch.h" /Fotmp/playerIdentity.obj `
+  src/OpenSHC/Synchrony/GameSynchronyState/translateMultiplayerIDsIntoPlayerIDs.cpp
+if ($LASTEXITCODE -ne 0) { throw 'Compilation failed' }
+python -m pip install pefile unicorn==2.1.4
+python tools/reimplementation-tests/player_identity.py tmp/playerIdentity.obj 'PATH/Stronghold Crusader.exe'
+python tools/reimplementation-tests/player_identity.py tmp/playerIdentity.obj 'PATH/Stronghold_Crusader_Extreme.exe'
+```
+
+The checker requires the known original executable SHA-256, compares the actual
+compiled source and native routine against an independent last-match model, and
+checks receiver relocation, missing/duplicate/sentinel handles, all slots,
+single-player/end-of-game modes, memory read/write bounds and stack/register
+behavior. Use Python 3.10+ without `-O`.
+
+This is readable C++03 with no generated-header or resolver changes. MSVC emits
+137 bytes, the same size as the original, but places the single-player return
+block differently. Exact-byte matching, a linked-DLL reccmp score and live
+multiplayer behavior are not claimed. The contribution supplies a verified
+native identity primitive; it does not enable multiplayer replay.

tools/reimplementation-tests/player_identity.py

@@ -0,0 +1,107 @@
+"""Compare the compiled player-handle translator to both original game variants.
+
+Requires pefile and unicorn. Reads files only; never launches the game.
+"""
+import argparse
+import hashlib
+from pathlib import Path
+import random
+import struct
+
+import pefile
+from unicorn import Uc, UC_ARCH_X86, UC_MODE_32, UC_HOOK_MEM_READ, UC_HOOK_MEM_WRITE
+from unicorn.x86_const import (UC_X86_REG_EAX, UC_X86_REG_EBX, UC_X86_REG_ECX,
+    UC_X86_REG_ESI, UC_X86_REG_EDI, UC_X86_REG_EBP, UC_X86_REG_ESP, UC_X86_REG_EIP)
+
+SYMBOL = b'?translateMultiplayerIDsIntoPlayerIDs@GameSynchronyState@Synchrony@OpenSHC@@QAEIH@Z'
+VARIANTS = {
+    '3bb0a8c1e72331b3a30a5aa93ed94beca0081b476b04c1960e26d5b45387ac5a': (0x47eaf0, 0x109e74),
+    '55648e6b05d67d37a5773fe699bbb17a2d6ad4de1bb9dbded9a21caef82bd7fb': (0x47ecc0, 0x166304),
+}
+
+
+def object_code(path):
+    data = path.read_bytes()
+    machine, sections, _, symbols, count, optional, _ = struct.unpack_from('<HHIIIHH', data)
+    assert machine == 0x14c and optional == 0
+    strings = symbols + count * 18
+    index = 0
+    while index < count:
+        name, value, section, kind, _, auxiliary = struct.unpack_from('<8sIhHBB', data, symbols + index * 18)
+        if name[:4] == b'\0' * 4:
+            offset = strings + struct.unpack_from('<I', name, 4)[0]
+            name = data[offset:data.index(b'\0', offset)]
+        else:
+            name = name.rstrip(b'\0')
+        if name == SYMBOL:
+            assert kind == 0x20 and 1 <= section <= sections
+            header = 20 + (section - 1) * 40
+            size, offset = struct.unpack_from('<II', data, header + 16)
+            assert struct.unpack_from('<H', data, header + 32)[0] == 0, 'Unexpected relocations'
+            return data[offset + value:offset + size]
+        index += 1 + auxiliary
+    raise ValueError('Player identity function is missing')
+
+
+def execute(code, mode, local_player, handles, sender, local_offset, relocated):
+    cpu = Uc(UC_ARCH_X86, UC_MODE_32)
+    start, stack, stop = 0x100000, 0x200000, 0x300000
+    state = 0x800000 if relocated else 0x400000
+    cpu.mem_map(start, 0x1000); cpu.mem_write(start, code)
+    cpu.mem_map(stack, 0x10000); cpu.mem_map(state, 0x200000)
+    def put(address, value): cpu.mem_write(address, struct.pack('<I', value & 0xffffffff))
+    put(state + 0x618, mode); put(state + local_offset, local_player)
+    for index, handle in enumerate(handles): put(state + 0x6a8 + index * 4, handle)
+    sp = stack + 0x8000
+    put(sp, stop); put(sp + 4, sender)
+    saved = {register: 0x12340000 + register for register in
+             (UC_X86_REG_EBX, UC_X86_REG_ESI, UC_X86_REG_EDI, UC_X86_REG_EBP)}
+    for register, value in saved.items(): cpu.reg_write(register, value)
+    cpu.reg_write(UC_X86_REG_ESP, sp); cpu.reg_write(UC_X86_REG_ECX, state)
+    reads = []
+    def read(uc, access, location, size, value, context):
+        if stack <= location and location + size <= stack + 0x10000: return
+        assert (location == state + 0x618 or location == state + local_offset or
+                state + 0x6ac <= location <= state + 0x6c8) and size == 4, 'Unexpected state read'
+        reads.append(location - state)
+    def write(uc, access, location, size, value, context):
+        assert stack <= location and location + size <= stack + 0x10000, 'Unexpected state write'
+    cpu.hook_add(UC_HOOK_MEM_READ, read); cpu.hook_add(UC_HOOK_MEM_WRITE, write)
+    cpu.emu_start(start, stop, count=1000)
+    assert cpu.reg_read(UC_X86_REG_EIP) == stop and cpu.reg_read(UC_X86_REG_ESP) == sp + 8
+    assert cpu.reg_read(UC_X86_REG_ECX) == state, 'Native callers retain this in ECX'
+    for register, value in saved.items(): assert cpu.reg_read(register) == value
+    if mode in (0, 99): assert reads == [0x618, local_offset], 'Single-player must not inspect handles'
+    return cpu.reg_read(UC_X86_REG_EAX)
+
+
+def main():
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument('object', type=Path); parser.add_argument('executable', type=Path)
+    args = parser.parse_args()
+    data = args.executable.read_bytes(); digest = hashlib.sha256(data).hexdigest()
+    assert digest in VARIANTS, 'Unsupported original executable'
+    address, local_offset = VARIANTS[digest]
+    pe = pefile.PE(data=data, fast_load=True)
+    native = pe.get_data(address - pe.OPTIONAL_HEADER.ImageBase, 0x89)
+    compiled = object_code(args.object)
+    assert native[:8] == bytes.fromhex('8b 91 18 06 00 00 33 c0') and native[-3:] == b'\xc2\x04\0'
+    rng = random.Random(0x47eaf0)
+    rosters = [list(range(9)), [-1] * 9, [0] * 9, [123, 7, 8, 7, 9, 10, 7, 11, 7]]
+    rosters += [[rng.randrange(-3, 10) for _ in range(9)] for _ in range(50)]
+    cases = 0
+    for mode in (0, 99, 1, 2, 666, -1):
+        for handles in rosters:
+            for sender in sorted(set(handles + [0x7fffffff, -2147483648, 0, -1])):
+                local_player = rng.randrange(1, 9)
+                matches = [i for i in range(1, 9) if handles[i] == sender]
+                expected = local_player if mode in (0, 99) else max(matches, default=0)
+                for code, offset in ((native, local_offset), (compiled, 0x109e74)):
+                    assert execute(code, mode, local_player, handles, sender, offset, cases % 2) == expected
+                cases += 1
+    print(f'PASS: {cases} native/C++ identity comparisons; input bounds, read-only state, ECX and thiscall ABI')
+    print(f'Native/compiled lengths: {len(native)}/{len(compiled)} bytes; exact bytes: {native == compiled}')
+    print(f'Executable SHA256: {digest}')
+
+
+if __name__ == '__main__': main()