268 lines
7.0 KiB
Markdown
268 lines
7.0 KiB
Markdown
# 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
|