Logging via ILogger instead of Console and showing logs in UI

[shame-mm]

Description

Improved logging and error handling

Error Logs in UI mentioned in discussions of https://github.com/momentum-mod/lumper/issues/14

What was done

• Replaced Console.Writeline with ILogger
• Added Logger Factory and ILogger
  • Create log anywhere using LumperLoggerFactory.GetInstance().CreateLogger(GetType()).LogInformation("My Log");
• Wrap any code to handle error logging on exceptions using

  await new LumperCodeExecutionHelper().ExecuteAndLogError(async () =>
  {
  // Any code here
  });

• Wrapped some existing UI commands with await new LumperCodeExecutionHelper().ExecuteAndLogError to catch and log errors
• Created view in UI to show logs
  • Menu: View --> Logs
  • This view is shown when application starts
  • Error logs triggers a switch to this view

UI

Example of loading a file that is not a bsp file

[tsa96]

Won't have time to review this fully for a week or so but I'm concerned by how complex this is. Sorry if I'm misunderstanding something, but isn't this creating a new logger instance for every call to LumperLoggerFactory()?

Similarly, all this stuff wrapped in await new LumperCodeExecutionHelper() is creating a new LumperCodeExecutionHelper class every time, which itself calls CreateLogger again:

public LumperCodeExecutionHelper()
{
    _logger = LumperLoggerFactory.GetInstance().CreateLogger(GetType());
}

also, isn't GetType() there just going to be LumperCodeExecutionHelper? Displaying the name of the class an error is coming from is good (and as far as I can tell, the only reason to not just have a single Logger singleton?), but judging by your screenshots that's not actually been shown in the logs.

I'm afraid I'm not experienced with C# logging stuff - and frankly C# more experience - to give particularly informed comments here. If I had more time I'd read up more, but I won't until I jump over to Lumper work in the next couple of weeks. Feel free to post in the #tooling channel on our Discord if you want to discuss this more casually.

[tsa96]

Oh also, ExecuteAndLogError absolutely should not be async when getting called with non-async stuff. We shouldn't be making existing non-async methods async just because a method they're called is async beside it might have to await some Task.

I don't know if C# lets you override an async method with non-async one or vice versa, if so I'd do that. Otherwise just have two differently named methods.

[tsa96]

Heads up that I'm working on Lumper at the moment and also intend to integrate logging into the UI. I'm not sure I can may use of these commits in there current state, but I'll may end up taking some code from them at some point, I'll let you know if I do.

That being said, even if we can't merge since we really appreciate the work and you're welcome to contribute more in the future, though it's best to let us know what you intend to work on first so we can avoid overlap with others. If you don't have a Momentum key already already feel free to contact me (either Discord or email on my Github is fine) and I'd be happy to give you one!

[tsa96]

Okay, my work has fulled superseded this at this point, so going to close this. Again, sorry we couldn't make use of this!