Obsidian icon indicating copy to clipboard operation
Obsidian copied to clipboard

Obsidian now uses a Generic Host to run the Server.

Open XanderStoffels opened this issue 2 years ago • 3 comments

Hi all,

This PR closes https://github.com/ObsidianMC/Obsidian/issues/279.

  • The Console App now uses a generic host.
    • Add Obsidian to the host using services.AddObsidian(IServerEnvironment), imported from Obsidian.Hosting.
    • Because we don't want to limit Obsidian being only able to run in a Console App, an instance of IServerEnvironment has to be passed. In here you can find the server configuration, saved worlds and an option that is used to indicate if the generic host should stop running if the Obsidian server stopped running. Server shutdown and crash events can also be handled here.
    • A default IServerEnvironment that assumes there is a file system and console input can be found in Obsidian.Hosting. Currently, the Console App uses this one and contains the previous logic that was already there before this PR. Can be created by calling IServerEnvironment.CreateDefaultAsync(ILogger).
  • Logging configuration is now done by the application that wants to run the generic host, in this case the Console App. While building the generic host, logging options are provided. The startup application is responsible for what kind of logging it wants, so Obsidian.Logging was moved to Obsidian.ConsoleApp.Logging. This makes Obsidian independent of what kind of logging the startup application wants to use (Console logging, File logging, etc) by just using Microsoft's injected ILogger and ILogger<T>.
  • Fixed a small bug that would occur upon saving the world during server startup.
    • World.cs:317 worldFile.CopyTo($"{this.LevelDataFilePath}.old"); would crash because the file already exists. Changing it into worldFile.CopyTo($"{this.LevelDataFilePath}.old", true); fixed it.
  • Partially rewrote RconServer to be compatible with the DI framework.

Next steps for a following pull request would be to let the Server's services being injected by the DI container. I already took some preparation for it in Obsidian.Hosting.DependencyInjection.

Let me know what you think. 😄

XanderStoffels avatar Sep 17 '22 04:09 XanderStoffels

I am wondering, how well does this still work with docker? I'm down to update the dockerfile and docker-compose for these updates.

for example; is it still possible to define a different working directory?

Naamloos avatar Sep 17 '22 21:09 Naamloos

never mind, considering we just call Directory.Exists("config") for example this should point to the current working directory. I'll finish my review in a bit.

Naamloos avatar Sep 17 '22 21:09 Naamloos

Some feedback from @Seb-stian on from Discord has been addressed:

Config → ServerConfiguration is nice 👍 But then using cryptic variable names (c, w, cToken, fs, env) and implicit var (where the type is nowhere to be seen) is... meh DefaultServerEnvironment is in a file DefaultServerStartup.cs And its async methods don't end with Async suffix

XanderStoffels avatar Sep 19 '22 04:09 XanderStoffels