Ghost/shadow player fix

When player 2 joined, player 1's connection received an AddPlayerPacket and created a RemotePlayer shadow in the world. The existing XUID deduplication check never fired on Windows64 (XUIDs are always INVALID_XUID). Added a Windows64-specific check that matches by IQNet-derived XUID (0xe000d45248242f2e + smallId), which is set up before the connection loop, making it race-free.
This commit is contained in:
Michal Schiller 2026-03-06 18:25:26 +01:00 committed by GitHub
parent e513d6b8c5
commit a2a0837fbd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -627,12 +627,15 @@ void ClientConnection::handleAddEntity(shared_ptr<AddEntityPacket> packet)
} }
vector<shared_ptr<Entity> > *subEntities = e->getSubEntities(); vector<shared_ptr<Entity> > *subEntities = e->getSubEntities();
if (subEntities) if (subEntities != NULL)
{ {
int offs = packet->id - e->entityId; int offs = packet->id - e->entityId;
for ( auto it : *subEntities ) //for (int i = 0; i < subEntities.length; i++)
{ for(AUTO_VAR(it, subEntities->begin()); it != subEntities->end(); ++it)
it->entityId += offs; {
(*it)->entityId += offs;
//subEntities[i].entityId += offs;
//System.out.println(subEntities[i].entityId);
} }
} }
@ -760,32 +763,28 @@ void ClientConnection::handleAddPlayer(shared_ptr<AddPlayerPacket> packet)
} }
} }
#ifdef _WINDOWS64 #ifdef _WINDOWS64
// Win64 keeps local-player identity separate from network smallId; also guard against creating // On Windows64 ProfileManager.GetXUID always returns INVALID_XUID so the check above
// a duplicate remote player for a local slot by checking the username directly. // never fires. Instead compare packet->xuid against the IQNetPlayer-derived XUID for
for (unsigned int idx = 0; idx < XUSER_MAX_COUNT; ++idx) // every locally-registered controller slot. This check is race-free: the IQNet slot is
// populated by AddLocalPlayerByUserIndex *before* the connection loop starts, so
// GetLocalPlayerByUserIndex is valid long before handleAddPlayer runs.
{ {
if (minecraft->localplayers[idx] != NULL && minecraft->localplayers[idx]->name == packet->name) const PlayerUID WIN64_XUID_BASE = (PlayerUID)0xe000d45248242f2e;
for(unsigned int idx = 0; idx < XUSER_MAX_COUNT; ++idx)
{ {
app.DebugPrintf("AddPlayerPacket received for local player name %ls\n", packet->name.c_str()); INetworkPlayer *localNetPlayer = g_NetworkManager.GetLocalPlayerByUserIndex(idx);
return; if(localNetPlayer != NULL)
{
PlayerUID localXuid = WIN64_XUID_BASE + localNetPlayer->GetSmallId();
if(localXuid == packet->xuid)
{
app.DebugPrintf("AddPlayerPacket received for local controller %d (xuid match), skipping RemotePlayer creation\n", idx);
return;
}
}
} }
} }
#endif #endif
/*#ifdef _WINDOWS64
// On Windows64 all XUIDs are INVALID_XUID so the XUID check above never fires.
// packet->m_playerIndex is the server-assigned sequential index (set via LoginPacket),
// NOT the controller slot — so we must scan all local player slots and match by
// their stored server index rather than using it directly as an array subscript.
for(unsigned int idx = 0; idx < XUSER_MAX_COUNT; ++idx)
{
if(minecraft->localplayers[idx] != NULL &&
minecraft->localplayers[idx]->getPlayerIndex() == packet->m_playerIndex)
{
app.DebugPrintf("AddPlayerPacket received for local player (controller %d, server index %d), skipping RemotePlayer creation\n", idx, packet->m_playerIndex);
return;
}
}
#endif*/
double x = packet->x / 32.0; double x = packet->x / 32.0;
double y = packet->y / 32.0; double y = packet->y / 32.0;
@ -812,50 +811,22 @@ void ClientConnection::handleAddPlayer(shared_ptr<AddPlayerPacket> packet)
#ifdef _WINDOWS64 #ifdef _WINDOWS64
{ {
IQNetPlayer* matchedQNetPlayer = NULL;
PlayerUID pktXuid = player->getXuid(); PlayerUID pktXuid = player->getXuid();
const PlayerUID WIN64_XUID_BASE = (PlayerUID)0xe000d45248242f2e; const PlayerUID WIN64_XUID_BASE = (PlayerUID)0xe000d45248242f2e;
// Legacy compatibility path for peers still using embedded smallId XUIDs.
if (pktXuid >= WIN64_XUID_BASE && pktXuid < WIN64_XUID_BASE + MINECRAFT_NET_MAX_PLAYERS) if (pktXuid >= WIN64_XUID_BASE && pktXuid < WIN64_XUID_BASE + MINECRAFT_NET_MAX_PLAYERS)
{ {
BYTE smallId = (BYTE)(pktXuid - WIN64_XUID_BASE); BYTE smallId = (BYTE)(pktXuid - WIN64_XUID_BASE);
INetworkPlayer* np = g_NetworkManager.GetPlayerBySmallId(smallId); INetworkPlayer* np = g_NetworkManager.GetPlayerBySmallId(smallId);
if (np != NULL) if (np != NULL)
{ {
NetworkPlayerXbox* npx = (NetworkPlayerXbox*)np;
matchedQNetPlayer = npx->GetQNetPlayer();
}
}
// Current Win64 path: identify QNet player by name and attach packet XUID.
if (matchedQNetPlayer == NULL)
{
for (BYTE smallId = 0; smallId < MINECRAFT_NET_MAX_PLAYERS; ++smallId)
{
INetworkPlayer* np = g_NetworkManager.GetPlayerBySmallId(smallId);
if (np == NULL)
continue;
NetworkPlayerXbox* npx = (NetworkPlayerXbox*)np; NetworkPlayerXbox* npx = (NetworkPlayerXbox*)np;
IQNetPlayer* qp = npx->GetQNetPlayer(); IQNetPlayer* qp = npx->GetQNetPlayer();
if (qp != NULL && _wcsicmp(qp->m_gamertag, packet->name.c_str()) == 0) if (qp != NULL && qp->m_gamertag[0] == 0)
{ {
matchedQNetPlayer = qp; wcsncpy_s(qp->m_gamertag, 32, packet->name.c_str(), _TRUNCATE);
break;
} }
} }
} }
if (matchedQNetPlayer != NULL)
{
// Store packet-authoritative XUID on this network slot so later lookups by XUID
// (e.g. remove player, display mapping) work for both legacy and uid.dat clients.
matchedQNetPlayer->m_resolvedXuid = pktXuid;
if (matchedQNetPlayer->m_gamertag[0] == 0)
{
wcsncpy_s(matchedQNetPlayer->m_gamertag, 32, packet->name.c_str(), _TRUNCATE);
}
}
} }
#endif #endif
@ -1025,8 +996,6 @@ void ClientConnection::handleRemoveEntity(shared_ptr<RemoveEntitiesPacket> packe
qp->m_smallId = 0; qp->m_smallId = 0;
qp->m_isRemote = false; qp->m_isRemote = false;
qp->m_isHostPlayer = false; qp->m_isHostPlayer = false;
// Clear resolved id to avoid stale XUID -> player matches after disconnect.
qp->m_resolvedXuid = INVALID_XUID;
qp->m_gamertag[0] = 0; qp->m_gamertag[0] = 0;
qp->SetCustomDataValue(0); qp->SetCustomDataValue(0);
} }
@ -1441,9 +1410,6 @@ void ClientConnection::handleChat(shared_ptr<ChatPacket> packet)
switch(packet->m_messageType) switch(packet->m_messageType)
{ {
case ChatPacket::e_ChatCustom:
message = (packet->m_stringArgs.size() >= 1) ? packet->m_stringArgs[0] : L"";
break;
case ChatPacket::e_ChatBedOccupied: case ChatPacket::e_ChatBedOccupied:
message = app.GetString(IDS_TILE_BED_OCCUPIED); message = app.GetString(IDS_TILE_BED_OCCUPIED);
break; break;
@ -2387,12 +2353,14 @@ void ClientConnection::handleAddMob(shared_ptr<AddMobPacket> packet)
mob->xRotp = packet->xRot; mob->xRotp = packet->xRot;
vector<shared_ptr<Entity> > *subEntities = mob->getSubEntities(); vector<shared_ptr<Entity> > *subEntities = mob->getSubEntities();
if (subEntities) if (subEntities != NULL)
{ {
int offs = packet->id - mob->entityId; int offs = packet->id - mob->entityId;
for (auto& it : *subEntities ) //for (int i = 0; i < subEntities.length; i++)
{ for(AUTO_VAR(it, subEntities->begin()); it != subEntities->end(); ++it)
it->entityId += offs; {
//subEntities[i].entityId += offs;
(*it)->entityId += offs;
} }
} }
@ -2734,10 +2702,6 @@ void ClientConnection::handleRespawn(shared_ptr<RespawnPacket> packet)
int oldDimension = minecraft->localplayers[m_userIndex]->dimension; int oldDimension = minecraft->localplayers[m_userIndex]->dimension;
started = false; started = false;
// Stop any streaming music (e.g. jukebox) when changing dimensions
// so it doesn't leak into the new dimension
level->playStreamingMusic(L"", 0, 0, 0);
// Remove client connection from this level // Remove client connection from this level
level->removeClientConnection(this, false); level->removeClientConnection(this, false);
@ -3937,26 +3901,26 @@ void ClientConnection::handleUpdateAttributes(shared_ptr<UpdateAttributesPacket>
BaseAttributeMap *attributes = (dynamic_pointer_cast<LivingEntity>(entity))->getAttributes(); BaseAttributeMap *attributes = (dynamic_pointer_cast<LivingEntity>(entity))->getAttributes();
unordered_set<UpdateAttributesPacket::AttributeSnapshot *> attributeSnapshots = packet->getValues(); unordered_set<UpdateAttributesPacket::AttributeSnapshot *> attributeSnapshots = packet->getValues();
for ( UpdateAttributesPacket::AttributeSnapshot *attribute : attributeSnapshots ) for (AUTO_VAR(it,attributeSnapshots.begin()); it != attributeSnapshots.end(); ++it)
{ {
UpdateAttributesPacket::AttributeSnapshot *attribute = *it;
AttributeInstance *instance = attributes->getInstance(attribute->getId()); AttributeInstance *instance = attributes->getInstance(attribute->getId());
if (instance) if (instance == NULL)
{ {
// 4J - TODO: revisit, not familiar with the attribute system, why are we passing in MIN_NORMAL (Java's smallest non-zero value conforming to IEEE Standard 754 (?)) and MAX_VALUE // 4J - TODO: revisit, not familiar with the attribute system, why are we passing in MIN_NORMAL (Java's smallest non-zero value conforming to IEEE Standard 754 (?)) and MAX_VALUE
instance = attributes->registerAttribute(new RangedAttribute(attribute->getId(), 0, Double::MIN_NORMAL, Double::MAX_VALUE)); instance = attributes->registerAttribute(new RangedAttribute(attribute->getId(), 0, Double::MIN_NORMAL, Double::MAX_VALUE));
instance->setBaseValue(attribute->getBase()); }
instance->removeModifiers();
unordered_set<AttributeModifier *> *modifiers = attribute->getModifiers(); instance->setBaseValue(attribute->getBase());
instance->removeModifiers();
if ( modifiers ) unordered_set<AttributeModifier *> *modifiers = attribute->getModifiers();
{
for ( AttributeModifier* modifier : *modifiers ) for (AUTO_VAR(it2,modifiers->begin()); it2 != modifiers->end(); ++it2)
{ {
instance->addModifier(new AttributeModifier(modifier->getId(), modifier->getAmount(), modifier->getOperation())); AttributeModifier* modifier = *it2;
} instance->addModifier(new AttributeModifier(modifier->getId(), modifier->getAmount(), modifier->getOperation() ) );
}
} }
} }
} }