-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: Stride Gamebuilder as an alternative for .NET IOC startup #2806
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Should we create a I feel like this too much of a change to fit into a minor release. By having a separate branch we could investigate further changes without setting anything into stone yet. |
|
I like this PR. |
|
I would be be happy with that. I am very hesitant to make some of these APIs public in a release since they were internal due to their unfinished state(specifically the windowing). My main goal of this PR was to get the other core contributors attention to see if there was anything absolutely atrocious that I did 😆 I think one of the benefits here would be for the GameStudio and how it instantiates the scene |
|
@Doprez, do you want me to run a copilot review on this and see what it will spit out? |
Sure, doesnt hurt to have some extra hints. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a customizable game startup using dependency injection to allow gradual upgrading of existing projects while exposing a new game windowing API. Key changes include refactoring GameBase and related classes to derive their GamePlatform from the GameContext, updating service registrations throughout, and adding a new GameBuilder for a streamlined startup process.
Reviewed Changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| sources/engine/Stride.Games/GameBase.cs | Refactored game platform and context handling, updated property access modifiers, and added helper methods for GameContext initialization. |
| sources/engine/Stride.Games/Desktop/GamePlatformDesktop.cs | Updated constructor to accept GameContext instead of GameBase. |
| sources/engine/Stride.Games/Android/GamePlatformAndroid.cs | Updated constructor to accept GameContext instead of GameBase. |
| sources/engine/Stride.Engine/Engine/Game.cs | Modified Game constructor and initialization to use provided or default GameContext. |
| sources/engine/Stride.Engine/Engine/Builder/*.cs | Added and updated game builder components to support DI and new service registrations. |
| sources/editor/* | Updated editor game startup code to integrate with the new startup and GameContext management. |
| sources/core/* | Updated service registration methods and file provider services to support context-based initialization. |
| sources/buildengine/* | Adjusted DatabaseFileProvider service to allow value updates. |
| sources/Directory.Packages.props | Added a new package reference for Microsoft.Extensions.Hosting. |
Comments suppressed due to low confidence (1)
sources/engine/Stride.Engine/Engine/Builder/GameBuilder.cs:120
- [nitpick] Consider renaming 'dataBase' to a more descriptive and properly cased variable name such as 'databaseProvider' for clarity.
var dataBase = Game.Services.GetService<IDatabaseFileProviderService>();
sources/engine/Stride.Engine/Engine/Builder/GameBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Introduce a new dependency-injection–driven startup pipeline for Stride games, enabling full customization of game initialization while minimizing breaking changes.
- Refactored
GameBaseto use aGameContext, support swapping contexts at runtime, and expose essential members asprotected. - Added
GameBuilder,MinimalGame, and extension methods to configure services, game systems, input sources, and logging viaIServiceCollection. - Extended
ServiceRegistryand core interfaces to allow explicit service registration by type and made database file provider mutable.
Reviewed Changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| sources/engine/Stride.Games/GameBase.cs | Refactored startup flow to use GameContext, added SetGameContext/EnsureGameContextIsSet, and made key properties protected. |
| sources/engine/Stride.Games/Desktop/GamePlatformDesktop.cs | Updated constructor to accept GameContext instead of GameBase. |
| sources/engine/Stride.Games/Android/GamePlatformAndroid.cs | Updated constructor to accept GameContext instead of GameBase. |
| sources/engine/Stride.Engine/Stride.Engine.csproj | Added Microsoft.Extensions.Hosting package reference. |
| sources/engine/Stride.Engine/Engine/Game.cs | Modified Game constructor, default context, input source initialization, and removed direct casts for database provider. |
| sources/engine/Stride.Engine/Engine/Design/GameSettings.cs | Removed unused System and Stride.Graphics usings. |
| sources/engine/Stride.Engine/Engine/Builder/MinimalGame.cs | Introduced hosted minimal game class with DI and hosted service implementation. |
| sources/engine/Stride.Engine/Engine/Builder/IGameBuilder.cs | Defined IGameBuilder interface to expose builder state. |
| sources/engine/Stride.Engine/Engine/Builder/GameBuilderExtensions.cs | Provided extension methods to register systems, services, logging, input, DB, and content for a game. |
| sources/engine/Stride.Engine/Engine/Builder/GameBuilder.cs | Implemented the GameBuilder class with service wiring, system registration, context setup, and build logic. |
| sources/editor/Stride.Editor/Preview/PreviewGame.cs | Updated constructor to accept GameContext. |
| sources/editor/Stride.Editor/Preview/GameStudioPreviewService.cs | Passed GameContext into PreviewGame. |
| sources/editor/Stride.Editor/Engine/EmbeddedGame.cs | Updated EmbeddedGame constructor to accept GameContext. |
| sources/editor/Stride.Editor/EditorGame/Game/EditorServiceGame.cs | Added overload constructor accepting GameContext. |
| sources/editor/Stride.Assets.Presentation/AssetEditors/EntityHierarchyEditor/Game/EntityHierarchyEditorGame.cs | Updated constructor to forward GameContext. |
| sources/core/Stride.Core/ServiceRegistry.cs | Added AddService(object, Type) overload to trigger ServiceAdded event. |
| sources/core/Stride.Core/IServiceRegistry.cs | Declared new AddService(object, Type) interface member. |
| sources/core/Stride.Core.Serialization/IO/IDatabaseFileProviderService.cs | Made FileProvider setter public for mutable DB plumbing. |
| sources/buildengine/Stride.Core.BuildEngine.Common/MicrothreadLocalDatabases.cs | Implemented FileProvider setter on microthread-local provider. |
| sources/Directory.Packages.props | Pin Microsoft.Extensions.Hosting version. |
Comments suppressed due to low confidence (4)
sources/engine/Stride.Engine/Engine/Game.cs:447
- The variables
rawInputEnabledandcontextare not defined in this scope. You likely meant to reference the instance fieldsContextand a correspondingRawInputEnabledproperty. Rename and reference the correct fields or pass them into this method.
if (rawInputEnabled && context is GameContextWinforms gameContextWinforms)
sources/engine/Stride.Engine/Engine/Builder/GameBuilder.cs:18
- The collection initializer
[]is invalid C#. Use a concrete constructor such asnew Dictionary<Type, object>();.
public Dictionary<Type, object> InternalServices { get; internal set; } = [];
sources/engine/Stride.Engine/Engine/Builder/GameBuilder.cs:24
- The list initializer
[]is invalid here. Replace withnew List<LogListener>();to correctly instantiate the collection.
public List<LogListener> LogListeners { get; internal set; } = [];
sources/engine/Stride.Engine/Engine/Builder/GameBuilder.cs:26
- Invalid list initializer
[]; usenew List<IInputSource>();to create the collection instance.
public List<IInputSource> InputSources { get; internal set; } = [];
| } | ||
| } | ||
|
|
||
| public virtual void SetGameContext(GameContext context) |
Copilot
AI
May 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the branch where Context is null and a non-null context is passed, you only set Context = context without wiring up GamePlatform events or registering services. Factor out the event hookup and service registration into a common helper so the first assignment is fully initialized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is intentional. We are assuming that the user has set up their own GameContext properly. Although I do want to revisit GameContext in general later when we have a branch where things can break more.
This one could definitely be better but was a workaround to keep the same functionality of the current Game class.
|
very nice! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 37 out of 37 changed files in this pull request and generated 18 comments.
| gameBuilder.InputSources.Add(inputSource); | ||
| return gameBuilder; | ||
| } | ||
| } |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing UseStrideFontSystem extension method. The PR description's example code shows usage of UseStrideFontSystem() but this method is not defined in GameBuilderExtensions. This will cause compilation errors for users following the example code in the PR description.
sources/buildengine/Stride.Core.BuildEngine.Common/MicrothreadLocalDatabases.cs
Outdated
Show resolved
Hide resolved
| _log.Info($"Registering service {service.Key.Name}."); | ||
| Game.Services.AddService(instance, service.Key); | ||
| InternalServices[service.Key] = instance; |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential null reference exception. When instance is null after both checks (lines 82-96), calling Game.Services.AddService(instance, service.Key) on line 99 will pass a null value which may cause issues. Consider logging a warning or skipping the service registration when the instance cannot be resolved.
| _log.Info($"Registering service {service.Key.Name}."); | |
| Game.Services.AddService(instance, service.Key); | |
| InternalServices[service.Key] = instance; | |
| if (instance != null) | |
| { | |
| _log.Info($"Registering service {service.Key.Name}."); | |
| Game.Services.AddService(instance, service.Key); | |
| InternalServices[service.Key] = instance; | |
| } | |
| else | |
| { | |
| _log.Warning($"Could not resolve service {service.Key.Name}. Skipping registration."); | |
| } |
| preferredParameters.PreferredBackBufferHeight = resizedBackBufferHeight; | ||
| } | ||
|
|
||
| graphicsDeviceFactory = game.Services.GetService<IGraphicsDeviceFactory>(); |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The graphicsDeviceFactory field is being reassigned here during device lookup. This seems unnecessary since it was already set in the constructor (line 133). Re-fetching it here could mask issues if the service was removed or replaced, and might introduce unexpected behavior. Consider removing this line unless there's a specific reason the factory needs to be refreshed at this point.
| graphicsDeviceFactory = game.Services.GetService<IGraphicsDeviceFactory>(); | |
| // Use the graphicsDeviceFactory set in the constructor; do not reassign here. |
| public GameSystemCollection GameSystems { get; internal set; } | ||
|
|
||
| /// <summary> | ||
| /// Adds log listeners to the game on <see cref="Build"/>. This is registered first so it will log build errors if they occur."/> |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation has incorrect closing tag. Should use </summary> instead of "/>.
| /// Adds log listeners to the game on <see cref="Build"/>. This is registered first so it will log build errors if they occur."/> | |
| /// Adds log listeners to the game on <see cref="Build"/>. This is registered first so it will log build errors if they occur. |
| public Game(GameContext context = null) : base() | ||
| { | ||
| Context = context ?? GetDefaultContext(); | ||
| Context.CurrentGame = this; |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing assignment: Context.Services is never assigned the value of Services (from GameBase). This will cause a NullReferenceException when GamePlatform constructor tries to access context.Services on line 49. Add Context.Services = Services; after line 194 (or in the appropriate place) to fix this issue.
| Context.CurrentGame = this; | |
| Context.CurrentGame = this; | |
| Context.Services = Services; |
|
|
||
| protected GamePlatform(GameBase game) | ||
| protected GamePlatform(GameContext context) | ||
| { |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential null reference issue: Context.Services is accessed in GamePlatform constructor (line 49), but GameContext instances created by GameContextFactory don't appear to initialize the Services property. This could cause a NullReferenceException when GamePlatform.Create(context) is called. Ensure that GameContext.Services is properly initialized before being used.
| { | |
| { | |
| if (context.Services == null) | |
| { | |
| context.Services = new ServiceRegistry(); | |
| } |
Co-authored-by: Copilot <[email protected]>
…LocalDatabases.cs Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
PR Details
This PR adds the ability to completely customize the startup of a users game by using dependency injection.
This is a rethinking of my original work to try and make it at least a minimal breaking change so that GameStudio and existing projects can be upgraded gradually. This is not feature complete but I would like to build the new windowing API based on this structure assuming this is approved as a valid change.
I also took some small inspiration from how Avalonia decided on their own startup structure so there are some very small similarities.
Why didn't I use existing AppBuilders/HostBuilders?
I did look into this with the original starting point being based on #1841 but without changing the core Stride libraries it wasn't as useful out of the box. I did still abstract over the
IServiceCollectionin order to allow us to migrate over to that system in the future or allow users to add dependencies that exist in .NET already. The other issue with theIServiceCollectionis that you can not add services at runtime like you can with StridesIServiceRegistrywhich would have been a major breaking change throughout everything.What about existing classes inheriting or using
Game?This should be working exactly as it does today apart from how content and
GameContextis loaded in the project and this is currently the main thing that could break projects if merged in. Thankfully most functionality was internal already so this shouldn't be an issue for 99% of users.Why make the
GameWindowpublic?Honestly just looking for some feedback here but making it public allows users to properly set up the
GameContextif they decide to use it with the new startup. This is not concrete as well since assuming this gets approved I would be changing how theGraphicsDevicesees theGameWindowas purely a position and size rather than a whole object.Why not fully use the IOC PR like #1841
This is a less breaking change and allows the underlying code to be modified without the users noticing. My PR is meant to be the path of least resistance.
Example Startup code
Extensions that are used in the default
Gameclass for a generic Stride games to run:Actual Startup code with custom inputs and logger:
Related Issue
Original runtime management change
#2404
This was my first attempt with the focus of only managing the
GameWindowbut quickly cascaded into a mess of breaking changes and turned into a mess of unmaintainable code. This went from "I want to change the window host" to "dear god everything is hard coded and broken after a single change"Second attempt at startup changes
#2574
This was my better PR for the startup change but broke WAY too much to justify the cascading changes required to work with GameStudio.
modern IOC change
#1841
I really loved the idea of this one but it breaks so much with the core systems of Stride since they rely on Stride specific alternatives. For example Services and logs which would need to be updated throughout Strides libraries which is out of scope for my time.
In an ideal world I think this one is a good option but without more manpower/hours feels unrealistic for now.
Types of changes
Checklist