Files
XCEngine/docs/plan/editor-engine-services-refactor-plan.md

268 lines
7.0 KiB
Markdown
Raw Normal View History

# EditorEngineServices Refactor Plan
## Problem
The root problem is no longer that `EngineEditorServices.cpp` used to be too large.
That implementation has already been split. The real architectural problem is
still `EditorEngineServices` itself:
- it is a god interface
- it mixes scene backend creation, viewport bridge logic, shader loading,
object-id resolving, and engine lifecycle
- editor subsystems still depend on one shared service-locator style entrypoint
- new bridge needs will keep accumulating in `editor/app/Services/Engine`
unless that dependency surface is actively reduced
This refactor must therefore optimize for dependency reduction, not file
splitting.
## Target State
The goal is not "no bridge code".
The goal is "distributed, narrow bridges at clear boundaries instead of one
central editor-to-engine gateway".
The target editor-side interfaces are:
- `EditorSceneBackendFactory`
- only `CreateSceneBackend`
- `SceneViewportEngineBridge`
- only scene viewport frame-plan build, render, and object-id resolve
- `GameViewportEngineBridge`
- only game viewport frame-plan build and render
- `EditorShaderProvider`
- only shader or resource acquisition
- `EditorEngineLifecycle`
- only `UpdateAsyncLoads` and `Shutdown`
If `EngineEditorServices.cpp` still exists at the end, it may only exist as an
internal composition-shell implementation. It must not remain a shared runtime
dependency across editor features.
## Freeze Rule
`EditorEngineServices` is now a compatibility shell under freeze:
- no new public methods
- no new feature bridges added to it
- no new panels, tools, passes, or runtimes depending on it
- only subtraction is allowed: move capabilities out of it
Without this rule, the refactor will fail even if some files get cleaner.
## Current Consumer Map
Refactoring must follow actual consumers, not folder aesthetics.
### `EditorContext`
Current usage:
- `CreateSceneBackend`
Conclusion:
- it does not need viewport, shader, game, or lifecycle capabilities
- it should depend only on `EditorSceneBackendFactory`
File:
- `editor/app/Composition/EditorContext.cpp`
### `SceneViewportRenderService`
Current usage:
- `BuildSceneViewportFramePlan`
- `RenderSceneViewportFramePlan`
- `TryResolveActiveSceneRenderObjectId`
Conclusion:
- it should depend only on `SceneViewportEngineBridge`
File:
- `editor/app/Rendering/Viewport/SceneViewportRenderService.cpp`
### `GameViewportRenderService`
Current usage:
- `BuildGameViewportFramePlans`
- `RenderGameViewportFramePlans`
Conclusion:
- it should depend only on `GameViewportEngineBridge`
File:
- `editor/app/Rendering/Viewport/GameViewportRenderService.cpp`
### Scene Viewport Passes
Current usage:
- `LoadShader`
Affected code:
- `SceneViewportGridPass`
- `SceneViewportSelectionOutlinePass`
Conclusion:
- low-level render passes are taking a full engine facade just to load shaders
- this is the wrong dependency direction
- they should depend only on `EditorShaderProvider`
Files:
- `editor/app/Rendering/Viewport/Passes/SceneViewportGridPass.cpp`
- `editor/app/Rendering/Viewport/Passes/SceneViewportSelectionOutlinePass.cpp`
### `Application`
Current usage:
- `UpdateAsyncLoads`
- `Shutdown`
Conclusion:
- it should depend only on `EditorEngineLifecycle`
File:
- `editor/app/Bootstrap/Application.cpp`
## Execution Order
This work should proceed in dependency order, not in arbitrary file order.
### Phase A. Freeze the Interface
Goals:
- document `EditorEngineServices` as a temporary compatibility shell
- enforce the "no new methods" rule in code review and follow-up work
Acceptance:
- `EditorEngineServices.h` clearly states the compatibility-shell role
- no new consumers are introduced
### Phase B. Split by Consumer
Goals:
- remove the easy, high-signal consumers from the god interface first
Execution order:
1. `EditorContext` -> `EditorSceneBackendFactory`
2. `SceneViewportRenderService` -> `SceneViewportEngineBridge`
3. `GameViewportRenderService` -> `GameViewportEngineBridge`
4. `Application` -> `EditorEngineLifecycle`
Why this order:
- low behavior risk
- mostly editor-side injection cleanup
- removes the largest legitimate consumers first
Acceptance:
- those four consumers no longer include or store `EditorEngineServices`
- each one is injected with a narrow dependency
### Phase C. Remove Low-Level Resource Leakage
Goals:
- remove `LoadShader` reach from low-level viewport passes
Execution order:
1. introduce `EditorShaderProvider`
2. migrate `SceneViewportGridPass`
3. migrate `SceneViewportSelectionOutlinePass`
Acceptance:
- scene viewport passes no longer include `EditorEngineServices.h`
- shader access flows through a narrow provider
### Phase D. Collapse the Compatibility Shell
Goals:
- remove the public architectural role of `EditorEngineServices`
Allowed end states:
1. delete `EditorEngineServices`
2. keep only an internal composition object that does not appear in feature,
runtime, or pass dependencies
Disallowed fake end states:
- split implementation files but keep the same shared public facade forever
- rename the god interface without shrinking its surface
## Non-Goals
This plan intentionally does not include:
- mixing this work with `EditorSceneComponentMutation` typed-command refactors
- copying Unreal Engine module structure mechanically
- removing all bridge code as a principle
- large behavior rewrites when dependency shrinkage is sufficient
## Code Movement Rules
All follow-up implementation should obey these rules:
- interfaces are named by consumer boundary, not by generic service language
- each interface exposes only the methods that one consumer actually uses
- render passes must not depend on scene backend factory or lifecycle code
- composition owns assembly; features and passes must not locate services on
their own
- bridge code is acceptable only when it stays narrow, local, and scoped to a
concrete boundary
## Completion Criteria
This refactor is complete only when all of the following are true:
- `EditorContext` no longer depends on `EditorEngineServices`
- `SceneViewportRenderService` no longer depends on `EditorEngineServices`
- `GameViewportRenderService` no longer depends on `EditorEngineServices`
- `Application` no longer depends on `EditorEngineServices`
- scene viewport passes no longer depend on `EditorEngineServices`
- `EditorEngineServices` has gained no new methods
- `editor/app/Services/Engine` has stopped being the default dumping ground for
every editor-engine bridge
## Immediate Next Cut
The next cut is not "split `EngineEditorServices.cpp` again".
The correct next cut is:
1. define `EditorSceneBackendFactory`
2. make `EditorContext` depend only on that interface
3. remove scene backend creation from the `EditorEngineServices` consumer
surface
Why this is next:
- it is the safest cut
- it has the narrowest dependency
- it starts emptying `EditorEngineServices` from the top of the editor stack
- it creates the migration template for viewport, game, lifecycle, and shader
provider splits