diff --git a/src/BlockEntities/JukeboxEntity.cpp b/src/BlockEntities/JukeboxEntity.cpp index 11d50b19a..b99d9d39c 100644 --- a/src/BlockEntities/JukeboxEntity.cpp +++ b/src/BlockEntities/JukeboxEntity.cpp @@ -24,19 +24,6 @@ cJukeboxEntity::cJukeboxEntity(BLOCKTYPE a_BlockType, NIBBLETYPE a_BlockMeta, Ve -cJukeboxEntity::~cJukeboxEntity() -{ - if (m_World && IsPlayingRecord()) - { - // Stop playing music when destroyed by any means - m_World->BroadcastSoundParticleEffect(EffectID::SFX_RANDOM_PLAY_MUSIC_DISC, GetPos(), 0); - } -} - - - - - void cJukeboxEntity::Destroy(void) { ASSERT(m_World != nullptr); diff --git a/src/BlockEntities/JukeboxEntity.h b/src/BlockEntities/JukeboxEntity.h index 5af2012bd..6e8443a7e 100644 --- a/src/BlockEntities/JukeboxEntity.h +++ b/src/BlockEntities/JukeboxEntity.h @@ -22,7 +22,6 @@ public: // tolua_export BLOCKENTITY_PROTODEF(cJukeboxEntity) cJukeboxEntity(BLOCKTYPE a_BlockType, NIBBLETYPE a_BlockMeta, Vector3i a_Pos, cWorld * a_World); - virtual ~cJukeboxEntity() override; // tolua_begin diff --git a/src/Chunk.cpp b/src/Chunk.cpp index 8f72d7c0f..df9292fae 100644 --- a/src/Chunk.cpp +++ b/src/Chunk.cpp @@ -103,24 +103,9 @@ cChunk::cChunk( cChunk::~cChunk() { - cPluginManager::Get()->CallHookChunkUnloaded(*m_World, m_PosX, m_PosZ); - // LOGINFO("### delete cChunk() (%i, %i) from %p, thread 0x%x ###", m_PosX, m_PosZ, this, GetCurrentThreadId()); - m_BlockEntities.clear(); - - // Remove and destroy all entities that are not players: - cEntityList Entities; - std::swap(Entities, m_Entities); // Need another list because cEntity destructors check if they've been removed from chunk - for (auto & Entity : Entities) - { - if (!Entity->IsPlayer()) - { - // Scheduling a normal destruction is neither possible (Since this chunk will be gone till the schedule occurs) nor necessary. - Entity->DestroyNoScheduling(false); // No point in broadcasting in an unloading chunk. Chunks unload when no one is nearby. - } - } - + // Inform our neighbours that we're no longer valid: if (m_NeighborXM != nullptr) { m_NeighborXM->m_NeighborXP = nullptr; @@ -137,6 +122,7 @@ cChunk::~cChunk() { m_NeighborZP->m_NeighborZM = nullptr; } + delete m_WaterSimulatorData; m_WaterSimulatorData = nullptr; delete m_LavaSimulatorData; @@ -221,6 +207,25 @@ bool cChunk::CanUnloadAfterSaving(void) const +void cChunk::OnUnload() +{ + // Note: this is only called during normal operation, not during shutdown + + // Notify all entities of imminent unload: + for (auto & Entity : m_Entities) + { + // Chunks cannot be unloaded when they still contain players: + ASSERT(!Entity->IsPlayer()); + + // Notify the entity: + Entity->OnRemoveFromWorld(*Entity->GetWorld()); + } +} + + + + + void cChunk::MarkSaving(void) { m_IsSaving = true; diff --git a/src/Chunk.h b/src/Chunk.h index 8aa73cde5..04f305f69 100644 --- a/src/Chunk.h +++ b/src/Chunk.h @@ -90,6 +90,11 @@ public: /** Returns true if the chunk could have been unloaded if it weren't dirty */ bool CanUnloadAfterSaving(void) const; + /** Called when the chunkmap unloads unused chunks. + Notifies contained entities that they are being unloaded and should for example, broadcast a destroy packet. + Not called during server shutdown; such cleanup during shutdown is unnecessary. */ + void OnUnload(); + bool IsLightValid(void) const {return m_IsLightValid; } /* diff --git a/src/ChunkMap.cpp b/src/ChunkMap.cpp index 71b8a7a24..53bb905a6 100644 --- a/src/ChunkMap.cpp +++ b/src/ChunkMap.cpp @@ -1717,10 +1717,17 @@ void cChunkMap::UnloadUnusedChunks(void) for (auto itr = m_Chunks.begin(); itr != m_Chunks.end();) { if ( - (itr->second.CanUnload()) && // Can unload + itr->second.CanUnload() && // Can unload !cPluginManager::Get()->CallHookChunkUnloading(*GetWorld(), itr->first.ChunkX, itr->first.ChunkZ) // Plugins agree ) { + // First notify plugins: + cPluginManager::Get()->CallHookChunkUnloaded(*m_World, itr->first.ChunkX, itr->first.ChunkZ); + + // Notify entities within the chunk, while everything's still valid: + itr->second.OnUnload(); + + // Kill the chunk: itr = m_Chunks.erase(itr); } else diff --git a/src/ClientHandle.cpp b/src/ClientHandle.cpp index ce4a56ae0..28cdb4aa7 100644 --- a/src/ClientHandle.cpp +++ b/src/ClientHandle.cpp @@ -129,7 +129,6 @@ cClientHandle::~cClientHandle() { RemoveFromAllChunks(); m_Player->GetWorld()->RemoveClientFromChunkSender(this); - m_Player->DestroyNoScheduling(true); } // Send the Offline PlayerList packet: cRoot::Get()->BroadcastPlayerListsRemovePlayer(*m_Player); diff --git a/src/Entities/Entity.cpp b/src/Entities/Entity.cpp index 3674d16c1..34d5bf6e5 100644 --- a/src/Entities/Entity.cpp +++ b/src/Entities/Entity.cpp @@ -250,21 +250,6 @@ void cEntity::Destroy() -void cEntity::DestroyNoScheduling(bool a_ShouldBroadcast) -{ - SetIsTicking(false); - if (a_ShouldBroadcast) - { - m_World->BroadcastDestroyEntity(*this); - } - - Destroyed(); -} - - - - - void cEntity::TakeDamage(cEntity & a_Attacker) { int RawDamage = a_Attacker.GetRawDamageAgainst(*this); diff --git a/src/Entities/Entity.h b/src/Entities/Entity.h index 627c1ce71..4322665c3 100644 --- a/src/Entities/Entity.h +++ b/src/Entities/Entity.h @@ -341,9 +341,6 @@ public: initialize clients with our position. */ Vector3d GetLastSentPosition(void) const { return m_LastSentPosition; } - /** Destroy the entity without scheduling memory freeing. This should only be used by cChunk or cClientHandle for internal memory management. */ - void DestroyNoScheduling(bool a_ShouldBroadcast); - /** Makes this entity take damage specified in the a_TDI. The TDI is sent through plugins first, then applied. If it returns false, the entity hasn't receive any damage. */ diff --git a/src/World.cpp b/src/World.cpp index 3b82d9e97..ed29123b8 100644 --- a/src/World.cpp +++ b/src/World.cpp @@ -226,6 +226,7 @@ cWorld::cWorld( cFile::CreateFolderRecursive(m_DataPath); + // TODO: unique ptr unnecessary m_ChunkMap = std::make_unique(this); m_ChunkMap->TrackInDeadlockDetect(a_DeadlockDetect, m_WorldName); @@ -964,11 +965,6 @@ void cWorld::Stop(cDeadlockDetect & a_DeadlockDetect) m_MapManager.SaveMapData(); } - - // Explicitly destroy the chunkmap, so that it's guaranteed to be destroyed before the other internals - // This fixes crashes on stopping the server, because chunk destructor deletes entities and those access the world. - // TODO: destructors should only be used for releasing resources, not doing extra work - m_ChunkMap.reset(); }