Closed Barsonax closed 4 years ago
I'm not a big fan of fluent APIs and builder patterns unless they're necessary to achieve something - and there's also, I believe, no precedent in the Duality core so far. How about something like this?
bool isDebugging = System.Diagnostics.Debugger.IsAttached || args.Contains(DualityApp.CmdArgDebug);
bool isProfiling = args.Contains(DualityApp.CmdArgProfiling);
using (DualityLauncher launcher = new DualityLauncher())
{
launcher.AddLogOutput(new ConsoleLogOutput());
launcher.AssemblyLoader = new DefaultAssemblyLoader();
launcher.WindowOptions = new WindowOptions
{
Size = DualityApp.UserData.WindowSize,
ScreenMode = isDebugging ? ScreenMode.Window : DualityApp.UserData.WindowMode,
RefreshMode = (isDebugging || isProfiling) ? RefreshMode.NoSync : DualityApp.UserData.WindowRefreshMode,
Title = DualityApp.AppData.AppName,
SystemCursorVisible = isDebugging || DualityApp.UserData.SystemCursorVisible
};
// Load the starting Scene
Scene.SwitchTo(DualityApp.AppData.StartScene);
launcher.Run();
}
Or, in simplified form with reasonable defaults for the DualityLauncher
class:
bool isDebugging = System.Diagnostics.Debugger.IsAttached || args.Contains(DualityApp.CmdArgDebug);
bool isProfiling = args.Contains(DualityApp.CmdArgProfiling);
using (DualityLauncher launcher = new DualityLauncher())
{
launcher.AddLogOutput(new ConsoleLogOutput());
if (isProfiling)
{
launcher.WindowOptions.RefreshMode = RefreshMode.NoSync;
}
if (isDebugging)
{
launcher.WindowOptions.SystemCursorVisible = true;
launcher.WindowOptions.ScreenMode = ScreenMode.Window;
launcher.WindowOptions.RefreshMode = RefreshMode.NoSync;
}
// Load the starting Scene
Scene.SwitchTo(DualityApp.AppData.StartScene);
launcher.Run();
}
I'm not a big fan of fluent APIs and builder patterns unless they're necessary to achieve something - and there's also, I believe, no precedent in the Duality core so far
Its mostly about getting a nice syntax while also making it possible to do additional validations in the build call and being extendable. Both approaches are fine I guess for now.
We should also abstract these lines away in some class so we don't have to keep duplicating these:
bool isDebugging = System.Diagnostics.Debugger.IsAttached || args.Contains(DualityApp.CmdArgDebug);
bool isProfiling = args.Contains(DualityApp.CmdArgProfiling);
We should also abstract these lines away in some class so we don't have to keep duplicating these:
Good point 👍
I think we could actually move them into the DualityLauncher
constructor, along with all the other data-dependent default values. And in that case, the conditions based on the two lines could be moved as well, leaving users with:
using (DualityLauncher launcher = new DualityLauncher())
{
launcher.AddLogOutput(new ConsoleLogOutput());
// Load the starting Scene
Scene.SwitchTo(DualityApp.AppData.StartScene);
launcher.Run();
}
Might even turn the console log output into a (toggle-or-removable-via-API) default, so it's even simpler:
using (DualityLauncher launcher = new DualityLauncher())
{
// Load the starting Scene
Scene.SwitchTo(DualityApp.AppData.StartScene);
launcher.Run();
}
Keeping this open since there is still some discussion going on about making things cleaner
We should also abstract these lines away in some class so we don't have to keep duplicating these:
Good point 👍
I think we could actually move them into the
DualityLauncher
constructor, along with all the other data-dependent default values. And in that case, the conditions based on the two lines could be moved as well, leaving users with:using (DualityLauncher launcher = new DualityLauncher()) { launcher.AddLogOutput(new ConsoleLogOutput()); // Load the starting Scene Scene.SwitchTo(DualityApp.AppData.StartScene); launcher.Run(); }
In the end I moved the args to a separate LauncherArgs class since its being used in multiple places and I think its cleaner to keep these concerns separated.
Might even turn the console log output into a (toggle-or-removable-via-API) default, so it's even simpler:
using (DualityLauncher launcher = new DualityLauncher()) { // Load the starting Scene Scene.SwitchTo(DualityApp.AppData.StartScene); launcher.Run(); }
On a side note I think its a good idea anyway to make the launcher a instance instead of a static class and have it implement IDisposable so we can also move cleanup logic there and maybe in the future even start to move some state to it and move away from static state step by step.
Summary
Move the logic to start a duality game to the core library and no longer ship a precompiled launcher exe. Instead the user can call the launcher logic themselves in their own project giving them more control over their game.
Analysis
Mockup
To make custom configuration easier and better discoverable we could provide a fluent API (though this will make the issue more complex so maybe for the future?):