Merge pull request mmbednarek#48 from mmbednarek/chunk-fixes

Chunk light fixes

library/container/include/minecpp/container/Queue.h

@@ -1,7 +1,7 @@
 #pragma once
-#include <mutex>
 #include <optional>
 #include <queue>
+#include <shared_mutex>
 
 namespace minecpp::container {
 
@@ -12,22 +12,22 @@ class Queue
    template<typename TPushItem>
    void push(TPushItem &&item)
    {
-      std::lock_guard lock{m_mutex};
+      std::unique_lock lock{m_mutex};
       m_queue.push(std::forward<TPushItem>(item));
    }
 
    template<typename... TArgs>
    void emplace(TArgs &&...args)
    {
-      std::lock_guard lock{m_mutex};
+      std::unique_lock lock{m_mutex};
       m_queue.template emplace(std::forward<TArgs>(args)...);
    }
 
    [[nodiscard]] TValue pop()
    {
       TValue result;
       {
-         std::lock_guard lock{m_mutex};
+         std::unique_lock lock{m_mutex};
          result = std::move(m_queue.front());
          m_queue.pop();
       }
@@ -36,7 +36,7 @@ class Queue
 
    [[nodiscard]] std::optional<TValue> try_pop()
    {
-      std::lock_guard lock{m_mutex};
+      std::unique_lock lock{m_mutex};
       if (m_queue.empty()) {
          return std::nullopt;
       }
@@ -48,17 +48,13 @@ class Queue
 
    [[nodiscard]] bool is_empty() const
    {
-      bool empty;
-      {
-         std::lock_guard lock{m_mutex};
-         empty = m_queue.empty();
-      }
-      return empty;
+      std::shared_lock lock{m_mutex};
+      return m_queue.empty();
    }
 
  private:
    std::queue<TValue> m_queue;
-   mutable std::mutex m_mutex;
+   mutable std::shared_mutex m_mutex;
 };
 
 }// namespace minecpp::container

library/world/include/minecpp/world/Chunk.h

@@ -99,6 +99,7 @@ class Chunk : public game::IBlockContainer
 
  private:
    mb::result<Section &> section_from_y_level(int y);
+   mb::result<Section &> assure_section_at_y_level(int y);
 
    game::ChunkPosition m_position{};
    bool m_is_full = false;

library/world/src/Chunk.cpp

@@ -41,19 +41,14 @@ void Chunk::create_empty_section(int8_t sec)
 
 mb::emptyres Chunk::set_block_at(const game::BlockPosition &position, game::BlockStateId state)
 {
-   auto sec  = static_cast<std::int8_t>(position.y() / 16);
-   auto iter = m_sections.find(sec);
-   if (iter == m_sections.end()) {
-      create_empty_section(sec);
-      iter = m_sections.find(sec);
-   }
+   auto section = this->assure_section_at_y_level(position.y());
+   MB_VERIFY(section);
 
-   auto &section = iter->second;
-   section.set_block(position, state);
+   section->set_block(position, state);
 
    BlockState block_state{state};
    auto block = repository::Block::the().get_by_id(block_state.block_id());
-   MB_VERIFY(block)
+   MB_VERIFY(block);
 
    if (block_state.solid_faces() & game::FaceMask::Top ||
        block_state.solid_faces() & game::FaceMask::Bottom || block_state.opacity() > 0) {
@@ -89,10 +84,8 @@ mb::result<game::LightValue> Chunk::light_value_at(game::LightType type, const g
 mb::emptyres Chunk::set_light_value_at(game::LightType type, const game::BlockPosition &position,
                                        game::LightValue value)
 {
-   auto section = section_from_y_level(position.y());
-   if (section.has_failed()) {
-      return std::move(section.err());
-   }
+   auto section = this->assure_section_at_y_level(position.y());
+   MB_VERIFY(section);
 
    section->set_light(type, position, value);
    return mb::ok;
@@ -155,6 +148,17 @@ mb::result<Section &> Chunk::section_from_y_level(int y)
    return iter->second;
 }
 
+mb::result<Section &> Chunk::assure_section_at_y_level(int y)
+{
+   auto sec  = static_cast<mb::i8>(y / 16);
+   auto iter = m_sections.find(sec);
+   if (iter == m_sections.end()) {
+      create_empty_section(sec);
+      iter = m_sections.find(sec);
+   }
+   return iter->second;
+}
+
 int Chunk::height_at(game::HeightType type, game::BlockPosition position) const
 {
    return heightmap_of(type).at(16 * position.offset_z() + position.offset_x());
@@ -248,6 +252,9 @@ void Chunk::read_net_chunk(const net::Chunk &chunk)
 
    std::int8_t section_y = 0;
    for (const auto &section : data.sections) {
+      if (section.block_count == 0)
+         continue;
+
       this->put_section(section_y, world::Section::from_net(section, section_y));
       ++section_y;
    }

library/world/test/src/SectionTest.cpp

@@ -1,5 +1,6 @@
 #include <gtest/gtest.h>
 
+#include "minecpp/repository/Repository.h"
 #include "minecpp/world/Chunk.h"
 #include "minecpp/world/ChunkSerializer.h"
 #include "minecpp/world/Section.h"
@@ -8,7 +9,22 @@ using minecpp::game::BlockStateId;
 using minecpp::world::Chunk;
 using minecpp::world::Section;
 
-TEST(Section, ProtoTest)
+class SectionTest : public ::testing::Test
+{
+ protected:
+   void SetUp() override
+   {
+      static bool g_initialised_repository = false;
+      if (not g_initialised_repository) {
+         ASSERT_TRUE(minecpp::repository::load_repository_from_file("repository.bin").ok());
+         g_initialised_repository = true;
+      }
+   }
+
+   void TearDown() override {}
+};
+
+TEST_F(SectionTest, ProtoTest)
 {
    std::vector<BlockStateId> ids;
    ids.resize(4096);
@@ -18,32 +34,55 @@ TEST(Section, ProtoTest)
 
    std::array<short, 256> heightmap{};
    Chunk chunk(0, 0, heightmap);
-   chunk.create_empty_section(0);
 
-   chunk.set_block_at({5, 5, 5}, 1);
-   chunk.set_block_at({5, 5, 6}, 1);
+   int count = 0;
+
+   for (int y = 0; y < 80; ++y) {
+      int section = y / 16;
+      for (int x = 0; x < 16; ++x) {
+         for (int z = 0; z < 16; ++z) {
+            if (y < 64) {
+               int block_id = (section == 0) ? 1 : (count % (1 << (section + 1)));
+               ASSERT_TRUE(chunk.set_block_at({x, y, z}, static_cast<BlockStateId>(block_id)).ok());
+            } else {
+               ASSERT_TRUE(chunk.set_light_value_at(minecpp::game::LightType::Sky, {x, y, z}, 16).ok());
+            }
+            ++count;
+         }
+      }
+   }
+
+   ASSERT_EQ(chunk.section_count(), 5);
 
    minecpp::world::ChunkSerializer chunk_serializer{chunk};
 
    minecpp::net::Chunk out_chunk;
    chunk_serializer.write_chunk(out_chunk);
 
-   Section section{1};
-   section.fill_light(minecpp::game::LightType::Block);
-   section.fill_light(minecpp::game::LightType::Sky);
-   section.data() = {ids.begin(), ids.end()};
-
-   //   ASSERT_EQ(proto.sky_light().size(), 2048);
-   //
-   //   auto decoded_section = Section::from_proto(proto);
-   //
-   //   ASSERT_EQ(decoded_section.data().palette().size(), section.data().palette().size());
-   //   for (std::size_t i{0}; i < decoded_section.data().palette().size(); ++i) {
-   //      ASSERT_EQ(decoded_section.data().palette()[i], section.data().palette()[i]);
-   //   }
-   //
-   //   ASSERT_EQ(decoded_section.data().size(), 4096);
-   //   for (std::size_t i{0}; i < 4096; ++i) {
-   //      ASSERT_EQ(decoded_section.data()[i], section.data()[i]);
-   //   }
+   Chunk chunk2(0, 0, heightmap);
+   chunk2.read_net_chunk(out_chunk);
+
+   ASSERT_EQ(chunk2.section_count(), 5);
+
+   for (int y = 0; y < 80; ++y) {
+      for (int x = 0; x < 16; ++x) {
+         for (int z = 0; z < 16; ++z) {
+            auto block_id = chunk.block_at({x, y, z});
+            ASSERT_TRUE(block_id.ok());
+
+            auto block_id2 = chunk2.block_at({x, y, z});
+            ASSERT_TRUE(block_id2.ok());
+
+            ASSERT_EQ(*block_id, *block_id2);
+
+            auto block_val = chunk.light_value_at(minecpp::game::LightType::Sky, {x, y, z});
+            ASSERT_TRUE(block_val.ok());
+
+            auto block_val2 = chunk2.light_value_at(minecpp::game::LightType::Sky, {x, y, z});
+            ASSERT_TRUE(block_val2.ok());
+
+            ASSERT_EQ(*block_val, *block_val2);
+         }
+      }
+   }
 }

service/engine/src/StorageResponseHandler.cpp

@@ -17,6 +17,8 @@ void StorageResponseHandler::set_chunk_system(ChunkSystem *chunk_system)
 
 void StorageResponseHandler::on_reply_empty_chunk(int a, const net::storage::cb::ReplyEmptyChunk &message)
 {
+   spdlog::info("storage-response-handler: handling empty chunk at {} {}", message.position.x(),
+                message.position.y());
    m_job_system.run([this, message]() {
       assert(m_chunk_system);
       m_chunk_system->handle_empty_chunk(game::ChunkPosition(message.position));
@@ -25,6 +27,8 @@ void StorageResponseHandler::on_reply_empty_chunk(int a, const net::storage::cb:
 
 void StorageResponseHandler::on_reply_chunk(int a, const net::storage::cb::ReplyChunk &message)
 {
+   spdlog::info("storage-response-handler: handling chunk data at {} {}", message.chunk.position.x(),
+                message.chunk.position.y());
    m_job_system.run([this, message]() {
       assert(m_chunk_system);
       m_chunk_system->handle_chunk_data(message.chunk);

service/engine/src/World.cpp

@@ -12,6 +12,7 @@
 #include <minecpp/util/Uuid.h>
 #include <minecpp/world/BlockState.h>
 #include <minecpp/world/SectionSlice.h>
+#include <spdlog/spdlog.h>
 #include <utility>
 
 using minecpp::game::Face;
@@ -187,6 +188,7 @@ mb::emptyres World::recalculate_light(game::LightType /*light_type*/, const game
 mb::emptyres World::send_chunk_to_player(game::PlayerId player_id, const game::ChunkPosition &position,
                                          bool is_initial)
 {
+   spdlog::info("world: issued chunk dispatch {} {}", position.x(), position.z());
    ACCESS_CHUNK_AT(position, [this, player_id, is_initial](world::Chunk *chunk) {
       m_dispatcher.send_chunk(player_id, chunk, is_initial);
    });