Infinidoge / nix-minecraft

An attempt to better support Minecraft-related content for the Nix ecosystem
MIT License
172 stars 18 forks source link

Implement Velocity Proxy server module #47

Open scottbot95 opened 8 months ago

scottbot95 commented 8 months ago

Implement Velocity Proxy server module.

It's basically just a special-purpose version of the minecraft-servers module. We probably could just use the minecraft-servers module but some properties (specifically serverProperties and whitelist) don't really make any sense for Velocity.

scottbot95 commented 8 months ago

I can run the formatter. Might take me a day or two before I can get back to this. I've also seen other flakes that include a style check in the actual flake checks so you can setup some GH actions to run flake check and it will fail if the files aren't properly formatted. I will take a look at how that's implemented. If it's easy enough I might just include it in this PR as well (or a separate PR).

Misterio77 commented 6 months ago

Hi! I have some thoughts on this:

I hate to be this person but, I don't think we should add specialized modules. Minecraft network setups vary a lot (what if they want two proxies? etc). We should focus effort in improving the base module while always keeping module complexity in check. Most of this is already possible (the convenience is very, very small):

services.minecraft-servers.servers.velocity = {
  enable = true;
  package = pkgs.velocity-server;
  files."velocity.toml".value = {
    bind = "0.0.0.0:25565";
    forwarding-secret-file = ...;
    servers = (mapAttrs (_: v: "localhost:${v.serverProperties.server-port}") config.minecraft-servers.servers) // {
      try = [ "foo" ];
    };
  };
};

VS

services.minecraft-servers.velocity = {
  enable = true;
  address = "0.0.0.0";
  port = 25565;
  forwarding-secret-file = ...;
  config.try = [ "foo" ];
};

In my opinion, the first is actually easier to use and much more consistent. With it, there's no need to learn a separate NixOS option set for velocity, just combine what you already know about minecraft-servers.servers and about velocity.toml.

I don't think we should give special treatment to specific configuration files. Each server has different ones (sometimes multiple files), and we can't hope to keep up with their changes. Even server.properties and whitelist.json are only around to lower the entry barrier for people coming from services.minecraft-server, are implemented using files/symlinks (and can be safely ommited or replaced with their equivalents), and finally don't have their format fully modeled as nixos options (so they're super duper easy to maintain).

If we're sure we want a specialized variant, it should be a very, very thin wrapper around minecraft-servers.servers.