Bentobox: Reload configuration

[SlimeDog]

BentoBox 0.16.0 and previous

Please consider supporting /bentobox reload for configuration file. This would also require that configuration not be rewritten on shutdown. It would make configuration testing a lot easier.

[Poslovitch]

Gonna work on this.

• [ ] Split config handling from YAML database
• [ ] Add the configuration backup
• [ ] Add the reload

[tastybento]

@SlimeDog Clarification question: is this a request for bentobox reload to reload the BentoBox plugin config, or try to reload all the addon configs as well?

@Poslovitch The latter could be very difficult because addons load their configs independently and may not actually use the BentoBox config system, e.g., they just use the Bukkit config. I've tried to look at doing this before and I thought of adding an interface to the Addon class called "reload" that would be called by BentoBox when this command is executed so each addon can decide how to react. Some may not be able to do any reloading, some may. What do you think?

If this request is just to have BentoBox reload a config itself, then this may be simpler.

In regards to splitting config handling from the YAML database - what is your thinking there? We do already have a separate Config class that abstracts the underlying loading and saving. The reasons it uses the YAML database code is to avoid code duplication (YAML is YAML) and to enable comment preservation when saving configs. I'm open to better ways, but I'd like to understand why code reuse is not a good approach in this case.

[SlimeDog]

I would start with _BentoBox/config.yml_. That is the main one that is going to be changed for testing.

[Poslovitch]

I'll implement that optional reload method for Addons, so that they can handle their reload themselves.

@tastybento Regarding splitting config handling from the YAML database... Well, code re-use is good, but the YAML saving/loading gets mixed up with config specific features which make the code harder to read. Maybe splitting the YAML methods in a few more methods so that we can override them at some point would be a more viable solution than code duplication... If I've got enough time today, I'll submit a PR so you can have a look at it.

[tastybento]

@poslovitch Okay.

[Poslovitch]

Okay, I thought about it a bit and I wondered that we were saving the updated config when disabling BentoBox. What if we were actually doing that when enabling BentoBox?

Basically, we'd save it just after loading it. And, thanks to how the database api works, the config would be updated. This way, we actually wouldn't have to save it when disabling BentoBox, therefore making the config actually editable when the server's running.

@tastybento May I ask your opinion on my idea above? And do you confirm that "reloading" the config consists in loading the config when running the reload command?

[tastybento]

@Poslovitch Yes, what you say sounds correct.

[tastybento]

By the way, this is also good practice for addons, i.e., save the config immediately after loading it and do not save it again onDisable unless there's some reason for doing so.

[SlimeDog]

100% yes.

[Poslovitch]

I'll push the commit soon, I'm not a 100 % sure it's the correct handling. @tastybento Could you have a look?

[Poslovitch]

Okay, it seems to be working.

I could switch the default from en-US to ja-JP then back to en-US again. It looks like it worked fine. Committing!
(And the config file is no longer resetted at the server shutdown - this might require us to change the config header ahah).

@tastybento Feel free to revert the commit and/or to fix it if it is causing critical bugs. From what I've tested, it's working fine, but we all know these good ol' bugs are pretty good at playing hide and seek.

[tastybento]

@Poslovitch Looks good!