refactor: config (#30)

* refactor(config): replace singleton with dependency injection

* refactor(client_player): return ChunkPosSet by value in non-const getter

Copy the internal set under the mutex lock to avoid exposing a mutable reference that could be used unsafely after the lock is released. This fixes a potential data race when the caller holds the returned reference beyond the critical section. Also update the caller in `client_world` to remove the now-unnecessary `std::move`.

* fix(gameplay): prevent use-after-free in pending delete queues during destruction

* refactor(config): separate game and server configuration with explicit paths

include/Cubed/app.hpp

@@ -5,6 +5,7 @@
 #include "Cubed/gameplay/server_world.hpp"
 #define GLFW_INCLUDE_NONE
 #include "Cubed/camera.hpp"
+#include "Cubed/config.hpp"
 #include "Cubed/dev_panel.hpp"
 #include "Cubed/render/renderer.hpp"
 #include "Cubed/texture_manager.hpp"
@@ -49,23 +50,23 @@ public:
     Window& window();
     ClientWorld& client_world();
     ServerWorld& server_world();
+    Config& config();
     const Argument& argument() const;
     AudioEngine& audio();
 
 private:
+    Config m_game_config;
     Camera m_camera;
     TextureManager m_texture_manager;
     NetworkServer m_server;
     std::shared_ptr<NetworkClient> m_client;
+    AudioEngine m_audio;
     ClientWorld m_client_world;
 
-    DevPanel m_dev_panel{*this};
-    Renderer m_renderer{m_camera, m_client_world, m_texture_manager,
-                        m_dev_panel};
+    DevPanel m_dev_panel;
+    Renderer m_renderer;
 
-    Window m_window{m_renderer};
-
-    AudioEngine m_audio;
+    Window m_window;
 
     inline static double last_time = glfwGetTime();
     inline static double current_time = glfwGetTime();
@@ -74,6 +75,7 @@ private:
     inline static int frame_count = 0;
     inline static int fps = 0;
     Argument m_argument;
+
     void init(int argc, char** argv);
     void handle_argument(int argc, char** argv);
     void handle_toml();

include/Cubed/audio/audio_engine.hpp

@@ -4,6 +4,7 @@
 #include "Cubed/audio/audio_source.hpp"
 #include "Cubed/audio/sound_manager.hpp"
 #include "Cubed/audio/source_pool.hpp"
+#include "Cubed/config.hpp"
 
 #include <AL/al.h>
 #include <AL/alc.h>
@@ -17,7 +18,7 @@ class ClientWorld;
 
 class AudioEngine {
 public:
-    AudioEngine();
+    AudioEngine(Config& config);
     AudioEngine(const AudioEngine&) = delete;
     AudioEngine(AudioEngine&&) = delete;
     AudioEngine& operator=(const AudioEngine&) = delete;
@@ -48,6 +49,7 @@ private:
     std::unique_ptr<AudioSource> m_bgm;
     FadeMap m_fade_map;
     SoundManager m_sounds;
+    Config& m_config;
     std::shared_ptr<SourcePool> m_pool;
     bool m_efx_supported = false;
     bool m_underwater = false;

include/Cubed/config.hpp

@@ -1,99 +1,75 @@
 #pragma once
-#include "Cubed/tools/cubed_assert.hpp"
 #include "Cubed/tools/toml.utils.hpp"
 
 namespace Cubed {
 
 class Config {
 public:
-    Config();
+    explicit Config(std::string_view path);
+    Config(const Config&) = delete;
+    Config(Config&&) = delete;
+    Config& operator=(const Config&) = delete;
+    Config& operator=(Config&&) = delete;
     ~Config();
 
-    static Config& get();
-
     toml::table& table();
 
-    void load_or_create_config();
+    void load_config();
     void save_to_file();
 
-    template <TOML::TomlValueType T> T get(std::string_view key) const {
-        size_t cur = 0;
-        auto pos = key.find('.');
-        const toml::table* table = &m_tbl;
-        while (pos != std::string_view::npos) {
-            std::string_view s = key.substr(cur, pos - cur);
-            if (s.empty()) {
-                Logger::error("Empty key/table name in path '{}'", key);
-                ASSERT(false);
-                std::abort();
-            }
-            cur = pos + 1;
-            pos = key.find('.', cur);
-            if (auto* next = (*table)[s].as_table()) {
-                table = next;
-            } else {
-                Logger::error("Can't find table {}", s);
-                ASSERT(false);
-                std::abort();
-            }
+    template <TOML::TomlValueType T>
+    T get(std::string_view key, T default_value) {
+        if (auto* node = find_node(m_tbl, key)) {
+            if (auto value = node->value<T>())
+                return *value;
         }
-        std::string_view n_key = key.substr(cur);
-        if (n_key.empty()) {
-            Logger::error("Trailing dot in path '{}'", key);
-            ASSERT(false);
-            std::abort();
-        }
-        auto opt = (*table)[n_key].value<T>();
-        if (opt) {
-            return *opt;
+
+        set(key, default_value);
+        save_to_file();
+
+        return default_value;
+    }
+
+    template <TOML::TomlValueType T> void set(std::string_view key, T&& value) {
+        auto pos = key.rfind('.');
+
+        toml::table* table;
+        std::string_view name;
+
+        if (pos == std::string_view::npos) {
+            table = &m_tbl;
+            name = key;
         } else {
-            Logger::error("Can't find key {}", n_key);
-            ASSERT(false);
-            std::abort();
+            table = find_or_create_table(key.substr(0, pos));
+            name = key.substr(pos + 1);
         }
+        // Insert node at the last level's table
+        table->insert_or_assign(name, std::forward<T>(value));
     }
-    template <typename T> void set(std::string_view key, T&& val) {
-        if constexpr (!TOML::TomlValueType<std::decay_t<T>>) {
-            static_assert(false, "Type Not Support");
-        }
-        size_t cur = 0;
-        auto pos = key.find('.');
-        toml::table* table = &m_tbl;
-        while (pos != std::string_view::npos) {
-            std::string_view s = key.substr(cur, pos - cur);
-            if (s.empty()) {
-                Logger::error("Empty key/table name in path '{}'", key);
-                ASSERT(false);
-                std::abort();
-            }
-            cur = pos + 1;
-            pos = key.find('.', cur);
-            if (auto* next = (*table)[s].as_table()) {
-                table = next;
-            } else {
-                auto [it, inserted] = table->insert_or_assign(s, toml::table{});
-                table = it->second.as_table();
-            }
-        }
-        std::string_view n_key = key.substr(cur);
-        if (n_key.empty()) {
-            Logger::error("Trailing dot in path '{}'", key);
-            ASSERT(false);
-            std::abort();
-        }
-        table->insert_or_assign(n_key, std::forward<T>(val));
-    }
+
     template <typename T> void set_and_save(std::string_view key, T&& val) {
         set(key, std::forward(val));
         save_to_file();
     }
-    toml::node_view<toml::node> val_view(std::string_view key);
 
 private:
     toml::table m_tbl;
-    constexpr static inline std::string_view CONGIF_PATH =
-        ASSETS_PATH "config.toml";
-    void create_config();
+    const std::string CONGIF_PATH;
+    const toml::node* find_node(const toml::table& root,
+                                std::string_view path) const;
+    // Follow the path to find the last-level toml::table, creating it if it
+    // does not exist
+    toml::table* find_or_create_table(std::string_view path);
 };
 
+template <>
+inline float Config::get(std::string_view key, float default_value) {
+    return static_cast<float>(
+        Config::get<double>(key, static_cast<double>(default_value)));
+}
+
+template <> inline void Config::set(std::string_view key, float&& value) {
+    Config::set<double>(key, static_cast<double>(value));
+}
+
 } // namespace Cubed

include/Cubed/dev_panel.hpp

@@ -1,5 +1,7 @@
 #pragma once
 
+#include "Cubed/config.hpp"
+
 #include <toml++/toml.hpp>
 
 namespace Cubed {
@@ -36,7 +38,8 @@ public:
 
 private:
     App& m_app;
-    ConfigView m_config;
+    Config& m_config;
+    ConfigView m_config_view;
     ClientPlayer* m_player;
     PlayerProfile m_player_profile;
     bool m_need_save_config = false;

include/Cubed/gameplay/client_player.hpp

@@ -25,7 +25,7 @@ public:
 
     void update_chunk_set(const ChunkPosSet& set);
     const ChunkPosSet& get_chunk_pos_set() const;
-    ChunkPosSet& get_chunk_pos_set();
+    ChunkPosSet get_chunk_pos_set();
 
     static AABB get_aabb(const glm::vec3& pos);
     const glm::vec3& get_front() const;

include/Cubed/gameplay/client_world.hpp

@@ -1,5 +1,6 @@
 #pragma once
 #include "Cubed/audio/audio_engine.hpp"
+#include "Cubed/config.hpp"
 #include "Cubed/gameplay/block.hpp"
 #include "Cubed/gameplay/chunk_pos.hpp"
 #include "Cubed/gameplay/client_chunk.hpp"
@@ -43,7 +44,7 @@ struct PlayerRenderData {
 
 class ClientWorld {
 public:
-    ClientWorld(AudioEngine& auido);
+    ClientWorld(AudioEngine& auido, Config& config);
     ~ClientWorld();
     void init(std::string_view player_name,
               std::shared_ptr<NetworkClient> client);
@@ -95,6 +96,7 @@ public:
     int chunk_size() const;
     static AABB get_block_aabb(const glm::ivec3& pos);
     AudioEngine& get_audio();
+    Config& get_config();
     template <typename Fn>
     void register_ticktimer(std::string_view id, TickType threshold, Fn&& f) {
         m_ticktimers.emplace(
@@ -103,6 +105,12 @@ public:
     }
 
 private:
+    std::atomic<bool> m_is_pending_delete_queue_free{false};
+    std::mutex m_delete_vbo_mutex;
+    std::mutex m_delete_vao_mutex;
+    std::vector<std::unique_ptr<VertexBuffer>> m_pending_delete_vbo;
+    std::vector<std::unique_ptr<VertexArray>> m_pending_delete_vao;
+
     enum class ChunkLoadStyle { RANDOM, CENTER };
     using ChunkHashMap =
         tbb::concurrent_hash_map<ChunkPos, std::shared_ptr<ClientChunk>,
@@ -124,18 +132,15 @@ private:
     OtherPlayerHashMap m_player_info;
     ChunkHashMap m_chunks;
     AudioEngine& m_audio;
+    Config& m_config;
     std::vector<glm::vec4> m_planes;
     std::jthread m_client_thread;
 
-    std::mutex m_delete_vbo_mutex;
-    std::mutex m_delete_vao_mutex;
     mutable std::shared_mutex m_player_info_mutex;
 
     tbb::concurrent_queue<std::unique_ptr<ClientChunk>> m_pending_upload_queue;
     tbb::concurrent_queue<ChunkPos> m_dirty_chunk_queue;
     tbb::concurrent_queue<PendingSound> m_pending_sound;
-    std::vector<std::unique_ptr<VertexBuffer>> m_pending_delete_vbo;
-    std::vector<std::unique_ptr<VertexArray>> m_pending_delete_vao;
 
     std::deque<ChunkPos> m_dirty_queue;
     std::vector<const ChunkRenderSnapshot*> m_render_snapshots;

include/Cubed/gameplay/network_server.hpp

@@ -1,4 +1,5 @@
 #pragma once
+#include "Cubed/config.hpp"
 #include "Cubed/gameplay/server_world.hpp"
 #include "Cubed/gameplay/session.hpp"
 
@@ -8,17 +9,19 @@ namespace Cubed {
 
 class NetworkServer {
 public:
-    NetworkServer(int port = 25530);
+    explicit NetworkServer();
     ~NetworkServer();
     void stop();
 
     // Run in another thread after initialization is complete
-    void start_server(int port = 25530);
-
+    void start_server(int port);
+    void start_server();
     int port() const;
     ServerWorld& server_world();
 
 private:
+    Config m_config;
+
     asio::io_context m_io;
     std::thread m_net_thread;
     int m_port = 25530;

include/Cubed/gameplay/server_world.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "Cubed/config.hpp"
 #include "Cubed/gameplay/cave_carver.hpp"
 #include "Cubed/gameplay/chunk_pos.hpp"
 #include "Cubed/gameplay/game_time.hpp"
@@ -25,7 +26,7 @@ class Session;
 class ServerWorld {
 public:
     enum class ThreadPoolKind { NET, GEN };
-    ServerWorld();
+    ServerWorld(Config& config);
     ~ServerWorld();
     void stop();
     void handle_player_exit(const std::string& uuid);
@@ -118,6 +119,9 @@ private:
 
     using uuid_acc = PlayerUUIDMap::accessor;
     using uuid_cacc = PlayerUUIDMap::const_accessor;
+
+    Config& m_config;
+
     // key = uuid
     PlayerHashMap m_players;
     ChunkHashMap m_chunks;

include/Cubed/map_table.hpp

@@ -1,26 +0,0 @@
-#pragma once
-// #include <string>
-// #include <unordered_map>
-// #include <vector>
-namespace Cubed {
-
-class MapTable {
-private:
-    /*
-        static inline std::unordered_map<unsigned, std::string> id_to_name_map;
-        static inline std::unordered_map<size_t, unsigned> name_to_id_map;
-        static inline std::vector<std::string> item_id_to_name;
-    */
-public:
-    // please using reference
-    /*
-    static std::string_view get_name_from_id(unsigned id);
-    static unsigned get_id_from_name(const std::string& name);
-
-    static std::string_view item_name(unsigned id);
-    static const std::vector<std::string>& item_map();
-    */
-    static void init_map();
-};
-
-} // namespace Cubed

include/Cubed/render/renderer.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "Cubed/config.hpp"
 #include "Cubed/constants.hpp"
 #include "Cubed/primitive_data.hpp"
 #include "Cubed/render/player_renderer.hpp"
@@ -23,7 +24,8 @@ public:
     constexpr static int NUM_VAO = 7;
 
     Renderer(const Camera& camera, ClientWorld& world,
-             const TextureManager& texture_manager, DevPanel& dev_panel);
+             const TextureManager& texture_manager, DevPanel& dev_panel,
+             Config& config);
     ~Renderer();
     void hot_reload();
     void init(bool debug_on);
@@ -109,7 +111,7 @@ private:
     std::vector<Vertex2D> m_ui;
 
     WorldRenderer m_world_renderer;
-
+    Config& m_config;
     void init_quad();
     void init_text();
 

include/Cubed/texture_manager.hpp

@@ -1,4 +1,5 @@
 #pragma once
+#include "Cubed/config.hpp"
 #include "Cubed/gameplay/block.hpp"
 #include "Cubed/render/texture.hpp"
 
@@ -19,7 +20,7 @@ private:
     std::unique_ptr<Texture> m_skin;
     std::vector<std::unique_ptr<Texture>> m_item_textures;
     GLfloat m_max_aniso = 0.0f;
-
+    Config& m_config;
     int m_aniso = 1;
 
     void load_block_status(unsigned status_id);
@@ -36,7 +37,7 @@ private:
     void hot_reload();
 
 public:
-    TextureManager();
+    TextureManager(Config& config);
     ~TextureManager();
 
     void delete_texture();

include/Cubed/tools/toml.utils.hpp

@@ -14,6 +14,7 @@ concept TomlValueType =
     std::same_as<std::decay_t<T>, toml::date> ||
     std::same_as<std::decay_t<T>, toml::time> ||
     std::same_as<std::decay_t<T>, toml::date_time> ||
+    std::same_as<std::decay_t<T>, float> ||
     std::same_as<std::decay_t<T>, std::string>;
 
 template <TomlValueType T>

include/Cubed/window.hpp

@@ -1,4 +1,7 @@
 #pragma once
+
+#include "Cubed/config.hpp"
+
 #define GLFW_INCLUDE_NONE
 #include <GLFW/glfw3.h>
 namespace Cubed {
@@ -6,7 +9,7 @@ namespace Cubed {
 class Renderer;
 class Window {
 public:
-    Window(Renderer& renderer);
+    Window(Renderer& renderer, Config& config);
     ~Window();
 
     bool is_mouse_enable() const;
@@ -29,6 +32,7 @@ private:
     int m_width;
     int m_height;
     Renderer& m_renderer;
+    Config& m_config;
 };
 
 } // namespace Cubed