Skip to content

Type the game's 0x20 slot as D2ClientInfoStrc* - #237

Open
devtejasx wants to merge 1 commit into
ThePhrozenKeep:masterfrom
devtejasx:fix/create-client-info-callback-160
Open

devtejasx wants to merge 1 commit into
ThePhrozenKeep:masterfrom
devtejasx:fix/create-client-info-callback-160

Conversation

@devtejasx

Copy link
Copy Markdown
Contributor

Refs #160.

D2GameStrc 0x20 was a bare uint32_t, filled by a callback returning uint32_t and passed to one taking uint32_t*. The rest of the callback table says what lives there:

  • pfLeaveGame, pfGetDatabaseCharacter, pfSaveDatabaseCharacter and pfRelockDatabaseCharacter all take D2ClientInfoStrc** ppClientInfo, and their call sites pass &pClient->pClientInfo.
  • pfUnlockDatabaseCharacter was the odd one out (uint32_t* pGameData). GAME_JoinGame passes it &pGame->nGameData in the branch where CLIENTS_AddToGame failed, so there is no client to take a pClientInfo from.

So 0x20 is now D2ClientInfoStrc* pClientInfo, FnUnlockDatabaseCharacter takes D2ClientInfoStrc** like its four siblings, and FnSetGameData becomes FnCreateClientInfo returning D2ClientInfoStrc*. Layout is unchanged (4-byte pointer on the 32-bit target).

Not done: the issue also gives the callback a D2ClientStrc* pClient parameter. Its only caller, CLIENTS_SetGameData, is reached from GAME_AllocGame / GAME_FreeGame with no client in scope and passes nothing today, so I left the signature argument-less. If you know the original takes one in ECX, I'll add it.

`D2GameStrc::nGameData` at 0x20 was a bare `uint32_t`, filled by a callback
declared to return `uint32_t` and read by a callback declared to take
`uint32_t*`. The rest of the table makes clear what actually lives there:

- `pfLeaveGame`, `pfGetDatabaseCharacter`, `pfSaveDatabaseCharacter` and
  `pfRelockDatabaseCharacter` all take `D2ClientInfoStrc** ppClientInfo`, and
  the call sites pass `&pClient->pClientInfo` or a local `D2ClientInfoStrc*`.
- `pfUnlockDatabaseCharacter` is the odd one out, typed `uint32_t* pGameData`,
  and `GAME_JoinGame` passes it `&pGame->nGameData` — the *game's* slot, in the
  branch where `CLIENTS_AddToGame` failed and there is no client to take a
  `pClientInfo` from.

So the same kind of handle is reached through two different types depending on
whether a client exists. Field 0x20 is now `D2ClientInfoStrc* pClientInfo`,
`FnUnlockDatabaseCharacter` takes `D2ClientInfoStrc**` like its four siblings,
and `FnSetGameData` becomes `FnCreateClientInfo` returning `D2ClientInfoStrc*`,
which is what `CLIENTS_SetGameData` stores into the slot.

The layout is unchanged: `D2GameStrc` is packed for a 32-bit target, where the
pointer occupies the same four bytes the `uint32_t` did.

ThePhrozenKeep#160 also gives the callback a `D2ClientStrc* pClient` parameter. That half is
left alone: the only call site, `CLIENTS_SetGameData`, is reached from
`GAME_AllocGame` and `GAME_FreeGame` with no client in scope, and passes no
argument today. Deciding whether the original takes one in ECX needs the
disassembly, not this tree.

Syntax-checked both touched translation units with g++ -fsyntax-only -m32 and
the DLL_DECL macros defined. Error counts are identical before and after
(Game.cpp 14, Clients.cpp 4) — all pre-existing MSVC-isms under GCC, including
the struct-size static_asserts, which fail on the unmodified tree too.

Fixes ThePhrozenKeep#160
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant