#21 Structural changes - #142
Conversation
Doesn't work quite yet
Everything after this point needs to have a structure and proper planning. It's been a wild west until now
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
There are multiple build- and runtime-blocking issues (logger type mismatch in EngineHandler inheritance, busy-spin in Engine.Initialize, WindowHandler self-deadlock/blocking Run call, and broken project/solution references) that must be resolved before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR performs broad structural refactoring across the engine, introducing new managers (input/shader/renderer), a DI-based app/engine bootstrap, and a new in-repo OBJ loader project while updating examples and tests to match.
Changes:
- Refactors window lifecycle and per-frame input/render orchestration (Window/SilkWindow + new managers).
- Introduces a DI-driven engine/app registration model (SharpEngine.Core.DependencyInjection) and updates examples accordingly.
- Adds a new SharpEngine.Core.ObjLoader project (parsers/loaders/datastore) and updates solution/test wiring.
File summaries
| File | Description |
|---|---|
| Tests/SharpEngine.Core.ArchitectureTests/SharpEngine.Core.ArchitectureTests.csproj | Adds test SDK + xUnit v3 and references Core project. |
| Tests/SharpEngine.Core.ArchitectureTests/GameObjectTests.cs | Adds architecture test for GameObject constructors. |
| Tests/SharpEngine.Core.ArchitectureTests/Class1.cs | Removes placeholder class. |
| Tests/ObjLoader.Test/SharpEngine.Core.ObjLoader.Tests.csproj | Updates ObjLoader test project references. |
| SharpEngine.slnx | Updates solution project paths (tests + ObjLoader project). |
| SharpEngine.Dto/SharpEngine.Core.Dto.csproj | Adds new DTO project scaffold. |
| SharpEngine.Dto/Class1.cs | Adds placeholder class for DTO project. |
| SharpEngine.Core/Windowing/Window.cs | Refactors window initialization, input, shader usage, and renderer execution. |
| SharpEngine.Core/Windowing/SilkWindow.cs | Adjusts window abstraction: title handling, background color, events, and CurrentWindow nullability. |
| SharpEngine.Core/SharpEngine.Core.csproj | Adjusts compile includes and adds IO project reference; removes some packages. |
| SharpEngine.Core/ShaderManager.cs | Introduces ShaderManager wrapper around shader loading/usage. |
| SharpEngine.Core/Scenes/SceneNode.cs | Moves EmptyNode out; adds initialization hooks and lifecycle methods. |
| SharpEngine.Core/Scenes/Scene.cs | Converts Scene into SaveableFile-based type and removes inline JSON persistence code. |
| SharpEngine.Core/Scenes/EmptyNode.cs | Adds EmptyNode as separate file. |
| SharpEngine.Core/Rendering/ShaderParameterBinder.cs | Makes logger optional and uses telemetry logger factory helper. |
| SharpEngine.Core/Renderers/UIRenderer.cs | Updates logging creation and adds UIElement query on attach. |
| SharpEngine.Core/RendererManager.cs | Introduces renderer discovery/selection/execution manager. |
| SharpEngine.Core/InputManager.cs | Introduces centralized input context + event dispatch. |
| SharpEngine.Core/Handlers/WindowHandler.cs | Reworks window handler lifecycle and queueing model. |
| SharpEngine.Core/Handlers/EngineHandler.cs | Changes handler start/stop model and cancellation handling. |
| SharpEngine.Core/Entities/UI/UIElement.cs | Moves UIElement toward component-based model and GL init via OnInitialized. |
| SharpEngine.Core/Entities/UI/MeshRenderer.cs | Makes MeshRenderer a component. |
| SharpEngine.Core/Entities/UI/Layouts/LayoutBase.cs | Makes layouts transformable UI nodes with spacing. |
| SharpEngine.Core/Entities/UI/Layouts/GridLayout.cs | Expands grid layout to auto-position UIElements with optional bounds. |
| SharpEngine.Core/Entities/UI/DraggableUIElement.cs | Adds new draggable UI element type. |
| SharpEngine.Core/Entities/Interfaces/IComponent.cs | Adds component interface + GameObject-related extension helpers. |
| SharpEngine.Core/Entities/Interfaces/IClickable.cs | Adds click interface + component/event payload types. |
| SharpEngine.Core/Entities/GameObject.cs | Shifts GameObject toward components/material default helper. |
| SharpEngine.Core/EngineServiceManager.cs | Changes handler registration/startup pattern. |
| SharpEngine.Core/Engine.cs | Converts Engine from static to instance, adds handler startup loop and version check. |
| SharpEngine.Core/DependencyInjection/ServiceCollectionExtensions.cs | Removes old window DI registration approach. |
| SharpEngine.Core/Abstractions/IGame.cs | Adds Window property and OnAfterRender hook to Game abstraction. |
| SharpEngine.Core/_Resources/Shaders/uiShader.vert | Switches UI vertex transform to pixel-space ortho projection approach. |
| SharpEngine.Core.Shared/Debug.cs | Adds Serilog-based debug helper scaffold. |
| SharpEngine.Core.ObjLoader/TypeParsers/VertexParser.cs | Adds OBJ vertex parser. |
| SharpEngine.Core.ObjLoader/TypeParsers/UseMaterialParser.cs | Adds OBJ usemtl parser. |
| SharpEngine.Core.ObjLoader/TypeParsers/TypeParserBase.cs | Adds base parser keyword matching helper. |
| SharpEngine.Core.ObjLoader/TypeParsers/TextureParser.cs | Adds OBJ texture coordinate parser. |
| SharpEngine.Core.ObjLoader/TypeParsers/SmoothingGroupParser.cs | Adds OBJ smoothing group parser. |
| SharpEngine.Core.ObjLoader/TypeParsers/ObjectNameParser.cs | Adds OBJ object-name parser. |
| SharpEngine.Core.ObjLoader/TypeParsers/NormalParser.cs | Adds OBJ normal parser. |
| SharpEngine.Core.ObjLoader/TypeParsers/MaterialLibraryParser.cs | Adds OBJ material-library parser integration. |
| SharpEngine.Core.ObjLoader/TypeParsers/ITypeParser.cs | Adds type parser interface. |
| SharpEngine.Core.ObjLoader/TypeParsers/GroupParser.cs | Adds OBJ group parser. |
| SharpEngine.Core.ObjLoader/TypeParsers/FaceParser.cs | Adds OBJ face parser. |
| SharpEngine.Core.ObjLoader/SharpEngine.Core.ObjLoader.csproj | Adds new in-repo ObjLoader project. |
| SharpEngine.Core.ObjLoader/Loaders/ObjLoader/ObjLoaderFactory.cs | Adds loader factory with extension-based dispatch. |
| SharpEngine.Core.ObjLoader/Loaders/ObjLoader/ObjLoader.cs | Adds OBJ file loader core. |
| SharpEngine.Core.ObjLoader/Loaders/ObjLoader/FileManager.cs | Adds file wrapper for testability. |
| SharpEngine.Core.ObjLoader/Loaders/MaterialLoader/MaterialLibraryLoaderFacade.cs | Adds facade for MTL loading relative to OBJ path. |
| SharpEngine.Core.ObjLoader/Loaders/MaterialLoader/MaterialLibraryLoader.cs | Adds MTL parser/loader implementation. |
| SharpEngine.Core.ObjLoader/Loaders/MaterialLoader/IMaterialLibraryLoaderFacade.cs | Adds facade interface. |
| SharpEngine.Core.ObjLoader/Loaders/LoaderBase.cs | Adds shared file parsing base. |
| SharpEngine.Core.ObjLoader/Loaders/IFileManager.cs | Adds file manager abstraction. |
| SharpEngine.Core.ObjLoader/ISmoothingGroupDataStore.cs | Adds smoothing-group datastore interface. |
| SharpEngine.Core.ObjLoader/IObjectNameDataStore.cs | Adds object-name datastore interface. |
| SharpEngine.Core.ObjLoader/IMaterialDataStore.cs | Adds material datastore interface. |
| SharpEngine.Core.ObjLoader/DataStore.cs | Adds OBJ parsing datastore implementation. |
| SharpEngine.Core.DependencyInjection/WindowServiceCollectionExtensions.cs | Adds new DI registration for WindowHandler + Window creation/configure hooks. |
| SharpEngine.Core.DependencyInjection/SharpEngine.Core.DependencyInjection.csproj | Adds new DI project with MS.Extensions deps. |
| SharpEngine.Core.DependencyInjection/HandlerRegistrationBuilder.cs | Adds handler registration builder pattern. |
| SharpEngine.Core.DependencyInjection/EngineServiceCollectionExtensions.cs | Adds AddEngine() service registration. |
| SharpEngine.Core.DependencyInjection/EngineRegistrationBuilder.cs | Adds engine registration builder. |
| SharpEngine.Core.DependencyInjection/AppBuilder.cs | Adds DI app builder for configuration/services. |
| SharpEngine.Core.DependencyInjection/App.cs | Refactors app run to resolve engine/game and start engine. |
| SharpEngine.Core.Components/Properties/Material.cs | Minor import adjustment for shaders. |
| Examples/Tutorial 4.1 - Model Loading/Tutorial 4.1 - Model Loading.csproj | Adds reference to new ObjLoader project. |
| Examples/MultipleWindows/Program.cs | Updates handler start and input wiring for new InputManager location. |
| Examples/MinecraftClone/Program.cs | Migrates to AddEngine()/WindowHandler registration and DI changes. |
| Examples/MinecraftClone/Minecraft.csproj | Adds dependency injection project reference. |
| Examples/MinecraftClone/Minecraft.cs | Updates UI initialization to use new window bounds + grid layout; updates OnAfterRender override. |
Review details
Suppressed comments (1)
SharpEngine.Core/Handlers/WindowHandler.cs:121
- WindowHandler.DequeueWindows calls window.Run() inside the handler loop. Run() is typically a blocking native-window loop, so this will prevent managing additional windows and prevent the outer ExecuteAsync loop from progressing.
- Files reviewed: 45/72 changed files
- Comments generated: 7
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| protected readonly ILogger<EngineHandler> Logger; | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of <see cref="EngineHandler"/>. | ||
| /// </summary> |
| ServicesManager.StartHandlers(_cancellationTokenSource); | ||
|
|
||
| Initialize(); | ||
| Services.RegisterHandler(new WindowHandler(window)); | ||
| while (!_cancellationTokenSource.Token.IsCancellationRequested) | ||
| { | ||
|
|
| foreach (var window in _windows) | ||
| window.Close(); | ||
|
|
||
| await StopAsync(); | ||
| } |
| using System.Reflection; | ||
| using SharpEngine.Core.Entities; | ||
| using Xunit; | ||
|
|
||
| namespace SharpEngine.Core.ArchitectureTests; | ||
|
|
||
| public class GameObjectTests | ||
| { | ||
|
|
||
| [Fact] | ||
| public void GameObjects_Should_Not_Have_Public_Constructors() | ||
| { | ||
| var gameObjectTypes = typeof(GameObject).Assembly | ||
| .GetTypes() | ||
| .Where(t => | ||
| !t.IsAbstract && | ||
| typeof(GameObject).IsAssignableFrom(t)); |
| <Folder Name="/Tests/IntegrationTests/" /> | ||
| <Folder Name="/Tests/UnitTests/"> | ||
| <Project Path="Tests/ObjLoader.Test/SharpEngine.Core.ObjLoader.Tests.csproj"> | ||
| <Project Path="Tests/SharpEngine.Core.ObjLoader.Test/SharpEngine.Core.ObjLoader.Tests.csproj"> |
| <ItemGroup> | ||
| <ProjectReference Include="..\..\ObjLoader\SharpEngine.Core.ObjLoader.csproj" /> | ||
| <ProjectReference Include="..\..\SharpEngine.Core.ObjLoader\SharpEngine.Core.ObjLoader.csproj" /> | ||
| </ItemGroup> |
| <ItemGroup> | ||
| <ProjectReference Include="..\..\ObjLoader\SharpEngine.Core.ObjLoader.csproj" /> | ||
| <ProjectReference Include="..\..\SharpEngine.Core.Components\SharpEngine.Core.Components.csproj" /> | ||
| <ProjectReference Include="..\..\SharpEngine.Core.ObjLoader\SharpEngine.Core.ObjLoader.csproj" /> | ||
| <ProjectReference Include="..\..\SharpEngine.Core\SharpEngine.Core.csproj" /> | ||
| </ItemGroup> |
|
|
For the time being, I want to get this and the other PRs in that waiting so that there is as little blockers for the first release as possible. I'll address any/all SonarCloud findings in a separate PR later |




#21 Structural changes
Contents
So far the development hasn't had a proper structure and it's been following waterfall in a sense that things were free styled. This PR is the last one of those and after this things should stabilize.
Checklist
mainto my branch.