Fix multiplayer player data mix between different players bug#536
Fix multiplayer player data mix between different players bug#536codeHusky merged 5 commits intosmartcmd:mainfrom
Conversation
Fixes a Win64 multiplayer issue where player data (`players/*.dat`) could be mismatched because identity was effectively tied to connection-order `smallId` XUIDs. Introduces a deterministic username-derived persistent XUID and integrates it into the existing XUID-based save pipeline. - Added `Windows64_NameXuid` for deterministic `name -> persistent xuid` resolution - On Win64 login (`PlayerList`), set `ServerPlayer::xuid` from username-based resolver - Aligned local player `xuid` assignment (`Minecraft`) for create/init/respawn paths to use the same resolver - Added Win64 local-self guard in `ClientConnection::handleAddPlayer` using name match to avoid duplicate local remote-player creation - Kept `IQNet::GetPlayerByXuid` compatibility fallback behavior, while extending lookup to also resolve username-based XUIDs - Moved implementation to `Minecraft.Client/Windows64/Windows64_NameXuid.h`; kept legacy `Win64NameXuid.h` as compatibility include Rename migration is intentionally out of scope (same-name identity only).
|
@kuwacom In my opinion it makes more sense for the game to simply generate a random Xuid value upon first boot and store it alongside where the username is configured. That way we mirror Java-like username-uuid decoupling and prevent issues where people change their username (very likely to happen) and just lose progress for no reason. It also makes their Xuid less easy to guess, though eventually (if we support stuff like Steam networking) we will just use platform ID values |
|
That's right. Considering the current situation, it might be better to generate the Xuid the first time and set it in a file. |
|
If you do that I'll test & merge this in |
|
Trying to fix this up in feat/multiplayer-fix |
|
This breaks loading existing saves data unfortunately. May have to just patch the save/load singleplayer to allow the hardcoded xuid as a fallback |
|
I got some sleep and went out.
That's so true. I’ll try modifying it so that only the first player in singleplayer uses the previous XUID as a hardcoded value (like in Bedrock Edition) |
…patibility - Add legacy embedded host XUID helper (base + 0). - When Minecraft.Client is hosting, force only the first host player to use legacy host XUID. - Keep name-based XUID for non-host players. - Prevent old singleplayer/hosted worlds from losing/mismatching host player data.
|
I implemented compatibility first. |
|
If that works and you can decouple usernames from UIDs that'd be about perfect. We'll have a long term solution in 4JLibs but loading the uid from something like a uid.dat would be ideal. If we have that then we have at least the foundation of working multiplayer |
|
I have some preliminary stuff I did in https://github.com/smartcmd/MinecraftConsoles/tree/feat/multiplayer-fix that probably doesn't work, but it's at least more or less what I was intending to do if that's any help |
…D based duplicate login guards - Replace Win64 username-derived XUID resolution with persistent `uid.dat`-backed identity (`Windows64_Xuid` / `Win64Xuid`). - Persist a per-client XUID next to the executable, with first-run generation, read/write, and process-local caching. - Keep legacy host compatibility by pinning host self to legacy embedded `base + 0` XUID for existing world/playerdata continuity. - Propagate packet-authoritative XUIDs into QNet player slots via `m_resolvedXuid`, and use it for `GetXuid`/`GetPlayerByXuid` with legacy fallback. - Update Win64 profile/network paths to use persistent XUID for non-host clients and clear resolved identity on disconnect. - Add login-time duplicate checks: reject connections when the same XUID is already connected (in addition to existing duplicate-name checks on Win64). - Add inline compatibility comments around legacy/new identity coexistence paths for easier future maintenance.
|
OK, everything is working smoothly.
|
|
If this implementation is going to be merged, I’d like to merge it into the dedicated server implementation as well. |
|
I'm not seeing uid.dat generate in the game directory on my end, testing with release builds. Do I need to connect to a server to generate it? |
|
Yep it's on connect - it'd be good to make the uid.dat generate on game launch and not just on connection to the world. If you change that up it's good to merge, and yes I do intend to merge this in |
|
separate issues but still notable (prob split into an issue after this pr)
occurs with dedicated server |
|
confirmed player data saves when doing LAN play so this is mergeable as-is when your good with it @kuwacom |
Is this an issue with the separated dedicated server version that my pull request? |
|
no, its an issue with the current |
|
So far, I haven’t observed this issue on my server. I remember that the world save/load logic was quite tricky on the client-side implementation (because of that, I ended up partially separating the world management and changing the save path). |
The merge I mentioned here refers to the isolation dedicated server in my pull request, not the I haven’t looked closely at the |
|
Where does the isolated dedicated server build to? I only see Minecraft.Client.exe in the builds I'm making in VS |
I should have linked it directly from the start. |
|
Could you merge the changes from #498 into this then? Would make it easier for me to test and it's a similarly important feature |
At startup, I added a check for |
|
Am I good to merge this as-is then? |
|
ok! |
Description
This PR fixes a Win64 multiplayer identity bug where player save data (
players/*.dat) could be loaded for the wrong player when join order changed.On Win64, player identity was effectively tied to connection-order
smallId-based XUIDs.This PR introduces a deterministic username-derived persistent XUID and wires it into the existing XUID-based save/load flow.
fixed.mp4
Changes
Previous Behavior
inventory/progress/map ownership could appear swapped between players.
Root Cause
Player::xuid.IQNetPlayer::GetXuid) was tied tosmallId, not a stable per-user identifier.New Behavior
Windows64_NameXuidfor deterministicname -> persistent xuidresolution.PlayerList),ServerPlayer::xuidis now resolved from username.xuidassignment (Minecraft) is aligned to the same resolver in create/init/respawn paths.ClientConnection::handleAddPlayer(name check) to prevent duplicate local remote-player creation.IQNet::GetPlayerByXuidkeeps compatibility fallback behavior, but now also resolves username-based XUIDs.Minecraft.Client/Windows64/Windows64_NameXuid.h; legacyWin64NameXuid.hremains as a compatibility include.Rename migration is intentionally out of scope (same-name identity only).
Related Issues