Add world editor with unified entity pipeline - #39
Conversation
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces an editor application for visually editing the MMO world, allowing developers to view and potentially manipulate game entities loaded from configuration files.
Changes:
- Added new editor application with free-flying camera controls (WASD + mouse look)
- Implemented entity loading and rendering for buildings and town NPCs from game configuration
- Integrated editor into build system with dependency on nlohmann/json library
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/editor/main.cpp | Entry point for the editor application with basic initialization and error handling |
| src/editor/editor_camera.hpp | Header defining free-flying editor camera with WASD movement and mouse look controls |
| src/editor/editor_camera.cpp | Implementation of camera movement, rotation, and view/projection matrix generation |
| src/editor/editor_application.hpp | Header for editor application class extending engine::Application |
| src/editor/editor_application.cpp | Main editor implementation including entity loading, rendering, and input handling |
| src/editor/CMakeLists.txt | Build configuration for editor executable |
| CMakeLists.txt | Added nlohmann/json dependency and editor subdirectory to root build |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #include "protocol/protocol.hpp" | ||
| #include "protocol/heightmap.hpp" | ||
| #include "server/heightmap_generator.hpp" | ||
| #include "engine/model_loader.hpp" |
There was a problem hiding this comment.
The include "engine/model_loader.hpp" is not used in this file and can be removed.
| registry_.emplace<Transform>(entity, world_x, world_y, world_z, | ||
| glm::radians(config.rotation)); |
There was a problem hiding this comment.
The Transform struct is an aggregate type without a constructor. While C++20 allows aggregate initialization with parentheses, this pattern is inconsistent with how Transform is constructed elsewhere in the codebase (e.g., src/client/game.cpp:684 uses default construction). Consider following the established pattern: create with default constructor, then assign fields, or use brace initialization.
| registry_.emplace<EntityInfo>(entity, info); | ||
|
|
||
| // Name | ||
| registry_.emplace<Name>(entity, config.name); |
There was a problem hiding this comment.
The Name struct is an aggregate type without a constructor. While C++20 allows aggregate initialization with parentheses, this pattern is inconsistent with how Name is constructed elsewhere in the codebase (e.g., src/client/game.cpp:688 uses default construction then assigns the value field). Consider following the established pattern for consistency.
| registry_.emplace<EntityInfo>(entity, info); | ||
|
|
||
| // Name | ||
| registry_.emplace<Name>(entity, config.name); |
There was a problem hiding this comment.
The Name struct is an aggregate type without a constructor. While C++20 allows aggregate initialization with parentheses, this pattern is inconsistent with how Name is constructed elsewhere in the codebase (e.g., src/client/game.cpp:688 uses default construction then assigns the value field). Consider following the established pattern for consistency.
| #include "editor_application.hpp" | ||
| #include <iostream> | ||
|
|
||
| int main(int argc, char* argv[]) { |
There was a problem hiding this comment.
The parameters argc and argv are unused. Consider adding (void)argc; (void)argv; to suppress compiler warnings, or remove the parameter names if they're not needed (e.g., int main(int, char*[])).
| int main(int argc, char* argv[]) { | |
| int main(int, char*[]) { |
| float world_z = config.y + config_.world().height / 2.0f; | ||
| float world_y = get_terrain_height(world_x, world_z); | ||
|
|
||
| registry_.emplace<Transform>(entity, world_x, world_y, world_z, 0.0f); |
There was a problem hiding this comment.
The Transform struct is an aggregate type without a constructor. While C++20 allows aggregate initialization with parentheses, this pattern is inconsistent with how Transform is constructed elsewhere in the codebase (e.g., src/client/game.cpp:684 uses default construction). Consider following the established pattern: create with default constructor, then assign fields, or use brace initialization.
|
|
||
| // Mouse look using relative mouse mode | ||
| float mouse_dx = 0.0f, mouse_dy = 0.0f; | ||
| SDL_GetRelativeMouseState(&mouse_dx, &mouse_dy); |
There was a problem hiding this comment.
SDL_GetRelativeMouseState is called but SDL relative mouse mode is never enabled. You should call SDL_SetRelativeMouseMode(true) during initialization (e.g., in on_init) to enable mouse capture for camera control. Without this, mouse look will not work correctly.
| target_link_libraries(mmo_editor PRIVATE | ||
| mmo_engine | ||
| mmo_protocol | ||
| mmo_server_lib |
There was a problem hiding this comment.
The target 'mmo_server_lib' does not exist. The server CMakeLists.txt (src/server/CMakeLists.txt) only creates an executable 'mmo_server', not a library. Either create a separate library target from the server code (excluding main.cpp), or remove this dependency. The editor needs access to server code like GameConfig and heightmap_generator, so a library target is the correct solution.
| mmo_server_lib |
| # nlohmann/json (JSON parsing library) | ||
| FetchContent_Declare( | ||
| json | ||
| GIT_REPOSITORY https://github.com/nlohmann/json.git | ||
| GIT_TAG v3.11.3 | ||
| ) | ||
| FetchContent_MakeAvailable(json) | ||
|
|
There was a problem hiding this comment.
This dependency declaration duplicates the one in src/server/CMakeLists.txt (lines 25-30). If mmo_server_lib is created as a library target, the nlohmann_json dependency should be transitively available and this duplication would be unnecessary. If the editor needs to use nlohmann_json directly (not through server code), this duplication could be avoided by declaring nlohmann_json once at the root level and having both server and editor link to it.
| # nlohmann/json (JSON parsing library) | |
| FetchContent_Declare( | |
| json | |
| GIT_REPOSITORY https://github.com/nlohmann/json.git | |
| GIT_TAG v3.11.3 | |
| ) | |
| FetchContent_MakeAvailable(json) |
| json | ||
| GIT_REPOSITORY https://github.com/nlohmann/json.git | ||
| GIT_TAG v3.11.3 |
There was a problem hiding this comment.
The FetchContent_Declare for nlohmann/json pulls source code directly from a third-party Git repository and pins it only to a Git tag (GIT_TAG v3.11.3), which is a mutable reference and can be retargeted, enabling a supply-chain compromise if the upstream repo or tag is hijacked. An attacker who controls that tag could inject malicious code into your build and gain the same privileges as the built binaries. To reduce this risk, pin the dependency to an immutable identifier (such as a specific commit SHA or verified archive hash) or vendor the library internally so that build inputs cannot be silently replaced upstream.
Editor is now the single source of truth for world data. The server loads entities and terrain from editor save files instead of running its own procedural generation. - Add editor tools: select, terrain brush, object placement with ImGui UI, raycasting, and world save/load (JSON + binary heightmap) - Add procedural generation bake to editor (town, environment, monsters) with click-to-place center, tunable parameters, and undo support - Add reverse-lookup functions in entity_config.hpp (model → enum) - Server spawn_from_world_data() reads editor save, replaces hardcoded spawn_town/spawn_environment/spawn_npcs - Server loads editor heightmap.bin, no procedural fallback - Engine: add ImGui integration, editor CMake target, input handler improvements, scene renderer line/circle drawing Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
environment.json was loaded by GameConfig but never consumed — the editor and server both use entity_config.hpp enums directly. Removed the file, load_environment(), get_env_type(), EnvironmentTypeConfig, and related members. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ImGui is only used by the editor, not engine/client/server. Also fix inverted horizontal camera rotation in editor. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
find_package imported targets are directory-scoped, so SDL3::SDL3 created in src/engine/ is not visible in src/editor/. When using system SDL (CI), call find_package again in the editor directory. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
mmo_editor) with ImGui UI, terrain brush, entity select/placement tools, and procedural generation bake (town, environment, monsters)data/editor_save/instead of running hardcoded procedural generationentity_config.hppso the server can reconstruct ECS components from saved model namesenvironment.jsonand its GameConfig loading codeTest plan
cmake --build buildcompiles all three targets (client, server, editor)mmo_editor, place generation center, Generate All, Ctrl+S savesdata/editor_save/world_entities.json+heightmap.bin🤖 Generated with Claude Code