From dee559bd16e5fc4fb1d8cdd16e7e3924666b01c9 Mon Sep 17 00:00:00 2001 From: Loki Rautio Date: Thu, 26 Mar 2026 01:37:23 -0500 Subject: [PATCH] Revert "Memory leak fix: Make chunks unload properly (#1406)" This reverts commit a24318eedc44af2f5967c4727b7ebf578b8e233a. This fix introduces broken behavior for dedicated servers. It will be merged back in once the related issue is fixed --- Minecraft.Client/MultiPlayerChunkCache.cpp | 25 ++++------ Minecraft.Client/PlayerChunkMap.cpp | 25 ---------- Minecraft.Client/PlayerList.cpp | 11 +---- Minecraft.Client/ServerChunkCache.cpp | 55 +++++++++++++++------- 4 files changed, 47 insertions(+), 69 deletions(-) diff --git a/Minecraft.Client/MultiPlayerChunkCache.cpp b/Minecraft.Client/MultiPlayerChunkCache.cpp index 03c47fcca..62361ce36 100644 --- a/Minecraft.Client/MultiPlayerChunkCache.cpp +++ b/Minecraft.Client/MultiPlayerChunkCache.cpp @@ -139,26 +139,19 @@ bool MultiPlayerChunkCache::reallyHasChunk(int x, int z) return hasData[idx]; } -void MultiPlayerChunkCache::drop(const int x, const int z) +void MultiPlayerChunkCache::drop(int x, int z) { - const int ix = x + XZOFFSET; - const int iz = z + XZOFFSET; - if ((ix < 0) || (ix >= XZSIZE)) return; - if ((iz < 0) || (iz >= XZSIZE)) return; - const int idx = ix * XZSIZE + iz; - LevelChunk* chunk = cache[idx]; - - if (chunk != nullptr && !chunk->isEmpty()) + // 4J Stu - We do want to drop any entities in the chunks, especially for the case when a player is dead as they will + // not get the RemoveEntity packet if an entity is removed. + LevelChunk *chunk = getChunk(x, z); + if (!chunk->isEmpty()) { - // Unload chunk but keep tile entities + // Added parameter here specifies that we don't want to delete tile entities, as they won't get recreated unless they've got update packets + // The tile entities are in general only created on the client by virtue of the chunk rebuild chunk->unload(false); - const auto it = std::find(loadedChunkList.begin(), loadedChunkList.end(), chunk); - if (it != loadedChunkList.end()) loadedChunkList.erase(it); - - cache[idx] = nullptr; - hasData[idx] = false; - chunk->loaded = false; + // 4J - We just want to clear out the entities in the chunk, but everything else should be valid + chunk->loaded = true; } } diff --git a/Minecraft.Client/PlayerChunkMap.cpp b/Minecraft.Client/PlayerChunkMap.cpp index ddf2bae2f..bcc3f6ba0 100644 --- a/Minecraft.Client/PlayerChunkMap.cpp +++ b/Minecraft.Client/PlayerChunkMap.cpp @@ -792,14 +792,6 @@ void PlayerChunkMap::setRadius(int newRadius) int xc = static_cast(player->x) >> 4; int zc = static_cast(player->z) >> 4; - for (auto it = addRequests.begin(); it != addRequests.end(); ) - { - if (it->player == player) - it = addRequests.erase(it); - else - ++it; - } - for (int x = xc - newRadius; x <= xc + newRadius; x++) for (int z = zc - newRadius; z <= zc + newRadius; z++) { @@ -809,26 +801,9 @@ void PlayerChunkMap::setRadius(int newRadius) getChunkAndAddPlayer(x, z, player); } } - - // Remove chunks that are outside the new radius - for (int x = xc - radius; x <= xc + radius; x++) - { - for (int z = zc - radius; z <= zc + radius; z++) - { - if (x < xc - newRadius || x > xc + newRadius || z < zc - newRadius || z > zc + newRadius) - { - getChunkAndRemovePlayer(x, z, player); - } - } - } } } - if (newRadius < radius) - { - level->cache->dropAll(); - } - assert(radius <= MAX_VIEW_DISTANCE); assert(radius >= MIN_VIEW_DISTANCE); this->radius = newRadius; diff --git a/Minecraft.Client/PlayerList.cpp b/Minecraft.Client/PlayerList.cpp index 331539cb4..ba82ec6ac 100644 --- a/Minecraft.Client/PlayerList.cpp +++ b/Minecraft.Client/PlayerList.cpp @@ -1690,16 +1690,7 @@ bool PlayerList::isXuidBanned(PlayerUID xuid) } // AP added for Vita so the range can be increased once the level starts -void PlayerList::setViewDistance(const int newViewDistance) +void PlayerList::setViewDistance(int newViewDistance) { viewDistance = newViewDistance; - - for (size_t i = 0; i < server->levels.length; i++) - { - ServerLevel* level = server->levels[i]; - if (level != nullptr) - { - level->getChunkMap()->setRadius(newViewDistance); - } - } } diff --git a/Minecraft.Client/ServerChunkCache.cpp b/Minecraft.Client/ServerChunkCache.cpp index 54312ffa1..c7d70c7d3 100644 --- a/Minecraft.Client/ServerChunkCache.cpp +++ b/Minecraft.Client/ServerChunkCache.cpp @@ -80,31 +80,54 @@ vector *ServerChunkCache::getLoadedChunkList() return &m_loadedChunkList; } -void ServerChunkCache::drop(const int x, const int z) +void ServerChunkCache::drop(int x, int z) { - const int ix = x + XZOFFSET; - const int iz = z + XZOFFSET; - if ((ix < 0) || (ix >= XZSIZE)) return; - if ((iz < 0) || (iz >= XZSIZE)) return; - const int idx = ix * XZSIZE + iz; - LevelChunk* chunk = cache[idx]; + // 4J - we're not dropping things anymore now that we have a fixed sized cache +#ifdef _LARGE_WORLDS - if (chunk != nullptr) + bool canDrop = false; +// if (level->dimension->mayRespawn()) +// { +// Pos *spawnPos = level->getSharedSpawnPos(); +// int xd = x * 16 + 8 - spawnPos->x; +// int zd = z * 16 + 8 - spawnPos->z; +// delete spawnPos; +// int r = 128; +// if (xd < -r || xd > r || zd < -r || zd > r) +// { +// canDrop = true; +//} +// } +// else { - const auto it = std::find(m_loadedChunkList.begin(), m_loadedChunkList.end(), chunk); - if (it != m_loadedChunkList.end()) m_loadedChunkList.erase(it); - - cache[idx] = nullptr; - chunk->loaded = false; + canDrop = true; } + if(canDrop) + { + int ix = x + XZOFFSET; + int iz = z + XZOFFSET; + // Check we're in range of the stored level + if( ( ix < 0 ) || ( ix >= XZSIZE ) ) return; + if( ( iz < 0 ) || ( iz >= XZSIZE ) ) return; + int idx = ix * XZSIZE + iz; + LevelChunk *chunk = cache[idx]; + + if(chunk) + { + m_toDrop.push_back(chunk); + } + } +#endif } void ServerChunkCache::dropAll() { +#ifdef _LARGE_WORLDS for (LevelChunk *chunk : m_loadedChunkList) { drop(chunk->x, chunk->z); - } +} +#endif } // 4J - this is the original (and virtual) interface to create @@ -934,10 +957,6 @@ bool ServerChunkCache::tick() m_unloadedCache[idx] = chunk; cache[idx] = nullptr; } - else - { - continue; - } } m_toDrop.pop_front(); }