Cleanup launcher

[Barsonax]

Changed the code a bit. Initializing duality now happens in the constructor. Actually starting the game loop now happens in the Run method.

Separating these makes it possible to use the launcher class also in the unit tests which reduces code duplication but more importantly makes sure its being tested often.

Also added some more cleanup logic to the launcher class since it now has a dispose method such as removing log outputs.

@ilexp @SirePi

[ilexp]

Moving this discussion out of the code, since it's more general.

I think the style could be a bit better on this. When the type is obvious its not needed to repeat the type again. Thats just extra noise (also we ship this to our users so we should make it as clean as possible). In the above code its clear that the type is LauncherArgs as we just used the constructor.

The editor config actually even has a option for this. We just need to change csharp_style_var_when_type_is_apparent = false:suggestion to csharp_style_var_when_type_is_apparent = true:suggestion.

To get this out of the way first, I won't reject this PR if you insist on using var in those two specific spots, because I think you've good reason to choose that way. I'd prefer it the other way around for consistency reasons, which you'll probably understand too, so I'd just leave it to you to juggle the two arguments and priorities like you see fit.

---

Now to the general style guide thing and editor config change:

You're right that it doesn't add more information when you immediately populate a variable with a newly constructed value, but it does in all other cases. Having a style guide special case here, in my opinion, only leads to confusion as to where to use or not to use var. The current baseline of "just avoid var by default" is easy to remember and apply.

Editor / IDE hints aside, if we adopt a guideline based on "when the type is apparent", I fear that we may be entering a slippery slope. You're talking about this one here:

var args = new LauncherArgs();

But then there's also:

var args = this.GetLauncherArgs();
var args = this.launcherArgs;
var fooValue = 42.0f;
var fooValue = 42.0;

"Just avoid var" is dead easy to do and a clear guideline with no mental overhead, which is why I'd keep it as-is.

And honestly, I don't think this one is so bad:

LauncherArgs args = new LauncherArgs();

Sure, that one word is there twice. So what? 😄 Code is very structured, and sometimes that structure involves repetition. Us developers are used to that anyway. Personally, I think it even makes it easier to read.

[Barsonax]

Moving this discussion out of the code, since it's more general.

I think the style could be a bit better on this. When the type is obvious its not needed to repeat the type again. Thats just extra noise (also we ship this to our users so we should make it as clean as possible). In the above code its clear that the type is LauncherArgs as we just used the constructor. The editor config actually even has a option for this. We just need to change csharp_style_var_when_type_is_apparent = false:suggestion to csharp_style_var_when_type_is_apparent = true:suggestion.

First off, I won't reject your PR if you insist on using var in those two specific spots, because I think you've good reason to choose that way. I'd prefer it the other way around for consistency reasons, which you'll probably understand too, so I'd just leave it to you to reconcile the two arguments and priorities like you see fit.

I know, just running into this at that specific line of code :P.

Now to the general style guide thing and editor config change:

You're right that it doesn't add more information when you immediately populate a variable with a newly constructed value, but it does in all other cases. Having a style guide special case here, in my opinion, only leads to confusion as to where to use or not to use var. The current baseline of "just avoid var by default" is easy to remember and apply.

Editor / IDE hints aside, if we adopt a guideline based on "when the type is apparent", I fear that we may be entering a slippery slope. You're talking about this one here:

var args = new LauncherArgs();

But then there's also:

var args = this.GetLauncherArgs();
var args = this.launcherArgs;
var fooValue = 42.0f;
var fooValue = 42.0;

Actually in all these cases the IDE will suggest you to use a explicit type even with that option turned on as the type is not apparent. Methods, fields and properties could return anything regardless of their name. 42.0 is a double in C# and 42.0f is a float in C# but thats only clear if you know C#.

"Just avoid var" is dead easy to do and a clear guideline with no mental overhead, which is why I'd keep it as-is.

And honestly, I don't think this one is so bad:

LauncherArgs args = new LauncherArgs();

Sure, that one word is there twice. So what? 😄 Code is very structured, and sometimes that structure involves repetition. Us developers are used to that anyway. Personally, I think it even makes it easier to read.

I agree its a small change but it does make the code more concise and reduces the amount of duplicate search results you get when searching for a type. Its not that hard to change the editorconfig style and it does exactly what you would want.

However since this PR is not about changing the style iam gonna leave that out of this PR and move it to this issue https://github.com/AdamsLair/duality/issues/854. There might be some other style changes we would like to do as well.