diff --git a/.clang-format b/.clang-format index 682be41f..1f2859f6 100644 --- a/.clang-format +++ b/.clang-format @@ -5,7 +5,7 @@ # # Note: clang-format can reflow and align comments, but it cannot enforce # documentation *content* (e.g. forbidding @brief). Use -# scripts/check_comment_style.py for that — see docs/CodingStyle.md. +# scripts/check_comment_style.py for that — see Skills/coding-style.md. BasedOnStyle: LLVM Language: Cpp diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ebb1221f..204009c8 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -164,7 +164,7 @@ jobs: mkdir -p Logs ./build/FlingTests/bin/FlingTests.exe - # Doc-comment conventions for first-party sources (see docs/CodingStyle.md / issue #166). + # Doc-comment conventions for first-party sources (see Skills/coding-style.md / issue #166). # clang-format cannot enforce comment *content*; this script does. comment-style: name: Comment style diff --git a/AGENTS.md b/AGENTS.md index 454ebd54..0f51c574 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,21 +30,19 @@ platform setup (Vulkan SDK, GLFW, etc.). ## Coding style -- Canonical style doc: [`docs/CodingStyle.md`](docs/CodingStyle.md) — documentation +- Style doc: [`Skills/coding-style.md`](Skills/coding-style.md) — documentation comment conventions (no `@brief`, no `/*!`), enforced by `python3 scripts/check_comment_style.py`. - Layout/formatting is enforced by `.clang-format` at the repo root (run `clang-format`, not manual formatting judgment). - Applies to first-party code only (`FlingEngine/`, `Sandbox/`, `FlingTests/`). **Never edit code under `external/`** — it's vendored third-party code. -- AI-assistant-focused summary and common pitfalls: [`Skills/coding-style.md`](Skills/coding-style.md). ## Architecture / module layout -- Current folder layout and the in-progress module split (Core, Graphics, Gameplay, - Resources, Editor) are documented in [`docs/BuildModules.md`](docs/BuildModules.md) — - read it before restructuring includes or CMake targets, it records locked decisions. -- Orientation for where things live today: [`Skills/architecture.md`](Skills/architecture.md). +- Orientation for where things live today, and the in-progress module split + (Core, Graphics, Gameplay, Resources, Editor): [`Skills/architecture.md`](Skills/architecture.md) — + read it before restructuring includes or CMake targets. - Adding a new CMake module (folder + `fling_add_module`, with a UI example): [`Skills/adding-modules.md`](Skills/adding-modules.md). diff --git a/README.md b/README.md index b1a2e9a8..421836bb 100644 --- a/README.md +++ b/README.md @@ -118,7 +118,7 @@ Notice the `-DDEFINE_SHIPPING` option is set to `ON`. This sets a definiton that #endif ``` -See [docs/CodingStyle.md](docs/CodingStyle.md) for documentation-comment conventions and +See [Skills/coding-style.md](Skills/coding-style.md) for documentation-comment conventions and how to run the comment-style check / clang-format. ## AI coding assistants diff --git a/Skills/README.md b/Skills/README.md index 606bead9..1e182fd7 100644 --- a/Skills/README.md +++ b/Skills/README.md @@ -11,7 +11,3 @@ detail on a given topic. These files are meant to be read on demand, not all at | [`coding-style.md`](coding-style.md) | Doc-comment conventions, formatting, what not to touch | | [`architecture.md`](architecture.md) | Current folder/module layout, where new code belongs | | [`adding-modules.md`](adding-modules.md) | How to add a new engine module (`fling_add_module`, UI example) | - -These are supplementary to, not a replacement for, the canonical docs they -reference (`docs/CodingStyle.md`, `docs/BuildModules.md`) — when in doubt, the -canonical doc wins. diff --git a/Skills/adding-modules.md b/Skills/adding-modules.md index 0d988d21..010dbce7 100644 --- a/Skills/adding-modules.md +++ b/Skills/adding-modules.md @@ -1,8 +1,9 @@ # Adding an engine module -Canonical design and locked decisions: [`docs/BuildModules.md`](../docs/BuildModules.md). -This file is the how-to. Do not invent a second pattern (extra globs in the root -CMakeLists, a hand-rolled `*_API` header, or edits under `external/`). +This file is the how-to for the in-progress module split described in +[`architecture.md`](architecture.md). Do not invent a second pattern (extra +globs in the root CMakeLists, a hand-rolled `*_API` header, or edits under +`external/`). `fling_add_module()` lives in [`CMake/FlingModule.cmake`](../CMake/FlingModule.cmake). Until `FlingEngine/CMakeLists.txt` `add_subdirectory`s each module, the engine is diff --git a/Skills/architecture.md b/Skills/architecture.md index c312b6f7..97b318b9 100644 --- a/Skills/architecture.md +++ b/Skills/architecture.md @@ -1,10 +1,10 @@ # Architecture / where things live -This is orientation for today's layout, not a design doc. For the in-progress -module split (Core/Resources/Gameplay/Graphics/Editor as separate CMake targets) -and its locked decisions, read [`docs/BuildModules.md`](../docs/BuildModules.md) -before touching CMake targets, `FLING_ENGINE_INC()`, or cross-folder includes — -several things that look like cleanups are explicitly deferred or ruled out there. +This is orientation for today's layout, not a design doc. The engine is +mid-way through an in-progress module split (Core/Resources/Gameplay/Graphics/Editor +as separate CMake targets) — the rules below hold regardless of how far that +split has progressed. Read [`adding-modules.md`](adding-modules.md) before +touching CMake targets, `FLING_ENGINE_INC()`, or cross-folder includes. ## Today @@ -16,13 +16,12 @@ several things that look like cleanups are explicitly deferred or ruled out ther location doesn't currently enforce isolation. `fling_add_module()` lives in `CMake/FlingModule.cmake` but is not yet used for a real shared module. - `Sandbox/` — the sample game + editor, one executable today (editor support is - toggled by a project-wide `WITH_EDITOR` define, which `docs/BuildModules.md` - plans to remove in favor of two separate executables). + toggled by a project-wide `WITH_EDITOR` define; a planned follow-up splits + this into two separate executables). - `FlingTests/` — Catch2 tests, links the full `FlingEngine` library. See [`testing.md`](testing.md). - `external/` — vendored third-party dependencies (submodules). Never modified. -- `docs/` — human-facing docs: `docs/CodingStyle.md`, `docs/BuildModules.md`, and - the `Fling-Engine-logo/` image used by the root README. +- `docs/` — the `Fling-Engine-logo/` image used by the root README. - `scripts/` — repo tooling (`check_comment_style.py`, the one-time `migrate_doxygen_comments.py` migration). - `CMake/` — CMake helper modules (e.g. `FlingEngineInc.cmake`). @@ -32,10 +31,9 @@ several things that look like cleanups are explicitly deferred or ruled out ther - Gameplay code (`Transform`, `Camera`, `World`, `Game`) must not include Graphics headers (`MeshRenderer.h`, Vulkan-facing types). Graphics may include Gameplay. - Editor is a leaf: `Engine` and `Graphics` must not include `BaseEditor` or any - Editor header. If you find yourself adding such an include, that's the bug - `docs/BuildModules.md` calls out explicitly — look for the debug-UI hook pattern - it describes instead. -- Don't add a new module folder or CMake target without reading the "Locked - decisions" section of `docs/BuildModules.md` first — several plausible-looking - approaches (per-module PCH, git submodules per system) are explicitly rejected there. - The how-to (including a UI module example) is [`adding-modules.md`](adding-modules.md). + Editor header. If you find yourself adding such an include, that's a bug — + look for the debug-UI hook pattern instead of a direct include. +- Don't add a new module folder or CMake target on a whim — per-module PCH and + git submodules per engine system have already been considered and rejected + for this project. The how-to (including a UI module example) is + [`adding-modules.md`](adding-modules.md). diff --git a/Skills/coding-style.md b/Skills/coding-style.md index da461fea..381d2788 100644 --- a/Skills/coding-style.md +++ b/Skills/coding-style.md @@ -1,7 +1,6 @@ # Coding Style -The canonical reference is [`docs/CodingStyle.md`](../docs/CodingStyle.md) — read -that first. This file is a quick-reference summary plus the pitfalls an AI assistant +Conventions for first-party Fling Engine C++, plus the pitfalls an AI assistant is most likely to hit. Scope: first-party code only — `FlingEngine/`, `Sandbox/`, `FlingTests/`. @@ -49,8 +48,7 @@ Trivial members/accessors can use a one-line `/** ... */`. ## Naming / conventions to preserve - Flat include names (`#include "Logger.h"`, not `#include "Core/Logger.h"`) are - intentional, even across the module split described in - [`docs/BuildModules.md`](../docs/BuildModules.md). Don't "fix" these to - path-qualified includes. + intentional, even across the in-progress module split described in + [`architecture.md`](architecture.md). Don't "fix" these to path-qualified includes. - `pch.h` is being deleted per the module-split plan — don't add new `#include "pch.h"` to public headers, and don't rely on it as a kitchen-sink include. diff --git a/docs/BuildModules.md b/docs/BuildModules.md deleted file mode 100644 index 37b0f370..00000000 --- a/docs/BuildModules.md +++ /dev/null @@ -1,426 +0,0 @@ -# Engine Build Modules - -Plan for [issue #169](https://github.com/flingengine/FlingEngine/issues/169): split the -monolithic `FlingEngine` library into Unreal-style **build modules** so a system -(for example Physics) can depend on math and gameplay types without pulling the -Vulkan pipeline. - -This document is the working design for branch `feature/169-engine-modules`. -Phase 0 (infrastructure + Core tree) is in progress. - -## Locked decisions - -These are settled. Do not re-open them while implementing. - -1. **Fold Foundation into Core.** Delete `FlingEngine/Foundation/` (the - `FoundationClass` stub, `FOUNDATION_API`, and its CMake target). Remove the - `#include "Foundation.h"` call from `Engine.cpp`. Do not keep a second - “core” DLL. -2. **Physically move `Utils/` and `Platform/` into `FlingEngine/Core/`.** - Keep the existing flat include names (`Logger.h`, `Platform.h`). After the - move, Core’s public include dir is `FlingEngine/Core/inc`. Delete the empty - `Utils/` and `Platform/` folders. Do not merge other modules into Core. -3. **Generate per-module `*API.h` from CMake** (e.g. `GraphicsAPI.h` defining - `GRAPHICS_API`). One template, configured per module into that module’s - binary dir. Do not check these files into git. -4. **Sandbox is two executables in the same CMake build:** `SandboxGame` and - `SandboxEditor`. `SandboxGame` must not compile or link any Editor module - (not `FlingEngine/Editor`, not `Sandbox/Editor`). A global `WITH_EDITOR` - `#define` cannot express this — both targets exist in one configure. - Consequence: **Engine and Graphics must not link or include the Editor - module.** Editor is a leaf linked only by editor executables. - -## Current state - -- Root `CMakeLists.txt` configures options, third-party deps, then adds - `FlingEngine`, `FlingTests`, and `Sandbox`. -- `FlingEngine/CMakeLists.txt` glob-recurses engine sources into **one** - `add_library(FlingEngine ...)` (static `.a` / `.lib`). -- `CMake/FlingEngineInc.cmake` still adds every remaining engine `inc/` folder - to consumers. Isolation via `target_link_libraries` starts in Phase 1. -- `CMake/FlingModule.cmake` provides `fling_add_module()`; it is not used by a - real shared module yet (Phase 1: Core). -- `Utils/` and `Platform/` have been folded into `FlingEngine/Core/`. - `ImGuiInputBinding.hpp` lives under `Graphics/inc/` (not Core). -- Foundation stub DLL has been deleted. -- DLL export today is still a single `FLING_API`, toggled by `FLING_EXPORT` - inside `Core/inc/pch.h`. Per-module `*_API` macros are generated by - `fling_add_module()` once modules exist. -- `pch.h` is **not actually a precompiled header**. Decision: **delete it** - in Phase 1. - -## Goal - -Match the *idea* of Unreal `*.Build.cs` files, implemented in CMake: - -1. Each module is its own CMake target (shared library by default). -2. Each module has its own `MODULE_API` export (`CORE_API`, `GRAPHICS_API`, …). -3. A module declares **public** and **private** dependencies. Public include - dirs and link libs propagate; private ones do not. -4. Adding a new module is a small CMake call plus a folder, not a root-file edit. -5. A game or tool links **only** the modules it needs. Physics can use - `FlingMath` / `Transform` without seeing Vulkan headers or linking - `spirv-cross`. - -Git submodules per engine system (mentioned in #169) are **out of scope**. -Separate repos only make sense after module boundaries are stable. - -**Do not refactor, reformat, or edit anything under `external/`.** Third-party -code stays as-is. First-party CMake may change *which module links* glfw, -SPIRV-Cross, ImGui, etc.; it must not change those projects' sources or their -own `CMakeLists.txt` files. - -## CMake analogue of Build.cs - -Unreal: - -```csharp -public class Physics : ModuleRules -{ - public Physics(ReadOnlyTargetRules Target) : base(Target) - { - PublicDependencyModuleNames.AddRange(new[] { "Core" }); - PrivateDependencyModuleNames.AddRange(new[] { "Gameplay" }); - } -} -``` - -Fling (proposed): - -```cmake -fling_add_module(Physics - PUBLIC_DEPS Core - PRIVATE_DEPS Gameplay -) -``` - -Implemented as `CMake/FlingModule.cmake`. The helper should: - -- `add_library(${NAME} SHARED)` (optional `STATIC` for tools/tests if needed). -- Glob `${module}/src` and `${module}/inc` with `CONFIGURE_DEPENDS` (`.cpp`, - `.h`, `.hpp`, `.inl`). Do not list every file by hand. -- `target_include_directories(PUBLIC inc PRIVATE src)`. -- `target_link_libraries(PUBLIC ${PUBLIC_DEPS} PRIVATE ${PRIVATE_DEPS})`. -- Define `FLING__BUILD` as a **PRIVATE** compile definition on that - target only (`FLING_GRAPHICS_BUILD`, `FLING_CORE_BUILD`, …). -- Configure `CMake/FlingModuleAPI.h.in` into - `${CMAKE_CURRENT_BINARY_DIR}/Generated/API.h` and add that directory as - a **PUBLIC** include of the module (so consumers get `GraphicsAPI.h` only by - linking Graphics). -- Alias `Fling::` to the target. -- Place the target in a Visual Studio folder `Fling/Modules`. -- Fail CMake configure if a dependency cycle is introduced. - -Consumers never call `include_directories()` for engine headers. Include paths -come only from `target_link_libraries`. That is what makes isolation real. - -`FLING_ENGINE_INC()` goes away once Sandbox and FlingTests link modules. - -Two INTERFACE umbrellas, because game and editor pull different modules: - -```cmake -add_library(Fling::GameStack INTERFACE) -target_link_libraries(Fling::GameStack INTERFACE - Fling::Core Fling::Resources Fling::Gameplay Fling::Graphics Fling::Engine -) - -add_library(Fling::EditorStack INTERFACE) -target_link_libraries(Fling::EditorStack INTERFACE - Fling::GameStack Fling::Editor -) -``` - -`Fling::Engine` is the composition root (`Engine::Run`), not the old monolith. -It does **not** depend on `Fling::Editor`. - -## Proposed module graph - -Dependencies must be a **DAG**. Shared libraries cannot have link cycles -(especially on Windows). - -``` -Core → Resources → Gameplay → Graphics → Engine - │ - ├── Physics (later; no Graphics) - └── Editor (leaf; editor executables only) -``` - -Editor is **not** under Engine. SandboxGame links through Engine and never -sees Editor. SandboxEditor links `Fling::EditorStack`. - -| Module | Folder after moves | Public deps | Third-party | -|--------|--------------------|-------------|-------------| -| **Core** | `FlingEngine/Core` (includes former Utils + Platform) | none | glm, spdlog, entt (public INTERFACE) | -| **Resources** | `FlingEngine/Resources` | Core | nlohmann json (private, via `Fling::Json`), inih, stb as needed | -| **Gameplay** | `FlingEngine/Gameplay` | Core, Resources | entt | -| **Graphics** | `FlingEngine/Graphics` | Core, Resources, Gameplay | Vulkan, glfw, SPIRV-Cross, tinyobjloader (**private**) | -| **Engine** | `FlingEngine/Engine` once split out of Core sources | Gameplay, Graphics | — | -| **Editor** | `FlingEngine/Editor` | Gameplay, Graphics | ImGui (**private**) | -| **Physics** (later) | `FlingEngine/Physics` | Core, Gameplay | none of Graphics | - -**Input** stays in Core. Window creation (`DesktopWindow`) belongs in Graphics, -not in `PlatformLinux.h`. - -### Core folder layout after the move - -``` -FlingEngine/Core/ - inc/ Platform.h, FlingTypes.h, Logger.h, FlingMath.h, Input/, … - src/ Logger.cpp, Timing.cpp, Engine.cpp (until Engine module split), … -FlingEngine/Resources/ -FlingEngine/Gameplay/ -FlingEngine/Graphics/ -FlingEngine/Editor/ -FlingEngine/Engine/ # created in Phase 3 when Engine.cpp leaves Core -``` - -Keep `#include "Logger.h"` style. Do not introduce `Core/Logger.h` prefixes. - -`Graphics/inc/ImGuiInputBinding.hpp` used to live under Platform. Input -`.cpp` files still include it while the engine is a monolith; do not move it -back into Core. Phase 1 must replace that include with a Graphics-registered -hook so Core has no ImGui dependency. - -Until Phase 3, `Engine.h` / `Engine.cpp` / `FlingEngine.h` may still live under -`Core/` but they are **not** part of the Core module’s sources once Engine is -its own target. Phase 3 moves them to `FlingEngine/Engine/`. - -### Why Gameplay sits *under* Graphics - -The Physics example only works if Gameplay does **not** depend on Graphics. - -- `Transform`, `Camera`, `World`, `Game` live in Gameplay. -- `MeshRenderer`, lights, Vulkan live in Graphics. -- Graphics may include Gameplay headers (`Transform`, `Camera`). -- Gameplay must **not** include `MeshRenderer.h` or Vulkan. - -Today World iterates `ComponentTypeRegistry` at runtime. Graphics registers -`MeshRenderer` and lights via `RegisterGraphicsComponents()`; Gameplay World -must **not** include `MeshRenderer.h` or light headers. That include was the -main Gameplay → Graphics cycle (see Phase 2). - -## Per-module API macros - -Replace one `FLING_API` with one generated header per module. - -`CMake/FlingModuleAPI.h.in`: - -```cpp -#pragma once -#include "FlingLibExports.h" -#if defined(FLING_@MODULE_UPPER@_BUILD) -# define @MODULE_UPPER@_API FLING_LIB_EXPORT -#else -# define @MODULE_UPPER@_API FLING_LIB_IMPORT -#endif -``` - -`fling_add_module(Graphics)` configures that to -`${CMAKE_CURRENT_BINARY_DIR}/Generated/GraphicsAPI.h` with `GRAPHICS_API`. -Same pattern for `CORE_API`, `RESOURCES_API`, `GAMEPLAY_API`, `ENGINE_API`, -`EDITOR_API`, `PHYSICS_API`. - -`FlingLibExports.h` lives in Core (`FLING_LIB_EXPORT` / `FLING_LIB_IMPORT` for -MSVC `__declspec` and GCC/Clang visibility). Public classes/functions that -cross a DLL boundary are marked with that module’s `_API`. Header-only -templates stay unmarked. - -CMake sets `FLING_GRAPHICS_BUILD` as a **PRIVATE** compile definition on the -Graphics target only. Consumers never get that define, so they `dllimport`. - -## Include rules (no PCH) - -- Public headers: `Module/inc/...`. Other modules may include these **only** if - they declared that module as a public or private dep. -- Private headers/sources: `Module/src/...`. Never on a consumer include path. -- No project-wide `include_directories()`. -- **Do not use precompiled headers.** This tree is small (~50 engine `.cpp` - files). PCH is not wired up today, and a shared PCH is hostile to module - isolation (every TU would still see Core+Input+Paths+Math). Per-module PCH - is not worth the CMake and include-order cost unless compile times become a - real problem later. -- Delete `pch.h` / `pch.cpp` / `MSVC_PCH.cmake`. Move `F_ENABLE_LOGGING` and - window-size defaults to a Core config header. Export macros come from CMake - per target, not from a catch-all include. -- `.cpp` files include only the headers they use. Public headers include what - they need and nothing else — never `pch.h`. -- `FlingEngine.h` becomes an optional convenience header in the Engine module - for games that want the full stack. Physics must not include it. - -Keep existing `#include "VulkanApp.h"` style (flat names) to avoid a mass -include rewrite. Isolation comes from include **paths**, not from renaming -headers to `Graphics/VulkanApp.h`. A later cleanup can namespace includes. - -## SandboxGame vs SandboxEditor - -Today one `Sandbox` exe is compiled with or without editor via a **global** -`WITH_EDITOR` flag. That goes away. - -Same CMake configure produces: - -| Target | Sources | Links | -|--------|---------|-------| -| **SandboxGame** | `Sandbox/Gameplay/*` + a game `main.cpp` | `Fling::GameStack` only | -| **SandboxEditor** | `Sandbox/Gameplay/*` + `Sandbox/Editor/*` + an editor `main.cpp` | `Fling::EditorStack` | - -Shared gameplay code should be an OBJECT or STATIC lib (`SandboxRuntime`) so -it is not compiled from a glob that also picks up `Editor/`. Do **not** glob -the whole `Sandbox/` tree into one target. - -`SandboxGame` must not have `FlingEngine/Editor/inc` or `Sandbox/Editor/inc` -on its include path. Including `BaseEditor.h` or `SandboxEditor.h` from the -game target is a bug. - -`WITH_IMGUI` can still exist as a Graphics feature (in-game debug HUD). It is -not the Editor module. `SandboxEditor` requires ImGui; `SandboxGame` does not -require the Editor module even if ImGui is on. - -`Engine::Run` takes only `T_GameType`. Editor entry: - -```cpp -// Sandbox/main_game.cpp -Engine.Run(argc, argv); - -// Sandbox/main_editor.cpp -Engine.Run(argc, argv); -// then register Sandbox::SandboxEditor with the debug-UI hook -``` - -Remove `WITH_EDITOR_FLAG` as a project-wide option that changes Engine’s -signature. If we keep a CMake option, it only controls whether the -`SandboxEditor` **target** is added, default ON. Both targets in one build is -the normal case. - -## Coupling that must be broken - -These will fail as soon as Graphics is no longer on Gameplay’s include path: - -1. **World → MeshRenderer / lights** - World no longer takes a caller-provided Graphics type list. Graphics - registers its types via `RegisterGraphicsComponents()`. Gameplay World - iterates the runtime `ComponentTypeRegistry` and must not include - `MeshRenderer.h`. - -2. **`PlatformLinux.h` / `PlatformWindows.h` → `DesktopWindow.h`** - Platform headers must not pull the windowing backend. Window types stay in - Graphics; platform headers keep macros, `FORCEINLINE`, and export helpers. - -3. **`Engine.h` / Graphics → `BaseEditor`** - Engine and Graphics must not include Editor headers. Today `Engine::Run` - is templated on `T_EditorType`, `VulkanApp::Init` takes - `shared_ptr`, and `ImGuiSubpass` stores one. Replace that with - an editor-free hook (optional `std::function` / thin `IDebugUI` in Core or - Graphics). `SandboxEditor` constructs `SandboxEditor` and registers the - draw callback. `SandboxGame` never sees the type. - `PRIVATE_WITH_EDITOR` / `PROTECTED_WITH_EDITOR` in `FlingTypes.h` cannot - stay as `#if WITH_EDITOR` if Gameplay is compiled once. Keep - `friend class BaseEditor;` always (elaborated friend; does not require - linking Editor). - -4. **`pch.h` `#define FLING_EXPORT`** - Export is a per-target CMake definition, not a header that every TU includes. - -5. **Graphics ↔ Gameplay in render code** - `GeometrySubpass`, `ShaderProgramReflections`, `ImGuiSubpass` including - `Transform` / `Camera` / `FirstPersonCamera` is allowed **if** Graphics - publicly or privately depends on Gameplay. Do not invert that. - -6. **Header-only cycles** - Forward-declare across modules. `Game.h` already forward-declares `World`; - keep doing that. - -## Implementation phases - -Work stays on `feature/169-engine-modules`. Each phase should leave -`SandboxGame` (and `SandboxEditor` once it exists) plus FlingTests building. - -### Phase 0 — Infrastructure + Core tree - -Done: - -- `CMake/FlingModule.cmake` (`fling_add_module()`) and `CMake/FlingModuleAPI.h.in`. -- `FlingLibExports.h` in Core. -- `Utils/` and `Platform/` moved into `FlingEngine/Core/`. -- `ImGuiInputBinding.hpp` moved to `Graphics/inc/`. -- Foundation stub deleted (`Engine.cpp` include and Sandbox/Tests link removed). -- `FLING_ENGINE_INC()` no longer references deleted Utils/Platform include dirs. - -### Phase 1 — Core as a real shared library - -- Core module sources: everything now under `FlingEngine/Core/` **except** - `Engine.h` / `Engine.cpp` / `FlingEngine.h` (those stay in the monolith - until Phase 3). -- Remaining engine sources still one lib that **links Core**. -- Replace `FLING_ENGINE_INC` shotgun includes for Core headers with - `target_link_libraries(... Fling::Core)`. -- Remove `pch.h` includes; give each file the headers it actually uses. -- Prove `CORE_API` exports on Windows and Linux. - -### Phase 2 — Resources, then Gameplay - -- Resources module; monolith links it. -- Gameplay module; **World must not include MeshRenderer.h** (graphics types - register through `RegisterGraphicsComponents()`) before splitting. -- Confirm a throwaway target that links only Core+Gameplay cannot `#include` - `VulkanApp.h` (compile-fail test is the isolation test). - -### Phase 3 — Graphics, Editor, Engine, two Sandbox exes - -- Graphics module: Vulkan, glfw, SPIRV-Cross stay **private** link deps. -- Decouple Engine/Graphics from `BaseEditor` (debug-UI hook). -- Editor module always built as its own target; only editor exes link it. -- Move `Engine.h` / `Engine.cpp` / `FlingEngine.h` to `FlingEngine/Engine/`. -- Split Sandbox into `SandboxRuntime` + `SandboxGame` + `SandboxEditor`. -- Remove project-wide `WITH_EDITOR` from Engine’s API. -- FlingTests link only the modules they exercise (`Fling::GameStack` unless a - test needs Editor). - -### Phase 4 — Prove the Physics story - -- Add a stub `Physics` module that includes `FlingMath.h` and `Transform.h` - and **does not** link Graphics. -- Optional: a small CMake test or `try_compile` that Graphics headers are - unreachable from Physics. - -## Success criteria - -- `ninja` / VS build produces `libCore.so`, `libGraphics.so`, … (or `.dll` + - import libs) plus **both** `SandboxGame` and `SandboxEditor`. -- `SandboxGame` does not link Editor and does not compile `Sandbox/Editor` - or `FlingEngine/Editor` sources. -- `SandboxEditor` still runs the deferred demo with the existing editor UI. -- `SandboxGame` still runs the deferred demo without the editor UI. -- FlingTests pass. -- A new module is added with one `fling_add_module()` and a folder; no edits to - a monolith glob in `FlingEngine/CMakeLists.txt`. -- Physics (stub) compiles against Core+Gameplay without Vulkan on its link line - or include path. - -## Non-goals for this branch - -- **Any edits under `external/`** (sources, headers, or third-party CMake). - Vendored libraries are consumed, not modified. -- Splitting modules into git submodules / separate repos. -- Rewriting the renderer or ECS. -- Changing asset pipelines. -- Mass-renaming includes to `Module/Header.h` (nice follow-up). -- Moving Graphics/Gameplay/Resources/Editor folders except as listed in - phases (Utils + Platform → Core, and Engine sources → `FlingEngine/Engine/` - in Phase 3). -- Forcing every type to be `MODULE_API`; start with classes that are used - across DLL boundaries (singletons, Engine, World, VulkanApp, etc.). Templates - and header-only types do not need export. - -## Risks - -- **Windows DLL + templates + singleton**: `Logger`, `VulkanApp`, - `ResourceManager` must live in one module and be exported, or you get duplicate - singletons. One instance per process, defined in that module’s `.cpp`. -- **MSVC needs at least one exported symbol** or the import `.lib` is not - produced (already noted in Foundation’s CMakeLists). -- **Glob + new files**: `CONFIGURE_DEPENDS` on module globs so adding a `.cpp` - re-runs CMake. -- **Runtime search path**: Linux `RPATH` / Windows DLL copy next to **both** - Sandbox executables so they find module `.so`/`.dll` files. -- **Compile times**: more DLLs can mean more relinks but better incrementality - within a module. Do not add PCH to “fix” that unless measurements say so. diff --git a/docs/CodingStyle.md b/docs/CodingStyle.md deleted file mode 100644 index a6ab6f47..00000000 --- a/docs/CodingStyle.md +++ /dev/null @@ -1,52 +0,0 @@ -# Coding Style - -Conventions for first-party Fling Engine C++ (`FlingEngine/`, `Sandbox/`, `FlingTests/`). -Third-party code under `external/` is left alone. - -## Documentation comments - -Prefer modern Doxygen-style blocks with a plain description. Do **not** use -Visual Studio / XML-wizard tags like `@brief`, and do not use `/*!`. - -```cpp -/** - * Holds onto the command line arguments passed to this application. - * - * @param Argc Number of arguments - * @param ArgV Argument values - * @return True if successfully initialized - */ -bool Init(const int32 Argc, const char* ArgV[]); -``` - -Guidelines: - -- Lead with a short description; skip `@brief`. -- Use `@param` / `@return` / `@see` only when they add information the signature does not already make obvious. -- Prefer `/** ... */` for API docs. One-line members can use `/** ... */` on a single line. -- Do not write auto-generated noise such as `@return float GetTimef` or empty `@brief` lines. - -Member fields and trivial accessors can use a short one-liner: - -```cpp -/** The time that the program started */ -double m_startTime = 0.0; -``` - -## Enforcement - -- **Comment content**: `python3 scripts/check_comment_style.py` - Rejects leftover `@brief` and `/*!` in first-party sources. -- **Layout**: `.clang-format` at the repo root. - clang-format does **not** validate documentation tags; it only formats code/comment layout. - -Format first-party files (example): - -```bash -find FlingEngine Sandbox FlingTests -type f \( -name '*.h' -o -name '*.hpp' -o -name '*.cpp' \) \ - | xargs clang-format -i -``` - -A one-time migration from the old `@brief`-heavy style lives in -`scripts/migrate_doxygen_comments.py` (see issue -[#166](https://github.com/flingengine/FlingEngine/issues/166)). diff --git a/scripts/check_comment_style.py b/scripts/check_comment_style.py index 4519ce60..c995df03 100755 --- a/scripts/check_comment_style.py +++ b/scripts/check_comment_style.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 """ Check that first-party C++ sources follow the FlingEngine doc-comment style -(see docs/CodingStyle.md and issue #166). +(see Skills/coding-style.md and issue #166). Fails if any of these legacy patterns remain: - @brief tags (prefer a plain description line)