Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Architecture organization #3426

Open
yangm97 opened this issue Nov 7, 2016 · 66 comments
Open

[Feature] Architecture organization #3426

yangm97 opened this issue Nov 7, 2016 · 66 comments

Comments

@yangm97
Copy link

yangm97 commented Nov 7, 2016

Ideally, binaries, configuration and user data should not be mixed together. Currently, cuberite, and most of the minecraft servers, use a single folder for everything, without much organization (ie. there's no "worlds" folder, if a server runs many worlds the root directory becomes a big mess; plugins and plugins data are stored together; etc.).
Since this is (thankfully) not a java application but instead a native, first class citizen application, it should be easy to implement it in a more organized way.

This could be useful for:

  • Docker users: currently I have mapped 22 volumes on my container... not ideal nor easy, while I could possibly be mapping 3 or so directories: worlds, settings, plugins;
  • Hosting companies and users looking forward space saving: a single cuberite binary on $PATH could host a thousand servers – this could also allow cuberite to be installed from a package manager; each plugin folder could also be symlinked between servers for even more space optimization;
  • Building a plugin package manager should be easy, since plugin "binaries" would not not be mixed with user data;
  • Upgrading multiple servers would be easier;
  • SELinux compliance (?);
  • More use cases probably exist.

I see three ways of doing this:

  1. A major overhaul of cuberite structure, making it more unix-like, storing it's binary at /usr/bin, etc;
  2. Keep the one-folder-to-rule-them-all, most people are used to this simplicity after all, but organize it such as bin, plugins, settings, worlds, data, etc;
  3. Both: storage method would be defined by compilation flag or binary flag.

There should also be a migration guide/script, for those wanting to switch between these two models.

@sphinxc0re
Copy link
Contributor

Thoughts so far? I think this is a beautiful idea @cuberite/core-team

@Pokechu22
Copy link
Contributor

I agree with most of this, but...

Since this is (thankfully) not a java application but instead a native, first class citizen application, it should be easy to implement it in a more organized way.

Language has nothing to do with it. The notchian server implementation (and bukkit/spigot) support a --universe option to change the location of the data; it just defaults to the current directory. True, things are still really messy within that location, but you can't say that it's java's fault and it mostly matches the way that the client does it. (Also, on the notchian implementation, but not bukkit/spigot, everything world-related is contained in the world folder (all dimensions included); this isn't the case with bukkit due to multiworld support).

Also, you do need to be careful about vanilla support - the actual contents of the worlds directory can't be changed since it needs to be (at least mostly) anvil-compatible. But you could put them in a subfolder without issue, methinks.

@yangm97
Copy link
Author

yangm97 commented Nov 7, 2016

Since this is (thankfully) not a java application but instead a native, first class citizen application, it should be easy to implement it in a more organized way.

I think I did not express myself very well there, what I was trying to say is that it would be very easy to do such thing since cuberite is a binary.

@madmaxoft
Copy link
Member

I have mixed feelings about this.

I've always wanted to have more order in the binary folder - have a separate folder for plugin configuration, separate folder for plugin data (-bases), possibly even modify the Lua API to disallow writing to the plugin "program files" folder.

On the other hand, I think the current way of running "portably", from a single folder, is much better than having an installation that uses some kind of OS-specific folders. It allows me to have multiple configurations of the server, try out multiple versions in multiple folders, all without the fear of one instance overwriting the data for another instance.

So I'd go with a mix - default to the portable installation, possibly with improved folder structure, and allow an installation-like operation too. There are a few questions for this, though:

  • What folder structure to use?
  • How to switch between the portable / installed mode
  • If we auto-detect mode, how?

Also we must not forget about plugins, they do make some assumptions about paths, we'd need new API to get the various paths and the plugins will need to start using them.

@PureTryOut
Copy link
Contributor

You could maybe also do a mixture of both:

  • If installed system-wide, default to looking in say $HOME/.local/share/cuberite
  • If portable, default to look in the working directory

Package managers could probably accomplish this by setting some kind of environment variable? I don't know much about this to be honest.

This base folder would then be split up in say plugins where the code goes, config where all config files go, data where the databases go, worlds where all the worlds go, etc.

@yangm97
Copy link
Author

yangm97 commented Nov 8, 2016

screen shot 2016-11-08 at 12 05 07

This is something I've scratched for the portable installation. I've put the binary on root folder instead of a /bin directory because I think it should be simpler for both, the user and the coders.

  • By default, the binary could assume it's running inside a portable folder
  • If it detects the old structure, it should migrate data automatically (maybe?)
  • If there's nothing beside the binary it could assume it's a new portable installation and generate/download default files

Then we could have a build flag for installed mode: things could get messy if we used ENV and the variable weren't properly set at runtime. This has the downside of building two binaries but is safer IMO.

As for migrating, the installed binary could have something like --import-from= and --export-to=.

@madmaxoft
Copy link
Member

What about plugins needing their own DB files (such as the Gallery plugin's database of all areas, or LastSeen's database of last online time of players)?
What about Cuberite's database files (whitelist, ranks)?
What about Cuberite's Player files?
These are neither Settings nor Plugins

@madmaxoft
Copy link
Member

@PureTryOut $HOME is not a good location for a system-wide install, which user's home will it be in a multi-user system?

@madmaxoft
Copy link
Member

madmaxoft commented Nov 8, 2016

We can do auto-detection of the folder layout based on, say, the items.ini or crafting.txt files. And we can have a command-line flag that would disable the autodetection and force a layout: --folder-layout=installed or --folder-layout=portable

@yangm97
Copy link
Author

yangm97 commented Nov 8, 2016

madmaxoft: I think these could go somewhere like (cuberite root)/data. Please also take a look on my comment on this issue regarding database management.

@ItsShadowCone
Copy link
Member

ItsShadowCone commented Nov 8, 2016

@madmaxoft @PureTryOut probably means that you install the binary system-wide but the server directory is at .local/share/cuberite

I wanna drop in my comment, that it's actually also atm possible to place the binary anywhere or run it from the PATH, for example for android the binary lies in the apps internal storage (/data/data/org.cuberite.android/files) which is only accessible by the app, but the server directory is on the external storage (usually /sdcard/cuberite) but configurable. This is also needed because android prevents binary execution on /sdcard and every other directory which is world writable (which every app and the user can access)

Apart from that, I don't really care how the server directory is structured

@PureTryOut
Copy link
Contributor

Yes, binary install to wherever the binary normally goes (/usr/bin/ on Linux), and just look for the server directory (plugins, settings, etc) in $HOME/.local/share/cuberite, or maybe /usr/share/cuberite.

@yangm97
Copy link
Author

yangm97 commented Nov 11, 2016

screen shot 2016-11-10 at 23 34 58
Played around a little more, added some more hierarchy and reduced unneeded complexity. Also added a zip from the directory above if anyone wants to play around.
cuberite.zip

Basically:

  • .cache: Just in case something needs to store temporary stuff on disk during runtime. This should not be considered somewhere safe to store data.
  • data: This should be the only place a plugin ever needs to write to, besides .cache. Each plugin has its own folder, but maybe plugins should also be able to see each other's data. Don't know if there are any privacy concerns around this.
  • dbs: If the user chooses to use sqlite (or other file based DBMS), allow the plugins to request Cuberite to create their own dbfile. Again, just like data, don't know if a plugin should be allowed to pry other's db. This could be created/deleted on demand: ie. there's no need to keep a db folder if the server is running from mysql.
  • logs: stays where it is, naming could improve a bit: use .log instead of .txt for the extension and use date for the name. Same as db, this could come and go if the users disables the cuberite logging. Why anyone would do that, you might ask. Well, some people prefer to use their own logging solutions that read from stdout and stderror.
  • plugins: should be read only by default to prevent plugins to modify themselves during runtime. But then this would prevent a plugin-based package manager to exist. If plugins go read-only, this could be a folder full of zips or git repositores/submodules, that would be nice for management.
  • settings: each plugin can have a single settings file. If a plugin needs to store multiple settings files, then these must be stored inside a folder for this plugin. And they all must use the same markup language. Also desired to be read only, aside from manual and webadmin editing.
  • worlds: pretty self explanatory. Preventing nasty plugins from writing here would be nice too.
  • root: should contain only the cuberite binary, maybe a package manager binary and of course if the user adds a startup.sh there that's none of our business. Plugins shouldn't write here too. Should they?

Thoughts? Too restrictive? Missing something? Do you need a hug?

@ItsShadowCone
Copy link
Member

I like it.

However, about the dbs: I would prefer cuberite has all databases open when it runs, of course every plugin gets its own database. When switching to mysql (for whatever reason, this can also happen during run time) the server connects to mysql, has a configurable prefix for databases and iterates over every database they have in sqlite. During iteration it means creating database on mysql, moving all content (creating tables, inserting everything) then replacing the access pointer (so plugins will use mysql without knowing from exactly this point) and then deleting the sqlite file.

And yeah, plugins config files should also be managed by cuberite, this makes sure they are in a good format where you can easily provide a webadmin interface for changes during runtime. Plugins should avoid thinking that a variable hasn't changed since they looked it up in the config the last time. Ask cuberite every time you need to use some config. (this makes reload almost every time obsolete...)

@madmaxoft
Copy link
Member

This is a bit too Unix-centric in my opinion - Windows doesn't use folder name to indicate hidden-ness, I don't quite like the dot in .cache.

There's still a few things to sort out. Where in this structure would you put the following:

  • crafting.txt (there IS a plugin that adds to the crafting recipes file - I think @NiLSPACE wrote it as a help for writing new recipes)
  • items.ini (some plugins parse this file, it's not unthinkable that some plugins may want to write to it)
  • furnace.txt (ditto)
  • favicon.png
  • players folder
  • Prefabs folder
  • webadmin folder

@PureTryOut
Copy link
Contributor

Aren't crafting.txt, items.ini and furnace.txt basically config files? I would put them in settings/ myself.
favicon.png should probably be in the root of the project, next to the executable.

webadmin, that's just a big plugin right? So put it in plugins/.

players/ and prefabs/ have to be new folders in the root I guess. Or maybe under a new folder misc/?

@madmaxoft
Copy link
Member

webadmin is not a plugin, it's a template that is used by Cuberite internally to display the webadmin pages.
favicon.png is more of a setting, too - it can be changed by the admin to anything they like.
Do we make the settings folder itself read-only for plugins, or read-write? If we make it read-only, it means that the motd.txt file (used by the Core plugin) will be in a subfolder and thus less discoverable.

@NiLSPACE
Copy link
Member

Making it read-only would also mean we can't use the webadmin to configure plugins.

@yangm97
Copy link
Author

yangm97 commented Nov 11, 2016

@madmaxoft: We would then set the hidden flag on the .cache folder so windows users don't see them too. It has to have the dot here because there's no way to set a hidden flag on a unix like system (well, macOS allows this, but linux and freeBSD don't, as far as I know).

It would be nice if the webadmin became a plugin, like I said on #3003 this would also allow plugins to show public facing pages, not just admin content.

Even though favicon.png is a setting, I guess it should go at data/ since it's a binary file. I'm not sure I understand what the prefabs/ does but I assume this and players/ should go at the root folder.

mot.txt would then be considered user data...? After configuration, the files under settings/ should never change, unless the user does so. Having this read only from plugins side would prevent some plugin from using it as storage, for instance. But then making it read only from plugins would make it impossible to change settings in-game. Maybe saying for the plugins developers "please don't be a jackass" should solve this instead of blocking write. I have mixed feelings about some of these restrictions myself.

@ItsShadowCone
Copy link
Member

ItsShadowCone commented Nov 11, 2016

Where to place the various ini files? I would suggest config files regarding the server Like crafting should be read and written only by the server and a plugin can access various contents (read and write) and cuberite handles where they are located and when it's written down. This is essentially because it preserves the state of the file, not that 5 plugins try to access them at the same time, and again allow cuberite to activate changes immediately.

And a general guidelines about what to consider as "data" and what as settings: data is what is generated during the process of playing, could also be stored in DATAbases most of the time.

@yangm97
Copy link
Author

yangm97 commented Nov 11, 2016

@Cl1608Ho the ones used as settings should go on settings/, the ones that get modified on runtime should go at data/cuberite or data/core.

@ItsShadowCone
Copy link
Member

Yeah I updated my post. Settings (also for plugins) should all be handled by cuberite and plugins get an interface, whereas data is either always stored in a database (why not) or to a file.

@yangm97
Copy link
Author

yangm97 commented Nov 11, 2016

I like where you are going with this. Just read your edit: some find it complex to deal with databases, so I guess it's nice to give them the option of using files.

@ItsShadowCone
Copy link
Member

ItsShadowCone commented Nov 11, 2016

Uhm actually I didn't mean that. I meant it to be a bit more high level, a plugin asks cuberite to store data and cuberite has the database implemented, lets say you can configure cuberite to either use files for the data backend or sqlite or mysql...... And actually all work with files is done through asking cuberite to do it

Edit: there should be command line options which data backend to use and for mysql eg user and password.... Or For passwords the option to provide them via environment variables (cause options can be seen by any user on the system, an environment variable not)

@yangm97
Copy link
Author

yangm97 commented Nov 11, 2016

I see. I think the user should be able to choose the driver for each plugin then, with sqlite being the default storage for everything. And also being able to change the default storage for everything at once (a key under cuberite's settings called default_storage would do it).

@madmaxoft
Copy link
Member

madmaxoft commented Nov 11, 2016

I don't think the discussion of the underlying storage is necessary for this issue.

@yangm97
Copy link
Author

yangm97 commented Nov 11, 2016

Better drop the dbs/ entirely. sqlite files should go under data/pluginname.sqlite then. Underlying storage is already being discussed here anyways.

@sphinxc0re
Copy link
Contributor

sphinxc0re commented Jan 6, 2017

Any more thoughts about this? Or should we move this discussion to the forum?

@yangm97
Copy link
Author

yangm97 commented Jan 12, 2017

I have changed my opinion a bit.

  • players should go under data, I hope me or someone else will make it into it’s own db, like banlist and whitelist.

  • webadmin should be embedded into the binary or become a plugin.

  • prefabs and the various .ini, .txt should be embedded into the binary too.

The cleanest way to modify these, which I don’t think many do manually, would be by loading a plugin (bonus points for not leaving left overs behind). If one still wants to change these manually, then they should create a folder called overrides at cuberite root. These files would override the ones built into the binary on runtime, as opposed to the plugin approach, which just appends something on runtime.

@yangm97 yangm97 closed this as completed Jan 12, 2017
@NiLSPACE
Copy link
Member

the Logs folder should stay as well.

Whoops, I forgot that one!

@ItsShadowCone
Copy link
Member

I would rather go for readonly plugins similar to readonly server config (readonly default plus files overwriting settings) because that makes updating cuberite easier.

At the moment I don't even check for updates of the Server directory for the Android app because any time you re-download you would overwrite the past settings completely. There should be a way to keep the settings while to just unzipping an update on top of an existing folder (and overwriting existing stuff)

@madmaxoft
Copy link
Member

@Cl1608Ho That's fixing the effect, rather than fixing the cause.
The effect is that plugins store their settings in their own folder.
The cause is that there is no proper way for a plugin to query path to where it should store settings.

@yangm97
Copy link
Author

yangm97 commented Jan 15, 2017

@yangm97 but if we're moving the config files and database files over to specific folders you don't have to worry about data-loss when overwriting a plugin folder right?

Right, but we need to break older plugins so devs update them with the new settings and data locations :P
On a serious note, that would be just further enforcing the (newly created) good practices.

@sphinxc0re
Copy link
Contributor

What is going on with this issue? I think this is a very serious and important topic @cuberite/core-team ?

@HelenaKitty
Copy link

HelenaKitty commented Feb 28, 2017

I like the idea of this, I would rather install Cuberite globally onto my server, can I suggest running a single binary as a user? Maybe a --user, or -u flag could be passed to the binary, or config option in settings.ini. You guys already have daemonization, but if we could use init to daemonize as a user then that would be great.

Right now creating an init script/systemd script is a nightmare, I was required to switch into the user to then run Cuberite.

@SafwatHalaby
Copy link
Member

SafwatHalaby commented Mar 3, 2017

I think this is a very serious and important topic @cuberite/core-team ?

This is neither serious nor important. Just a nice bonus.

@SafwatHalaby
Copy link
Member

Maybe a --user, or -u flag could be passed to the binary,

I think one could write a simple Linux-specific wrapper script for achieving this.

# if -u <USER> is detected
su $USER -c "/path/to/cuberite"

@SafwatHalaby
Copy link
Member

SafwatHalaby commented Mar 3, 2017

If we make sure Cuberite respects current directory, then for a "portable self contained config, system-wide binary" mode, one could do this:

# cuberite-wrapper.sh -u johnSmith -d /home/johnSmith/myTestWorld4
# cuberite-wrapper.sh -u $USER -d $DIR
cd $DIR
su $USER -c "/path/to/cuberite"

@sphinxc0re
Copy link
Contributor

sphinxc0re commented Mar 3, 2017

This is neither serious nor important. Just a nice bonus.

This is your opinion. In my opinion this is as serious as anything else we do for this project.

@lkolbly
Copy link
Contributor

lkolbly commented Aug 17, 2017

Okay, so I sketched out a sample implementation at https://github.com/lkolbly/MCServer/tree/architectureChanges. Basically, everything is grouped into a category, and then there's a class (https://github.com/lkolbly/MCServer/blob/architectureChanges/src/FileLayout.h) which defines paths for each category. It'll auto-detect whether you're using the new file format or the legacy file format.

Here are the categories and descriptions I've come up with so far (largely based on the conversation here so far):

class cFileLayoutProvider
{
public:
	virtual ~cFileLayoutProvider() {}

	/** The path to the world data. World data should only be accessed by
	cWorld and classes storing per-world data. You know who you are. */
	virtual AString GetWorldDataPrefix(void) = 0;

	/** The path to player data. */
	virtual AString GetPlayerPrefix(void) = 0;

	/** The path to the log files. */
	virtual AString GetLogsPrefix(void) = 0;

	/** Path to server configuration data. Files here should not be modified
	by the server, except to generate default values. These are files that
	primarily affect server operations instead of gameplay. For example,
	a file that specifies what network port the server should listen on
	should be in config. */
	virtual AString GetConfigPrefix(void) = 0;

	/** Path to server resources. Files here should not be modified by the
	server, except to generate default values. These files primarily affect
	gameplay, but not the operation of the server. For example, a crafting
	grid specification belongs here, because it affects gameplay without
	affecting server ops.
	Note that prefabs should go in their own folder, see GetPrefabPrefix. */
	virtual AString GetResourcePrefix(void) = 0;

	/** Path to plugin configuration. Files here should not be modified by
	plugins except to generate default values. Files which configure the
	operation of a plugin belong here, but Lua files and other executable
	files belong in the Plugin path (see GetPluginPrefix). */
	virtual AString GetPluginConfigPrefix(const AString & a_Plugin) = 0;

	/** Path to plugin Lua and executable files. Files here should not be
	modified by plugins, and this directory may be read-only or shared
	across multiple instances of Cuberite. */
	virtual AString GetPluginPrefix(const AString & a_Plugin) = 0;

	/** Path to plugin data files. Files here should only be modified by
	plugins. Plugins can assume that the files will be persistant from run
	to run, but must not fail if the files are absent. For example, a
	plugin which stores portal locations should assume that if the data
	files are gone, there are no portals. */
	virtual AString GetPluginDataPrefix(const AString & a_Plugin) = 0;

	/** Path to server-wide internal data. Files here should not be
	modified by the server operator, but rather by the server executable.
	Server-wide data should be stored here, per-world data should be stored
	in the world data path. For example, a database storing credentials for
	logged in users should be stored here. */
	virtual AString GetDataPrefix(void) = 0;

	/** Path to prefabs. These are files that represent structures used in
	world generation. The server should not modify these files. */
	virtual AString GetPrefabPrefix(void) = 0;
};

Thoughts?

@yangm97
Copy link
Author

yangm97 commented Aug 27, 2017

Sorry for the delay, I wanted to see it running but I couldn’t get it to compile (using macOS), so I kind of forgot to reply.

First, thanks for taking a look at this issue. Your code seems fine to me, but I’m no xoft 😝

It'll auto-detect whether you're using the new file format or the legacy file format.

Wondering if that could be extended in the future to allow vanilla/spigot migrations too.

Moving a world is not a good idea anyway, because the world may already depend on the server's specific Prefabs, or Plugins, for example.

As I read this issue again, this got me thinking: couldn’t the prefabs folder be moved to each world’s folder? Then I started thinking about the plugins dependencies (a world can have), to finally realize I was reinventing Minecraft PE behaviour packs/addon packs. I wonder if that’s an idea to be explored further. In Cuberite context that would mean something like per-world crafting, prefabs, plugins, etc. and the concept that there’s a “vanilla” behaviour pack bundled into the binary itself.

A different issue perhaps? Thoughts?

@ItsShadowCone
Copy link
Member

I wouldn't exactly ship everything with the World, but naming it would be good. So that the server outputs a warning if it loads a map without a plugin that's mentioned inside the map.

I would really really vote for two distinct config files, one default and one for overriding. This makes automated updating far easier.

@lkolbly
Copy link
Contributor

lkolbly commented Aug 29, 2017

I couldn’t get it to compile (using macOS)

I was going to take a look at that, but then I merged master and now it doesn't compile for me either. I guess I should go fix it before it gets worse.

Wondering if that could be extended in the future to allow vanilla/spigot migrations too.

To some extent, probably, although there would have to be further work done to auto-convert the config files as well. The auto-detector could be expanded so that it detects not just what the folder layout is, but whether the config files are yaml or ini (and cRoot could select an appropriate parser based on that).

something like per-world crafting, prefabs, plugins

Some things would probably have to stay per-server (plugins that operate across worlds, for instance). With things like BungeeCord, operationally it may make more sense to just run multiple servers when you want that functionality.

In my mind worlds are somewhat ephemeral - they could be created and destroyed programatically (and in fact I have a branch that does exactly that: https://github.com/lkolbly/MCServer/tree/dynamicWorlds) (imagine, if you will, a HungerGames plugin that creates a new world for each game).

Crafting, prefabs, plugins, etc. are not ephemeral, they're setup by the server operator manually (or by script).

I'm definitely no authority on the subject, though, I'd be curious what others think.

A different issue perhaps?

Yes, but still a good issue.

So that the server outputs a warning if it loads a map without a plugin that's mentioned inside the map.

...and if resource packs were ever implemented it might be nifty to have worlds that specified resource pack dependencies.

EDIT: Fixed the quotes.

@yangm97
Copy link
Author

yangm97 commented Aug 30, 2017

World-scoped plugins would make sense for instances where you want to host a survival, a creative and a minigame on the same server (though cuberite is really lean and you might want to get yourself a bungeecord network with multiple cuberite instances…).

I think the first step should be rearrange the current, portable directory structure (as @lkolbly did), then add overrides for (almost) everything, varying scopes:

  • bundled binary defaults (or a defaults folder sitting alongside the binary)
  • system-wide overrides (for installed version)
  • server-wide overrides (your average settings.ini)
  • world-wide overrides (your average worlds/worldname/world.ini)
  • binary/runtime flag overrides (cuberite --key=value)
  • environment variables overrides ($CUBERITE_PORT=25567)

(the order matters)

Do note I’m calling anything that overrides the default behaviour, be it a plugin, a setting or a prefab, an override. And of course some of these scopes apply to one but not another: I don’t think someone would want to store a plugin inside an environment variable, but I wouldn’t use the settings.ini file if I had the option to use env vars.

I think this kind of overlaps with the “Ultimate solution”, but instead of trying to read different config file syntax, the class would read stuff from above locations. Adding other backends should not be too hard (like, I don’t know, docker secrets to store the webadmin password).

Config could be identified internally like:

  • org.cuberite.cuberite.port: Settings which need to be server-wide
  • org.cuberite.cuberite.world.spawnmonsters: Set a property which should be applied to every world. Similarly, org.cuberite.cuberite.world.worldname.spawnmonsters would set only to worldname
  • org.cuberite.protectionareas.world.worldname.enable: enable a plugin on a certain world.

In order to make setting cli/env vars easier, org.cuberite.cuberite.something could be shortened to --something, the same can not be said to 3rd party settings.

File overrides (like the crafting recipes) would need special treatment. There needs to be ways to wipe/turn off inherited files/values. Having the ability to append to would be nice too. Perhaps this should work like systemd unit files?

Binary overrides (like the favicon.png and prefabs) only need ways to be deleted (maybe using a setting) and to be completely overridden.

So much for defining where to put things in a minecraft server 🤣

@yangm97
Copy link
Author

yangm97 commented Aug 30, 2017

@lkolbly I didn’t see your comment before I finished mine, but luckily it seem to answer most of what you pointed out. But:

imagine, if you will, a HungerGames plugin that creates a new world for each game

It could work perfectly fine as an world-scoped plugin. The fact that the plugin wipes region files and changes the seed every now and then doesn’t mean it can’t live the same folder the world does. You have a good point about maps which need server-wide plugins to function though.

@NiLSPACE
Copy link
Member

I'd prefer not to have such long config names. Also, World-specific plugins sounds like an big architectural change in the code.

@yangm97
Copy link
Author

yangm97 commented Aug 30, 2017

I think internal config could be aliased to much more shorter, but I don’t see how third party (read plugin config) could be different (so that no clashing occurs). I had in mind a lua table holding the whole server config, that’s why I wen’t for this kind of reverse DNS thing.

There might be better solutions though.

World-specific plugins could be temporary implemented as an alternative plugin loading directory (just like the system wide one), just for the sake of finishing this issue.

@NiLSPACE
Copy link
Member

NiLSPACE commented Aug 30, 2017

It's not just the plugin loading. Since Cuberite started supporting plugins there has always been one PluginManager instance. Now suddenly each world would require it's own PluginManager. This could give allot of unforeseen side-effects.

That doesn't mean plugin loading isn't also an issue. Where do you keep the plugins? Do we still have one plugin folder but in the config we assign plugins to different worlds or does every world get it's own Plugin folder? The latter would be rather wasteful as updating a plugin would require updating the plugin in every world. However the first one could have an issue where a plugin could conflict with itself as it's trying to load some files two or more times. Think of a plugin that uses a SQLite database. Once the plugin is loaded a second time for a different world it tries to load the database again even though it might be locked now.

Moving all the config over to Lua has the downside that we can't add comments to the config file. That was one of the reasons we didn't support YAML as configuration language: #2560

@yangm97
Copy link
Author

yangm97 commented Aug 30, 2017

Now suddenly each world would require it's own PluginManager. This could give allot of unforeseen side-effects.

I don’t get it. How is this any different than having a plugin elsewhere symlinked to the current plugin folder?

The latter would be rather wasteful as updating a plugin would require updating the plugin in every world.

If you want a plugin in “every world” then it only makes sense to add it to one of the server-wide locations described above, not in every world’s folder.

However the first one could have an issue where a plugin could conflict with itself as it’s trying to load some files two or more times.

If you get the same plugin multiple times (ex: in system wide, server wide and world wide locations), then the lowest in the chain should be the only one loaded (just like you would do with a binary file like the favicon.png).

Moving all the config over to Lua

I did not state that. Config files can be whatever (just like what’s described on xoft’s comment on #2560 ), I just said how I think that could be implemented: overriding settings from “higher up” and some additional backends, like runtime flags and env vars. Then webadmin would only write to the files (ini, nbt, sql, yml, whatever) the bits that are indeed different from the settings higher in the chain.

Another example: you could set world settings from:

  • /etc/cuberite/settings.ini (system-wide setting)
  • /srv/myserver/settings.ini (server-wide setting)
  • /srv/myserver/worlds/myworld/world.ini (world-wide setting)
  • cuberite --world.myworld.spawnpoint
  • $CUBERITE_WORLD_MYWORLD_SPAWNPOINT

Although I personally would store all world settings on NBT or world.ini (and so would the webadmin, I guess), this would be a unified configuration framework, used by the server, it’s plugins and worlds. Also the reason I came up with reverse DNS naming, to prevent collision.

I said it was inspired by the way lua tables work because I don’t know what would be the C++ equivalent to such data structure.

EDIT:
More on the configuration, a few more examples:
settings.ini attributes would have the “domain” org.cuberite.cuberite, unless set otherwise:

Description=Cuberite - in C++!
org.cuberite.protectionareas.AllowInteractNoArea=true

The same applies if you wanted to set those from cli. But if you wanted to set the same setting but using the plugin’s setting file, you could’ve simply used AllowInteractNoArea=true.

So, (for the purpose of making updating cuberite not a nightmare) some overrides are way more important than others. So I think we could focus on those instead, possibly breaking this issue in two.

What do you guys think?

@lkolbly
Copy link
Contributor

lkolbly commented Aug 31, 2017

Too long, don't want to read: The first three points are all "if you have per-world plugins, how does aliasing work?" (I'm coming from a standpoint that each per-world plugin gets its own global state) and the last point is "Unified global configuration seems cool! I haven't thought about it very hard yet, though."

I don’t get it. How is this any different than having a plugin elsewhere symlinked to the current plugin folder?

How would a world-specific plugin interact with the server-wide plugin, and vice-versa? If there are multiple world-specific plugins "HungerGames" (for multiple worlds), and a server-wide plugin calls DoWithPlugin("HungerGames"), what happens?

If you want a plugin in “every world” then it only makes sense to add it to one of the server-wide locations described above

Not necessarily. I can imagine someone lazy (i.e. me) making a plugin that uses global variables to store e.g. game status (I can imagine this because, guess what, this is unfortunately the architecture of my BedWars plugin). This is totally acceptable for a per-world plugin - if you want multiple BedWars worlds, just stick the BedWars plugin in each world, and each one gets its own global state. A server operator may not have the time or the expertise to re-architect my poorly written plugin if they want a whole bunch of BedWars worlds.

If you get the same plugin multiple times ... then the lowest in the chain should be the only one loaded

If per-world plugins are lower in the chain than per-server and per-system, this could be difficult to detect, and even impossible if runtime world creation is ever implemented, since it requires checking every world for plugins before loading any plugin. And then, if a server operator has a world that uses e.g. ProtectionAreas, but then they decide to install ProtectionAreas for the whole server and forget to remove it from the world, they may be confused.

You get the same problem if the chain is reversed.

For this reason, I think you'd have to load every plugin, and deal with aliasing somehow.

... this would be a unified configuration framework, used by the server, it’s plugins and worlds

I like! That said, I think you could get away with a terser DNS-style configurations, maybe just cuberite. for cuberite internals, world. for world configuration, and plugin. for plugin configuration (or something). I'm not sure when the org.cuberite. part changes (also, prefixing anything with org. gives me traumatic memories of knowing Java).

@ItsShadowCone
Copy link
Member

  1. The first point was about loading plugins. Totally legit

But imo the easiest thing wold be to make a setting in the world I I that allows worlds to specify plugins they depend on. No need to ship them with the world. Plus, you could specify which plugins should be executed (maybe all can be loaded at server startup but for example hooks are only executed for a certain world

@lkolbly
Copy link
Contributor

lkolbly commented Aug 31, 2017

  1. Oops, I meant the TL;DR to refer to my own post. I've never understood putting them at the end - by the time you get there, you've already read it. :/

I like the idea of allowing worlds to list plugin dependencies, though I think for backward compatibility the default would have to be that all plugins interact with the world.

@madmaxoft
Copy link
Member

I don't think we should even consider world-restrained plugins. All plugins should be written in such a way that they work in worlds where the admin has configured them to work, and not touch the other worlds. If you're too lazy to make your plugin correct, your plugin isn't worth running on such a wonderful piece of software like Cuberite is today :)

@yangm97
Copy link
Author

yangm97 commented Sep 2, 2017

I don’t think we should even consider world-restrained plugins.

I have to admit it would be a case of “premature optimisation”. Because if Cuberite ever adds support for PE (or someone hacks a compatibility layer on top of forge to bring PE features or…), then, correct me if I’m wrong, Cuberite will need to support per-world “add-on packs”.

So we could agree to call that a dead subject until someone starts implementing PE support or something.

@lkolbly
Copy link
Contributor

lkolbly commented Sep 2, 2017

If you're too lazy to make your plugin correct, your plugin isn't worth running on such a wonderful piece of software like Cuberite is today :)

Yeah, you won't want to run any plugins I write. :-)

@lkolbly
Copy link
Contributor

lkolbly commented Sep 3, 2017

I couldn’t get it to compile (using macOS)

I fixed the architectureChanges branch, now it compiles (don't know about macOS, but clang will compile it for me).

@alpharde
Copy link

alpharde commented Nov 4, 2023

News on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests