From 6ab9afd0fd808fad99cd8387c72ce461c37aef80 Mon Sep 17 00:00:00 2001 From: Tiger Wang Date: Sun, 20 Jul 2014 10:46:45 +0100 Subject: Bug and crash fixes * Fixes end portals' solidity * Fixed crashes to do with multithreading and removing an entity from the wrong world * Fixed crashes due to bad merge * Fixed crashes due to an object being deleted twice * Simplified cWorld::Start() and added comments to configuration files --- src/BlockInfo.cpp | 1 + src/Chunk.cpp | 5 ++--- src/ClientHandle.cpp | 2 ++ src/Entities/Entity.cpp | 48 ++++++++++++++++++++++++++++++------------------ src/Entities/Entity.h | 15 +++++++++------ src/Entities/Player.cpp | 35 +++++++++++++++++------------------ src/Entities/Player.h | 15 +++++++++------ src/MobSpawner.cpp | 3 ++- src/Root.cpp | 9 +++++++-- src/Root.h | 13 +++++++++++-- src/World.cpp | 24 ++++++++++++++++-------- src/World.h | 2 +- 12 files changed, 107 insertions(+), 65 deletions(-) diff --git a/src/BlockInfo.cpp b/src/BlockInfo.cpp index 97e89359f..0882d2740 100644 --- a/src/BlockInfo.cpp +++ b/src/BlockInfo.cpp @@ -496,6 +496,7 @@ void cBlockInfo::Initialize(cBlockInfoArray & a_Info) a_Info[E_BLOCK_CROPS ].m_IsSolid = false; a_Info[E_BLOCK_DANDELION ].m_IsSolid = false; a_Info[E_BLOCK_DETECTOR_RAIL ].m_IsSolid = false; + a_Info[E_BLOCK_END_PORTAL ].m_IsSolid = false; a_Info[E_BLOCK_FIRE ].m_IsSolid = false; a_Info[E_BLOCK_FLOWER ].m_IsSolid = false; a_Info[E_BLOCK_HEAVY_WEIGHTED_PRESSURE_PLATE].m_IsSolid = false; diff --git a/src/Chunk.cpp b/src/Chunk.cpp index 2f3bdff81..4588de9f3 100644 --- a/src/Chunk.cpp +++ b/src/Chunk.cpp @@ -591,7 +591,6 @@ void cChunk::Tick(float a_Dt) if (!((*itr)->IsMob())) { (*itr)->Tick(a_Dt, *this); - continue; } } // for itr - m_Entitites[] @@ -605,10 +604,10 @@ void cChunk::Tick(float a_Dt) itr = m_Entities.erase(itr); delete ToDelete; } - else if ((*itr)->IsTravellingThroughPortal()) // Remove all entities that are travelling to another world + else if ((*itr)->IsWorldTravellingFrom(m_World)) // Remove all entities that are travelling to another world { MarkDirty(); - (*itr)->SetIsTravellingThroughPortal(false); + (*itr)->SetWorldTravellingFrom(NULL); itr = m_Entities.erase(itr); } else if ( // If any entity moved out of the chunk, move it to the neighbor: diff --git a/src/ClientHandle.cpp b/src/ClientHandle.cpp index f90da94dd..8caaae917 100644 --- a/src/ClientHandle.cpp +++ b/src/ClientHandle.cpp @@ -120,6 +120,8 @@ cClientHandle::~cClientHandle() } if (World != NULL) { + m_Player->SetWorldTravellingFrom(NULL); // Make sure that the player entity is actually removed + World->RemovePlayer(m_Player); // Must be called before cPlayer::Destroy() as otherwise cChunk tries to delete the player, and then we do it again m_Player->Destroy(); } delete m_Player; diff --git a/src/Entities/Entity.cpp b/src/Entities/Entity.cpp index bd1839580..4768d38ae 100644 --- a/src/Entities/Entity.cpp +++ b/src/Entities/Entity.cpp @@ -37,7 +37,7 @@ cEntity::cEntity(eEntityType a_EntityType, double a_X, double a_Y, double a_Z, d , m_Gravity(-9.81f) , m_LastPos(a_X, a_Y, a_Z) , m_IsInitialized(false) - , m_IsTravellingThroughPortal(false) + , m_WorldTravellingFrom(NULL) , m_EntityType(a_EntityType) , m_World(NULL) , m_IsFireproof(false) @@ -1028,10 +1028,11 @@ void cEntity::DetectCacti(void) void cEntity::DetectPortal() { - if (!GetWorld()->GetNetherWorldName().empty() && !GetWorld()->GetEndWorldName().empty()) + if (GetWorld()->GetDimension() == dimOverworld) { - return; + if (GetWorld()->GetNetherWorldName().empty() && GetWorld()->GetEndWorldName().empty()) { return; } } + else if (GetWorld()->GetLinkedOverworldName().empty()) { return; } int X = POSX_TOINT, Y = POSY_TOINT, Z = POSZ_TOINT; if ((Y > 0) && (Y < cChunkDef::Height)) @@ -1040,7 +1041,7 @@ void cEntity::DetectPortal() { case E_BLOCK_NETHER_PORTAL: { - if (GetWorld()->GetNetherWorldName().empty() || m_PortalCooldownData.second) + if (m_PortalCooldownData.second) { return; } @@ -1054,8 +1055,13 @@ void cEntity::DetectPortal() switch (GetWorld()->GetDimension()) { - case dimNether: + case dimNether: { + if (GetWorld()->GetLinkedOverworldName().empty()) + { + return; + } + m_PortalCooldownData.second = true; // Stop portals from working on respawn if (IsPlayer()) @@ -1068,6 +1074,11 @@ void cEntity::DetectPortal() } case dimOverworld: { + if (GetWorld()->GetNetherWorldName().empty()) + { + return; + } + m_PortalCooldownData.second = true; // Stop portals from working on respawn if (IsPlayer()) @@ -1079,28 +1090,25 @@ void cEntity::DetectPortal() return; } - default: break; + default: return; } - return; } case E_BLOCK_END_PORTAL: { - if (GetWorld()->GetNetherWorldName().empty() || m_PortalCooldownData.second) - { - return; - } - - if (m_PortalCooldownData.first != 80) + if (m_PortalCooldownData.second) { - m_PortalCooldownData.first++; return; } - m_PortalCooldownData.first = 0; switch (GetWorld()->GetDimension()) { case dimEnd: { + if (GetWorld()->GetLinkedOverworldName().empty()) + { + return; + } + m_PortalCooldownData.second = true; // Stop portals from working on respawn if (IsPlayer()) @@ -1115,6 +1123,11 @@ void cEntity::DetectPortal() } case dimOverworld: { + if (GetWorld()->GetEndWorldName().empty()) + { + return; + } + m_PortalCooldownData.second = true; // Stop portals from working on respawn if (IsPlayer()) @@ -1126,9 +1139,8 @@ void cEntity::DetectPortal() return; } - default: break; + default: return; } - return; } default: break; } @@ -1169,7 +1181,7 @@ bool cEntity::MoveToWorld(const AString & a_WorldName, cWorld * a_World, bool a_ } // Remove all links to the old world - SetIsTravellingThroughPortal(true); // cChunk handles entity removal + SetWorldTravellingFrom(GetWorld()); // cChunk handles entity removal GetWorld()->BroadcastDestroyEntity(*this); // Queue add to new world diff --git a/src/Entities/Entity.h b/src/Entities/Entity.h index 8b6fc05f7..eea48a12f 100644 --- a/src/Entities/Entity.h +++ b/src/Entities/Entity.h @@ -387,11 +387,11 @@ public: // tolua_end - /** Returns if the entity is travelling through a portal. Set to true by MoveToWorld and to false when the entity is removed by the old chunk */ - bool IsTravellingThroughPortal(void) const { return m_IsTravellingThroughPortal; } + /** Returns if the entity is travelling away from a specified world */ + bool IsWorldTravellingFrom(cWorld * a_World) const { return (m_WorldTravellingFrom == a_World); } - /** Sets if the entity has begun travelling through a portal or not */ - void SetIsTravellingThroughPortal(bool a_Flag) { m_IsTravellingThroughPortal = a_Flag; } + /** Sets the world the entity will be leaving */ + void SetWorldTravellingFrom(cWorld * a_World) { (m_WorldTravellingFrom = a_World); } /// Updates clients of changes in the entity. virtual void BroadcastMovementUpdate(const cClientHandle * a_Exclude = NULL); @@ -491,8 +491,11 @@ protected: /** True when entity is initialised (Initialize()) and false when destroyed pending deletion (Destroy()) */ bool m_IsInitialized; - /** True when entity is being moved across worlds, false anytime else */ - bool m_IsTravellingThroughPortal; + /** World entity is travelling from + Set by MoveToWorld and back to NULL when the entity is removed by the old chunk + Can't be a simple boolean as context switches between worlds may leave the new chunk processing (and therefore immediately removing) the entity before the old chunk could remove it + */ + cWorld * m_WorldTravellingFrom; eEntityType m_EntityType; diff --git a/src/Entities/Player.cpp b/src/Entities/Player.cpp index 2a91600e7..0b1b4ce5f 100644 --- a/src/Entities/Player.cpp +++ b/src/Entities/Player.cpp @@ -88,7 +88,7 @@ cPlayer::cPlayer(cClientHandle* a_Client, const AString & a_PlayerName) : m_PlayerName = a_PlayerName; - cWorld * World; + cWorld * World = NULL; if (!LoadFromDisk(World)) { m_Inventory.Clear(); @@ -136,8 +136,6 @@ cPlayer::~cPlayer(void) SaveToDisk(); - m_World->RemovePlayer(this); - m_ClientHandle = NULL; delete m_InventoryWindow; @@ -979,7 +977,7 @@ void cPlayer::Respawn(void) m_LifetimeTotalXp = 0; // ToDo: send score to client? How? - m_ClientHandle->SendRespawn(GetWorld()->GetDimension()); + m_ClientHandle->SendRespawn(GetWorld()->GetDimension(), true); // Extinguish the fire: StopBurning(); @@ -1643,11 +1641,12 @@ bool cPlayer::MoveToWorld(const AString & a_WorldName, cWorld * a_World, bool a_ } // Remove player from the old world - SetIsTravellingThroughPortal(true); // cChunk handles entity removal - m_World->RemovePlayer(this); + SetWorldTravellingFrom(GetWorld()); // cChunk handles entity removal + GetWorld()->RemovePlayer(this); // Queue adding player to the new world, including all the necessary adjustments to the object World->AddPlayer(this); + SetWorld(World); return true; } @@ -1697,12 +1696,6 @@ void cPlayer::LoadPermissionsFromDisk() bool cPlayer::LoadFromDisk(cWorldPtr & a_World) { - a_World = cRoot::Get()->GetWorld(GetLoadedWorldName()); - if (a_World == NULL) - { - a_World = cRoot::Get()->GetDefaultWorld(); - } - LoadPermissionsFromDisk(); // Load from the UUID file: @@ -1740,6 +1733,11 @@ bool cPlayer::LoadFromDisk(cWorldPtr & a_World) LOG("Player data file not found for %s (%s, offline %s), will be reset to defaults.", GetName().c_str(), m_UUID.c_str(), OfflineUUID.c_str() ); + + if (a_World == NULL) + { + a_World = cRoot::Get()->GetDefaultWorld(); + } return false; } @@ -1747,7 +1745,7 @@ bool cPlayer::LoadFromDisk(cWorldPtr & a_World) -bool cPlayer::LoadFromFile(const AString & a_FileName, cWorld * a_World) +bool cPlayer::LoadFromFile(const AString & a_FileName, cWorldPtr & a_World) { // Load the data from the file: cFile f; @@ -1800,9 +1798,6 @@ bool cPlayer::LoadFromFile(const AString & a_FileName, cWorld * a_World) m_LifetimeTotalXp = (short) root.get("xpTotal", 0).asInt(); m_CurrentXp = (short) root.get("xpCurrent", 0).asInt(); m_IsFlying = root.get("isflying", 0).asBool(); - m_LastBedPos.x = root.get("SpawnX", a_World->GetSpawnX()).asInt(); - m_LastBedPos.y = root.get("SpawnY", a_World->GetSpawnY()).asInt(); - m_LastBedPos.z = root.get("SpawnZ", a_World->GetSpawnZ()).asInt(); m_GameMode = (eGameMode) root.get("gamemode", eGameMode_NotSet).asInt(); @@ -1815,6 +1810,11 @@ bool cPlayer::LoadFromFile(const AString & a_FileName, cWorld * a_World) cEnderChestEntity::LoadFromJson(root["enderchestinventory"], m_EnderChestContents); m_LoadedWorldName = root.get("world", "world").asString(); + a_World = cRoot::Get()->GetWorld(GetLoadedWorldName(), true); + + m_LastBedPos.x = root.get("SpawnX", a_World->GetSpawnX()).asInt(); + m_LastBedPos.y = root.get("SpawnY", a_World->GetSpawnY()).asInt(); + m_LastBedPos.z = root.get("SpawnZ", a_World->GetSpawnZ()).asInt(); // Load the player stats. // We use the default world name (like bukkit) because stats are shared between dimensions/worlds. @@ -1822,7 +1822,7 @@ bool cPlayer::LoadFromFile(const AString & a_FileName, cWorld * a_World) StatSerializer.Load(); LOGD("Player %s was read from file \"%s\", spawning at {%.2f, %.2f, %.2f} in world \"%s\"", - GetName().c_str(), a_FileName.c_str(), GetPosX(), GetPosY(), GetPosZ(), m_LoadedWorldName.c_str() + GetName().c_str(), a_FileName.c_str(), GetPosX(), GetPosY(), GetPosZ(), a_World->GetName().c_str() ); return true; @@ -1834,7 +1834,6 @@ bool cPlayer::LoadFromFile(const AString & a_FileName, cWorld * a_World) bool cPlayer::SaveToDisk() { - cFile::CreateFolder(FILE_IO_PREFIX + AString("players")); cFile::CreateFolder(FILE_IO_PREFIX + AString("players/") + m_UUID.substr(0, 2)); // create the JSON data diff --git a/src/Entities/Player.h b/src/Entities/Player.h index ad434f036..226ec5e68 100644 --- a/src/Entities/Player.h +++ b/src/Entities/Player.h @@ -340,14 +340,17 @@ public: typedef cWorld * cWorldPtr; - /** Loads the player data from the disk file. -Takes a (NULL) cWorld pointer which it will assign a value to based on either the loaded world or default world - Returns true on success, false on failure. */ + /** Loads the player data from the disk file + Takes a (NULL) cWorld pointer which it will assign a value to based on either the loaded world or default world by calling LoadFromFile() + Returns true on success, false on failure + */ bool LoadFromDisk(cWorldPtr & a_World); - /** Loads the player data from the specified file. - Returns true on success, false on failure. */ - bool LoadFromFile(const AString & a_FileName, cWorld * a_World); + /** Loads the player data from the specified file + Takes a (NULL) cWorld pointer which it will assign a value to based on either the loaded world or default world + Returns true on success, false on failure + */ + bool LoadFromFile(const AString & a_FileName, cWorldPtr & a_World); void LoadPermissionsFromDisk(void); // tolua_export diff --git a/src/MobSpawner.cpp b/src/MobSpawner.cpp index da3e7c406..d09bbc943 100644 --- a/src/MobSpawner.cpp +++ b/src/MobSpawner.cpp @@ -241,7 +241,8 @@ bool cMobSpawner::CanSpawnHere(cChunk * a_Chunk, int a_RelX, int a_RelY, int a_R (m_Random.NextInt(2, a_Biome) == 0) ); } - + + case cMonster::mtMagmaCube: case cMonster::mtSlime: { return ( diff --git a/src/Root.cpp b/src/Root.cpp index 822be92ff..e1eebb711 100644 --- a/src/Root.cpp +++ b/src/Root.cpp @@ -322,7 +322,7 @@ cWorld * cRoot::CreateAndInitializeWorld(const AString & a_WorldName, eDimension } cWorld * NewWorld = new cWorld(a_WorldName.c_str(), a_Dimension, a_OverworldName); m_WorldsByName[a_WorldName] = NewWorld; - NewWorld->Start(!a_OverworldName.empty()); + NewWorld->Start(); NewWorld->InitializeSpawn(); m_PluginManager->CallHookWorldStarted(*NewWorld); return NewWorld; @@ -381,13 +381,18 @@ cWorld * cRoot::GetDefaultWorld() -cWorld * cRoot::GetWorld(const AString & a_WorldName) +cWorld * cRoot::GetWorld(const AString & a_WorldName, bool a_SearchForFolder) { WorldMap::iterator itr = m_WorldsByName.find(a_WorldName); if (itr != m_WorldsByName.end()) { return itr->second; } + + if (a_SearchForFolder && cFile::IsFolder(FILE_IO_PREFIX + a_WorldName)) + { + return CreateAndInitializeWorld(a_WorldName); + } return NULL; } diff --git a/src/Root.h b/src/Root.h index c3aca37f9..4e3fcf080 100644 --- a/src/Root.h +++ b/src/Root.h @@ -51,8 +51,17 @@ public: // tolua_begin cServer * GetServer(void) { return m_Server; } cWorld * GetDefaultWorld(void); - cWorld * GetWorld(const AString & a_WorldName); - cWorld * CreateAndInitializeWorld(const AString & a_WorldName, eDimension a_Dimension = dimOverworld, const AString & a_OverworldName = ""); + + /** Returns a pointer to the world specified + If no world of that name was currently loaded and a_SearchForFolder was true, it will consult cFile::IsFolder() to see if a world folder of that name exists and if so, initialise a world based on that name + */ + cWorld * GetWorld(const AString & a_WorldName, bool a_SearchForFolder = false); + + /** Returns a pointer to a world of specified name - will search loaded worlds first, then create anew if not found + The dimension parameter is used to create a world with a specific dimension + a_OverworldName should be set for non-overworld dimensions if one wishes that world to link back to an overworld via portals + */ + cWorld * CreateAndInitializeWorld(const AString & a_WorldName, eDimension a_Dimension = dimOverworld, const AString & a_OverworldName = ""); // tolua_end /// Calls the callback for each world; returns true if the callback didn't abort (return true) diff --git a/src/World.cpp b/src/World.cpp index 924d5adbb..d27ad1eb4 100644 --- a/src/World.cpp +++ b/src/World.cpp @@ -512,7 +512,7 @@ void cWorld::InitializeSpawn(void) -void cWorld::Start(bool a_WasDimensionSet) +void cWorld::Start() { m_SpawnX = 0; m_SpawnY = cChunkDef::Height; @@ -523,11 +523,15 @@ void cWorld::Start(bool a_WasDimensionSet) if (!IniFile.ReadFile(m_IniFileName)) { LOGWARNING("Cannot read world settings from \"%s\", defaults will be used.", m_IniFileName.c_str()); + + // TODO: More descriptions for each key + IniFile.AddHeaderComment(" This is the per-world configuration file, managing settings such as generators, simulators, and spawn points"); + IniFile.AddKeyComment("LinkedWorlds", "This section governs portal world linkage; leave a value blank to disabled that associated method of teleportation"); } - AString Dimension = IniFile.GetValueSet("General", "Dimension", a_WasDimensionSet ? DimensionToString(GetDimension()) : "Overworld"); - m_Dimension = StringToDimension(Dimension); - m_OverworldName = IniFile.GetValue("LinkedWorlds", "OverworldName", a_WasDimensionSet ? m_OverworldName : ""); + // The presence of a configuration value overrides everything + // If no configuration value is found, GetDimension() is written to file and the variable is written to again to ensure that cosmic rays haven't sneakily changed its value + m_Dimension = StringToDimension(IniFile.GetValueSet("General", "Dimension", DimensionToString(GetDimension()))); // Try to find the "SpawnPosition" key and coord values in the world configuration, set the flag if found int KeyNum = IniFile.FindKey("SpawnPosition"); @@ -535,8 +539,8 @@ void cWorld::Start(bool a_WasDimensionSet) ( (KeyNum >= 0) && ( - (IniFile.FindValue(KeyNum, "X") >= 0) || - (IniFile.FindValue(KeyNum, "Y") >= 0) || + (IniFile.FindValue(KeyNum, "X") >= 0) && + (IniFile.FindValue(KeyNum, "Y") >= 0) && (IniFile.FindValue(KeyNum, "Z") >= 0) ) ); @@ -575,11 +579,15 @@ void cWorld::Start(bool a_WasDimensionSet) int Weather = IniFile.GetValueSetI("General", "Weather", (int)m_Weather); m_TimeOfDay = IniFile.GetValueSetI("General", "TimeInTicks", m_TimeOfDay); - if ((GetDimension() != dimNether) && (GetDimension() != dimEnd)) + if (GetDimension() == dimOverworld) { m_NetherWorldName = IniFile.GetValueSet("LinkedWorlds", "NetherWorldName", DEFAULT_NETHER_NAME); m_EndWorldName = IniFile.GetValueSet("LinkedWorlds", "EndWorldName", DEFAULT_END_NAME); } + else + { + m_OverworldName = IniFile.GetValueSet("LinkedWorlds", "OverworldName", GetLinkedOverworldName()); + } // Adjust the enum-backed variables into their respective bounds: m_GameMode = (eGameMode) Clamp(GameMode, (int)gmSurvival, (int)gmAdventure); @@ -2420,7 +2428,7 @@ void cWorld::AddPlayer(cPlayer * a_Player) void cWorld::RemovePlayer(cPlayer * a_Player) { - if (!a_Player->IsTravellingThroughPortal()) + if (!a_Player->IsWorldTravellingFrom(this)) { m_ChunkMap->RemoveEntity(a_Player); } diff --git a/src/World.h b/src/World.h index 73b8b81b0..09cbc6b48 100644 --- a/src/World.h +++ b/src/World.h @@ -669,7 +669,7 @@ public: void InitializeSpawn(void); /** Starts threads that belong to this world */ - void Start(bool a_WasDimensionSet = true); + void Start(); /** Stops threads that belong to this world (part of deinit) */ void Stop(void); -- cgit v1.2.3