From 16aeb84cd35996a6b41f10cbc48a677eeccc911c Mon Sep 17 00:00:00 2001 From: Tiger Wang Date: Sat, 2 Jan 2021 13:50:34 +0000 Subject: Fix potential destruction crashes (#5095) * Fix potential destruction crashes * Fix destructors accessing destroyted objects * Fix cPlayer not destroying windows (Destroyed never called) * Tentatively fixes #4608, fixes #3236, fixes #3262 - Remove cEntity::Destroyed() and replace with cEntity::OnRemoveFromWorld() * Add missing call to OnRemoveFromWorld --- src/Entities/Minecart.cpp | 157 +++++++++++++++++++++++++--------------------- 1 file changed, 84 insertions(+), 73 deletions(-) (limited to 'src/Entities/Minecart.cpp') diff --git a/src/Entities/Minecart.cpp b/src/Entities/Minecart.cpp index 59d369c5b..fbe4f756e 100644 --- a/src/Entities/Minecart.cpp +++ b/src/Entities/Minecart.cpp @@ -1203,7 +1203,6 @@ bool cMinecart::DoTakeDamage(TakeDamageInfo & TDI) { if ((TDI.Attacker != nullptr) && TDI.Attacker->IsPlayer() && static_cast(TDI.Attacker)->IsGameModeCreative()) { - Destroy(); TDI.FinalDamage = GetMaxHealth(); // Instant hit for creative SetInvulnerableTicks(0); return Super::DoTakeDamage(TDI); // No drops for creative @@ -1217,43 +1216,32 @@ bool cMinecart::DoTakeDamage(TakeDamageInfo & TDI) m_World->BroadcastEntityMetadata(*this); - if (GetHealth() <= 0) - { - Destroy(); + return true; +} - cItems Drops; - switch (m_Payload) - { - case mpNone: - { - Drops.push_back(cItem(E_ITEM_MINECART, 1, 0)); - break; - } - case mpChest: - { - Drops.push_back(cItem(E_ITEM_CHEST_MINECART, 1, 0)); - break; - } - case mpFurnace: - { - Drops.push_back(cItem(E_ITEM_FURNACE_MINECART, 1, 0)); - break; - } - case mpTNT: - { - Drops.push_back(cItem(E_ITEM_MINECART_WITH_TNT, 1, 0)); - break; - } - case mpHopper: - { - Drops.push_back(cItem(E_ITEM_MINECART_WITH_HOPPER, 1, 0)); - break; - } - } - m_World->SpawnItemPickups(Drops, GetPosX(), GetPosY(), GetPosZ()); + + + +void cMinecart::KilledBy(TakeDamageInfo & a_TDI) +{ + Super::KilledBy(a_TDI); + + Destroy(); +} + + + + + +void cMinecart::OnRemoveFromWorld(cWorld & a_World) +{ + if (m_bIsOnDetectorRail) + { + m_World->SetBlock(m_DetectorRailPosition.x, m_DetectorRailPosition.y, m_DetectorRailPosition.z, E_BLOCK_DETECTOR_RAIL, m_World->GetBlockMeta(m_DetectorRailPosition) & 0x07); } - return true; + + Super::OnRemoveFromWorld(a_World); } @@ -1314,18 +1302,6 @@ void cMinecart::DoSetSpeed(double a_SpeedX, double a_SpeedY, double a_SpeedZ) -void cMinecart::Destroyed() -{ - if (m_bIsOnDetectorRail) - { - m_World->SetBlock(m_DetectorRailPosition.x, m_DetectorRailPosition.y, m_DetectorRailPosition.z, E_BLOCK_DETECTOR_RAIL, m_World->GetBlockMeta(m_DetectorRailPosition) & 0x07); - } -} - - - - - //////////////////////////////////////////////////////////////////////////////// // cRideableMinecart: @@ -1340,6 +1316,15 @@ cRideableMinecart::cRideableMinecart(Vector3d a_Pos, const cItem & a_Content, in +void cRideableMinecart::GetDrops(cItems & a_Drops, cEntity * a_Killer) +{ + a_Drops.emplace_back(E_ITEM_MINECART); +} + + + + + void cRideableMinecart::OnRightClicked(cPlayer & a_Player) { Super::OnRightClicked(a_Player); @@ -1386,6 +1371,31 @@ cMinecartWithChest::cMinecartWithChest(Vector3d a_Pos): +void cMinecartWithChest::GetDrops(cItems & a_Drops, cEntity * a_Killer) +{ + m_Contents.CopyToItems(a_Drops); + a_Drops.emplace_back(E_ITEM_CHEST_MINECART); +} + + + + + +void cMinecartWithChest::OnRemoveFromWorld(cWorld & a_World) +{ + const auto Window = GetWindow(); + if (Window != nullptr) + { + Window->OwnerDestroyed(); + } + + Super::OnRemoveFromWorld(a_World); +} + + + + + void cMinecartWithChest::OnRightClicked(cPlayer & a_Player) { // If the window is not created, open it anew: @@ -1419,32 +1429,6 @@ void cMinecartWithChest::OpenNewWindow() -void cMinecartWithChest::Destroyed() -{ - if (GetWindow() != nullptr) - { - GetWindow()->OwnerDestroyed(); - } - cItems Pickups; - m_Contents.CopyToItems(Pickups); - - - // Schedule the pickups creation for the next world tick - // This avoids a deadlock when terminating the world - // Note that the scheduled task may be run when this object is no longer valid, we need to store everything in the task's captured variables - auto posX = GetPosX(); - auto posY = GetPosY() + 1; - auto posZ = GetPosZ(); - GetWorld()->ScheduleTask(1, [Pickups, posX, posY, posZ](cWorld & World) - { - World.SpawnItemPickups(Pickups, posX, posY, posZ, 4); - }); -} - - - - - //////////////////////////////////////////////////////////////////////////////// // cMinecartWithFurnace: @@ -1459,6 +1443,15 @@ cMinecartWithFurnace::cMinecartWithFurnace(Vector3d a_Pos): +void cMinecartWithFurnace::GetDrops(cItems & a_Drops, cEntity * a_Killer) +{ + a_Drops.emplace_back(E_ITEM_FURNACE_MINECART); +} + + + + + void cMinecartWithFurnace::OnRightClicked(cPlayer & a_Player) { if (a_Player.GetEquippedItem().m_ItemType == E_ITEM_COAL) @@ -1526,6 +1519,15 @@ cMinecartWithTNT::cMinecartWithTNT(Vector3d a_Pos): +void cMinecartWithTNT::GetDrops(cItems & a_Drops, cEntity * a_Killer) +{ + a_Drops.emplace_back(E_ITEM_MINECART_WITH_TNT); +} + + + + + //////////////////////////////////////////////////////////////////////////////// // cMinecartWithHopper: @@ -1536,3 +1538,12 @@ cMinecartWithHopper::cMinecartWithHopper(Vector3d a_Pos): // TODO: Make it suck up blocks and travel further than any other cart and physics and put and take blocks // AND AVARYTHING!! + + + + + +void cMinecartWithHopper::GetDrops(cItems & a_Drops, cEntity * a_Killer) +{ + a_Drops.emplace_back(E_ITEM_MINECART_WITH_HOPPER); +} -- cgit v1.2.3