From 05c9e06be142aef46f8f2830a5007af4ffa815a0 Mon Sep 17 00:00:00 2001 From: peterbell10 Date: Sat, 30 Nov 2024 11:16:53 +0000 Subject: [PATCH] Remove blocking GetHeight (#4689) * WIP: Remove blocking GetHeight * Make CheckBasicStyle.lua happy * Update comment and style * Update lua bindings and docs * Rework chunk loading in GenerateRandomSpawn Explicitly use chunk stays instead of relying on chunks loaded by `cSpawnPrepare` to not unload. * Fix variable shadowing * Update to C++17 * Add note about empty chunks to IsWeatherWetAtXYZ API docs * Cleanup, fix some of the remaining merge issues. --------- Co-authored-by: Alexander Harkness --- Server/Plugins/APIDump/Classes/World.lua | 14 ++- src/Bindings/ManualBindings_World.cpp | 45 ++++++++-- src/Blocks/BlockCauldron.h | 1 + src/Blocks/WorldInterface.h | 10 ++- src/ChunkMap.cpp | 33 ++----- src/ChunkMap.h | 10 ++- src/SpawnPrepare.h | 7 ++ src/World.cpp | 106 +++++++++++++++-------- src/World.h | 13 +-- 9 files changed, 156 insertions(+), 83 deletions(-) diff --git a/Server/Plugins/APIDump/Classes/World.lua b/Server/Plugins/APIDump/Classes/World.lua index 841c93cd0..e45b72109 100644 --- a/Server/Plugins/APIDump/Classes/World.lua +++ b/Server/Plugins/APIDump/Classes/World.lua @@ -1782,7 +1782,7 @@ function OnAllChunksAvailable() All return values from the callbacks are i Type = "number", }, }, - Notes = "Returns the maximum height of the particula block column in the world. If the chunk is not loaded, it waits for it to load / generate. WARNING: Do not use, Use TryGetHeight() instead for a non-waiting version, otherwise you run the risk of a deadlock!", + Notes = "DEPRECATED, use TryGetHeight instead. Returns the maximum height of the particular block column in the world. If the chunk is not loaded, this function used to block until the chunk was loaded, leading to possible deadlock. Now it returns 0 instead.", }, GetIniFileName = { @@ -2040,6 +2040,16 @@ function OnAllChunksAvailable() All return values from the callbacks are i }, Notes = "Returns the Z coord of the default spawn", }, + GetSpawnPos = + { + Returns = + { + { + Type = "Vector3d" + }, + }, + Notes = "Returns the default spawn position", + }, GetStorageLoadQueueLength = { Returns = @@ -2500,7 +2510,7 @@ function OnAllChunksAvailable() All return values from the callbacks are i Type = "boolean", }, }, - Notes = "Returns true if the specified location has wet weather (rain or storm), using the same logic as IsWeatherWetAt, except that any rain-blocking blocks above the specified position will block the precipitation and this function will return false.", + Notes = "Returns true if the specified location has wet weather (rain or storm), using the same logic as IsWeatherWetAt, except that any rain-blocking blocks above the specified position will block the precipitation and this function will return false. Note if the chunk is unloaded then the weather state for the world will be returned.", }, PickupsFromBlock = { diff --git a/src/Bindings/ManualBindings_World.cpp b/src/Bindings/ManualBindings_World.cpp index 7713b8823..3b3bae172 100644 --- a/src/Bindings/ManualBindings_World.cpp +++ b/src/Bindings/ManualBindings_World.cpp @@ -1689,7 +1689,7 @@ static int tolua_cWorld_SpawnSplitExperienceOrbs(lua_State* tolua_S) static int tolua_cWorld_TryGetHeight(lua_State * tolua_S) { - /* Exported manually, because tolua would require the out-only param a_Height to be used when calling + /* Exported manually because we don't export optional Function signature: world:TryGetHeight(a_World, a_BlockX, a_BlockZ) -> IsValid, Height */ @@ -1716,12 +1716,11 @@ static int tolua_cWorld_TryGetHeight(lua_State * tolua_S) } // Call the implementation: - int Height = 0; - bool res = self->TryGetHeight(BlockX, BlockZ, Height); - L.Push(res); - if (res) + auto Height = self->GetHeight(BlockX, BlockZ); + L.Push(Height.has_value()); + if (Height.has_value()) { - L.Push(Height); + L.Push(Height.value()); return 2; } return 1; @@ -1731,6 +1730,39 @@ static int tolua_cWorld_TryGetHeight(lua_State * tolua_S) +static int tolua_cWorld_GetHeight(lua_State * tolua_S) +{ + // Signature: world:GetHeight(a_World, a_BlockX, a_BlockZ) -> Height + + // Check params: + cLuaState L(tolua_S); + if ( + !L.CheckParamSelf("cWorld") || + !L.CheckParamNumber(2, 3) || + !L.CheckParamEnd(4) + ) + { + return 0; + } + + // Get params: + cWorld * self = nullptr; + int BlockX = 0; + int BlockZ = 0; + L.GetStackValues(1, self, BlockX, BlockZ); + + // Call the implementation: + L.LogStackTrace(); + FLOGWARN("cWorld:GetHeight is DEPRECATED, use TryGetHeight instead"); + auto Height = self->GetHeight(BlockX, BlockZ); + L.Push(Height.value_or(0)); + return 1; +} + + + + + void cManualBindings::BindWorld(lua_State * tolua_S) { tolua_beginmodule(tolua_S, nullptr); @@ -1776,6 +1808,7 @@ void cManualBindings::BindWorld(lua_State * tolua_S) tolua_function(tolua_S, "GetBlockMeta", tolua_cWorld_GetBlockMeta); tolua_function(tolua_S, "GetBlockSkyLight", tolua_cWorld_GetBlockSkyLight); tolua_function(tolua_S, "GetBlockTypeMeta", tolua_cWorld_GetBlockTypeMeta); + tolua_function(tolua_S, "GetHeight", tolua_cWorld_GetHeight); tolua_function(tolua_S, "GetSignLines", tolua_cWorld_GetSignLines); tolua_function(tolua_S, "GetTimeOfDay", tolua_cWorld_GetTimeOfDay); tolua_function(tolua_S, "GetWorldAge", tolua_cWorld_GetWorldAge); diff --git a/src/Blocks/BlockCauldron.h b/src/Blocks/BlockCauldron.h index 82247944c..9742e47a3 100644 --- a/src/Blocks/BlockCauldron.h +++ b/src/Blocks/BlockCauldron.h @@ -186,6 +186,7 @@ private: ) const override { auto WorldPos = a_Chunk.RelativeToAbsolute(a_RelPos); + if (!a_WorldInterface.IsWeatherWetAtXYZ(WorldPos.addedY(1))) { // It's not raining at our current location or we do not have a direct view of the sky diff --git a/src/Blocks/WorldInterface.h b/src/Blocks/WorldInterface.h index b3fc9878d..442bb7171 100644 --- a/src/Blocks/WorldInterface.h +++ b/src/Blocks/WorldInterface.h @@ -4,6 +4,8 @@ #include "../FunctionRef.h" #include "../Mobs/MonsterTypes.h" +#include + class cBlockEntity; class cBroadcastInterface; class cItems; @@ -76,7 +78,9 @@ public: virtual bool IsWeatherWetAt(int a_BlockX, int a_BlockZ) = 0; /** Returns true if it is raining or storming at the specified location, - and the rain reaches the specified block position. */ + and the rain reaches the specified block position. + Returns the global weather state for unloaded chunks. + */ virtual bool IsWeatherWetAtXYZ(Vector3i a_Pos) = 0; /** Returns or sets the minumim or maximum netherportal width */ @@ -91,8 +95,8 @@ public: virtual void SetMinNetherPortalHeight(int a_NewMinHeight) = 0; virtual void SetMaxNetherPortalHeight(int a_NewMaxHeight) = 0; - /** Returns the world height at the specified coords; waits for the chunk to get loaded / generated */ - virtual int GetHeight(int a_BlockX, int a_BlockZ) = 0; + /** Returns the world height at the specified coords; returns nullopt for unloaded / generated chunks */ + virtual std::optional GetHeight(int a_BlockX, int a_BlockZ) = 0; /** Wakes up the simulators for the specified block */ virtual void WakeUpSimulators(Vector3i a_Block) = 0; diff --git a/src/ChunkMap.cpp b/src/ChunkMap.cpp index d982d0cf3..85ce7b0c1 100644 --- a/src/ChunkMap.cpp +++ b/src/ChunkMap.cpp @@ -380,42 +380,19 @@ bool cChunkMap::HasChunkAnyClients(int a_ChunkX, int a_ChunkZ) const -int cChunkMap::GetHeight(int a_BlockX, int a_BlockZ) +std::optional cChunkMap::GetHeight(int a_BlockX, int a_BlockZ) { - for (;;) - { - cCSLock Lock(m_CSChunks); - int ChunkX, ChunkZ, BlockY = 0; - cChunkDef::AbsoluteToRelative(a_BlockX, BlockY, a_BlockZ, ChunkX, ChunkZ); - auto & Chunk = GetChunk(ChunkX, ChunkZ); - if (Chunk.IsValid()) - { - return Chunk.GetHeight(a_BlockX, a_BlockZ); - } - - // The chunk is not valid, wait for it to become valid: - cCSUnlock Unlock(Lock); - m_evtChunkValid.Wait(); - } // while (true) -} - - - - - -bool cChunkMap::TryGetHeight(int a_BlockX, int a_BlockZ, int & a_Height) -{ - // Returns false if chunk not loaded / generated + // Returns std::nullopt if chunk not loaded / generated. cCSLock Lock(m_CSChunks); int ChunkX, ChunkZ, BlockY = 0; cChunkDef::AbsoluteToRelative(a_BlockX, BlockY, a_BlockZ, ChunkX, ChunkZ); const auto Chunk = FindChunk(ChunkX, ChunkZ); if ((Chunk == nullptr) || !Chunk->IsValid()) { - return false; + return std::nullopt; } - a_Height = Chunk->GetHeight(a_BlockX, a_BlockZ); - return true; + + return Chunk->GetHeight(a_BlockX, a_BlockZ); } diff --git a/src/ChunkMap.h b/src/ChunkMap.h index 8538b7624..2169acdfd 100644 --- a/src/ChunkMap.h +++ b/src/ChunkMap.h @@ -5,6 +5,8 @@ #pragma once +#include + #include "ChunkDataCallback.h" #include "EffectID.h" #include "FunctionRef.h" @@ -91,10 +93,10 @@ public: bool IsWeatherWetAt(int a_BlockX, int a_BlockZ) const; bool IsWeatherWetAt(Vector3i a_Position) const; - bool IsChunkValid (int a_ChunkX, int a_ChunkZ) const; - bool HasChunkAnyClients (int a_ChunkX, int a_ChunkZ) const; - int GetHeight (int a_BlockX, int a_BlockZ); // Waits for the chunk to get loaded / generated - bool TryGetHeight (int a_BlockX, int a_BlockZ, int & a_Height); // Returns false if chunk not loaded / generated + bool IsChunkValid (int a_ChunkX, int a_ChunkZ) const; + bool HasChunkAnyClients (int a_ChunkX, int a_ChunkZ) const; + + std::optional GetHeight(int a_BlockX, int a_BlockZ); // Returns nullopt if chunk not loaded / generated /** Sets the block at the specified coords to the specified value. The replacement doesn't trigger block updates, nor wake up simulators. diff --git a/src/SpawnPrepare.h b/src/SpawnPrepare.h index 62daf1f84..4f588eca4 100644 --- a/src/SpawnPrepare.h +++ b/src/SpawnPrepare.h @@ -1,6 +1,8 @@ #pragma once +#include "ChunkDef.h" + class cWorld; @@ -17,6 +19,11 @@ public: static void PrepareChunks(cWorld & a_World, int a_SpawnChunkX, int a_SpawnChunkZ, int a_PrepareDistance); + static void PrepareChunks(cWorld & a_World, cChunkCoords a_SpawnChunk, int a_PrepareDistance) + { + PrepareChunks(a_World, a_SpawnChunk.m_ChunkX, a_SpawnChunk.m_ChunkZ, a_PrepareDistance); + } + protected: cWorld & m_World; int m_SpawnChunkX; diff --git a/src/World.cpp b/src/World.cpp index 781d44c3a..035007a1b 100644 --- a/src/World.cpp +++ b/src/World.cpp @@ -711,27 +711,23 @@ void cWorld::GenerateRandomSpawn(int a_MaxSpawnRadius) for (int BiomeCheckIndex = 0; BiomeCheckIndex < BiomeCheckCount; ++BiomeCheckIndex) { EMCSBiome Biome = GetBiomeAt(BiomeOffset.x, BiomeOffset.z); - if ((Biome == EMCSBiome::biOcean) || (Biome == EMCSBiome::biFrozenOcean)) + if ((Biome != EMCSBiome::biOcean) && (Biome != EMCSBiome::biFrozenOcean)) { - BiomeOffset += Vector3d(cChunkDef::Width * 4, 0, 0); - continue; + // Found a usable biome + break; } - // Found a usable biome - // Spawn chunks so we can find a nice spawn. - int ChunkX = 0, ChunkZ = 0; - cChunkDef::BlockToChunk(BiomeOffset.x, BiomeOffset.z, ChunkX, ChunkZ); - cSpawnPrepare::PrepareChunks(*this, ChunkX, ChunkZ, a_MaxSpawnRadius); - break; + BiomeOffset.x += cChunkDef::Width * 4; } // Check 0, 0 first. - int SpawnY = 0; - if (CanSpawnAt(BiomeOffset.x, SpawnY, BiomeOffset.z)) + Vector3i BiomeSpawn = BiomeOffset; + if (CanSpawnAt(BiomeSpawn.x, BiomeSpawn.y, BiomeSpawn.z)) { - SetSpawn(BiomeOffset.x, SpawnY, BiomeOffset.z); - - FLOGINFO("World \"{}\": Generated spawnpoint position at {}", m_WorldName, Vector3i{m_SpawnX, m_SpawnY, m_SpawnZ}); + SetSpawn(BiomeSpawn.x, BiomeSpawn.y, BiomeSpawn.z); + auto ChunkPos = cChunkDef::BlockToChunk(BiomeSpawn); + cSpawnPrepare::PrepareChunks(*this, ChunkPos, a_MaxSpawnRadius); + FLOGINFO("World \"{}\": Generated spawnpoint position at {}", m_WorldName, BiomeSpawn); return; } @@ -755,24 +751,25 @@ void cWorld::GenerateRandomSpawn(int a_MaxSpawnRadius) { for (int SearchGridIndex = 0; SearchGridIndex < PerRadiSearchCount; ++SearchGridIndex) { - const Vector3i PotentialSpawn = BiomeOffset + (ChunkOffset[SearchGridIndex] * RadiusOffset); + auto PotentialSpawn = BiomeOffset + (ChunkOffset[SearchGridIndex] * RadiusOffset); - if (CanSpawnAt(PotentialSpawn.x, SpawnY, PotentialSpawn.z)) + if (CanSpawnAt(PotentialSpawn.x, PotentialSpawn.y, PotentialSpawn.z)) { - SetSpawn(PotentialSpawn.x, SpawnY, PotentialSpawn.z); + SetSpawn(PotentialSpawn.x, PotentialSpawn.y, PotentialSpawn.z); - int ChunkX, ChunkZ; - cChunkDef::BlockToChunk(m_SpawnX, m_SpawnZ, ChunkX, ChunkZ); - cSpawnPrepare::PrepareChunks(*this, ChunkX, ChunkZ, a_MaxSpawnRadius); + auto ChunkPos = cChunkDef::BlockToChunk(PotentialSpawn); + cSpawnPrepare::PrepareChunks(*this, ChunkPos, a_MaxSpawnRadius); - FLOGINFO("World \"{}\":Generated spawnpoint position at {}", m_WorldName, Vector3i{m_SpawnX, m_SpawnY, m_SpawnZ}); + FLOGINFO("World \"{}\":Generated spawnpoint position at {}", m_WorldName, PotentialSpawn); return; } } } - m_SpawnY = GetHeight(m_SpawnX, m_SpawnZ); - FLOGWARNING("World \"{}\": Did not find an acceptable spawnpoint. Generated a random spawnpoint position at {}", m_WorldName, Vector3i{m_SpawnX, m_SpawnY, m_SpawnZ}); + SetSpawn(BiomeSpawn.x, BiomeSpawn.y, BiomeSpawn.z); + auto ChunkPos = cChunkDef::BlockToChunk(BiomeSpawn); + cSpawnPrepare::PrepareChunks(*this, ChunkPos, a_MaxSpawnRadius); + FLOGWARNING("World \"{}\": Did not find an acceptable spawnpoint. Generated a random spawnpoint position at {}", m_WorldName, BiomeSpawn); } @@ -792,11 +789,57 @@ bool cWorld::CanSpawnAt(int a_X, int & a_Y, int a_Z) E_BLOCK_NETHERRACK }; + + class cCanSpawnChunkStay: + public cChunkStay + { + cEvent m_ChunksReady; + public: + + cCanSpawnChunkStay(double a_WorldX, double a_WorldZ) + { + auto Chunk = cChunkDef::BlockToChunk(Vector3d{a_WorldX, 0.0, a_WorldZ}.Floor()); + for (int XOffset = -1; XOffset != 2; ++XOffset) + { + for (int ZOffset = -1; ZOffset != 2; ++ZOffset) + { + Add(Chunk.m_ChunkX + XOffset, Chunk.m_ChunkZ + ZOffset); + } + } + } + + virtual ~cCanSpawnChunkStay() override + { + Disable(); + } + + virtual bool OnAllChunksAvailable() override + { + m_ChunksReady.Set(); + return false; // Keep chunk stay active + } + + virtual void OnChunkAvailable(int, int) override {} + virtual void OnDisabled() override {} + + void Wait() + { + m_ChunksReady.Wait(); + } + }; + + // Use chunk stay to load 3x3 chunk area around a_X, a_Z + cCanSpawnChunkStay ChunkStay(a_X, a_Z); + ChunkStay.Enable(m_ChunkMap); + ChunkStay.Wait(); + static const int ValidSpawnBlocksCount = ARRAYCOUNT(ValidSpawnBlocks); // Increase this by two, because we need two more blocks for body and head - static const int HighestSpawnPoint = GetHeight(a_X, a_Z) + 2; - static const int LowestSpawnPoint = HighestSpawnPoint / 2; + auto Height = GetHeight(static_cast(a_X), static_cast(a_Z)); + ASSERT(Height.has_value()); + static const int HighestSpawnPoint = *Height + 2; + static const int LowestSpawnPoint = static_cast(HighestSpawnPoint / 2.0f); for (int PotentialY = HighestSpawnPoint; PotentialY > LowestSpawnPoint; --PotentialY) { @@ -845,6 +888,8 @@ bool cWorld::CanSpawnAt(int a_X, int & a_Y, int a_Z) return true; } + // Always set a_Y so it can be used when we fail to find any spawn point + a_Y = HighestSpawnPoint - 2; return false; } @@ -2122,7 +2167,7 @@ void cWorld::SendBlockTo(int a_X, int a_Y, int a_Z, const cPlayer & a_Player) -int cWorld::GetHeight(int a_X, int a_Z) +std::optional cWorld::GetHeight(int a_X, int a_Z) { return m_ChunkMap.GetHeight(a_X, a_Z); } @@ -2131,15 +2176,6 @@ int cWorld::GetHeight(int a_X, int a_Z) -bool cWorld::TryGetHeight(int a_BlockX, int a_BlockZ, int & a_Height) -{ - return m_ChunkMap.TryGetHeight(a_BlockX, a_BlockZ, a_Height); -} - - - - - void cWorld::SendBlockEntity(int a_BlockX, int a_BlockY, int a_BlockZ, cClientHandle & a_Client) { m_ChunkMap.SendBlockEntity(a_BlockX, a_BlockY, a_BlockZ, a_Client); diff --git a/src/World.h b/src/World.h index 352bc19f3..6e84393ef 100644 --- a/src/World.h +++ b/src/World.h @@ -1,6 +1,8 @@ #pragma once +#include + #include "Simulator/SimulatorManager.h" #include "ChunkMap.h" #include "WorldStorage/WorldStorage.h" @@ -132,9 +134,6 @@ public: virtual eDimension GetDimension(void) const override { return m_Dimension; } - /** Returns the world height at the specified coords; waits for the chunk to get loaded / generated */ - virtual int GetHeight(int a_BlockX, int a_BlockZ) override; - // tolua_end virtual cTickTime GetTimeOfDay(void) const override; @@ -144,8 +143,8 @@ public: virtual void SetTimeOfDay(cTickTime a_TimeOfDay) override; - /** Retrieves the world height at the specified coords; returns false if chunk not loaded / generated */ - bool TryGetHeight(int a_BlockX, int a_BlockZ, int & a_Height); // Exported in ManualBindings.cpp + /** Retrieves the world height at the specified coords; returns nullopt if chunk not loaded / generated */ + virtual std::optional GetHeight(int a_BlockX, int a_BlockZ) override; // Exported in ManualBindings.cpp // Broadcast respective packets to all clients of the chunk where the event is taking place // Implemented in Broadcaster.cpp @@ -585,6 +584,10 @@ public: int GetSpawnX(void) const { return m_SpawnX; } int GetSpawnY(void) const { return m_SpawnY; } int GetSpawnZ(void) const { return m_SpawnZ; } + Vector3i GetSpawnPos() const + { + return {m_SpawnX, m_SpawnY, m_SpawnZ}; + } /** Wakes up the simulators for the specified block */ virtual void WakeUpSimulators(Vector3i a_Block) override;