pop4959 / ChunkyBorder

An add-on for Chunky which lets you create and manage world borders.
GNU General Public License v3.0
47 stars 11 forks source link

Added subcommand to move existing border #14

Closed aerulion closed 3 years ago

aerulion commented 3 years ago

I'm quite frequently moving the world border (for example when resetting mining worlds) and it would be easier to be able to just move it, instead of deleting and re-adding.

pop4959 commented 3 years ago

Hi, thanks for the PR! You can actually change borders very easily as long as the selection is still present for the border you created. For example: chunky center 0 0 -> cb add -> chunky center 100 100 -> cb add would move the center from 0, 0 to 100, 100. The removal command is optional. Now of course that only works if the border was just created and it sounds like you have a special case here where you need to move the border frequently.

Your idea with the move command is good, as I've also acknowledged myself that there isn't a way to edit details of a border after having created it. However I think it might be best to generalize this so that it can be used in more situations (e.g. what if someone wants to change the radius? your code only handles the center currently). Perhaps some command to load the border into your selection would be appropriate. For example: cb load -> cb center 100 100 -> cb add to update it. This is also shorter than the previous set of commands which also required the pre-existing selection. What do you think?

pop4959 commented 3 years ago

I've also noticed that I forgot to add a license to this project, but it is licensed as GPL 3 (in line with both Bukkit and Chunky's licenses). I just want to give you a heads-up since this was just added right now, and you should be in agreement with this license before contributing code to the project.

aerulion commented 3 years ago

Sure why not, I didn't know that removing wasn't necessary (although I could have easily found it myself), I've never needed to change something other than the center, but nevertheless I think that generalizing the command is the better approach.

aerulion commented 3 years ago

But then wouldn't it be more intuitive to use /chunky worldborder to load the current worldborder settings as a chunky selection, then edit the center and add it again to overwrite?

pop4959 commented 3 years ago

Sure why not, I didn't know that removing wasn't necessary (although I could have easily found it myself), I've never needed to change something other than the center, but nevertheless I think that generalizing the command is the better approach.

Yeah, it's perhaps not immediately intuitive but the decision there was that since there is only ever one border in a world, adding a border will replace an existing one if it is already present. There's nothing wrong with removing the border first before re-adding it, but it's a completely optional step which the user can choose to take.

But then wouldn't it be more intuitive to use /chunky worldborder to load the current worldborder settings as a chunky selection, then edit the center and add it again to overwrite?

By all means that sounds like a perfect solution if you already have a vanilla world border set up which you want to match it to the selection. Many users don't use the vanilla world border though, so loading directly from the existing border would be a nice feature to have.

aerulion commented 3 years ago

if you already have a vanilla world border set up which you want to match it to the selection. Many users don't use the vanilla world border though,

Correct me if I'm wrong, but wouldn't it be possible to specify which border to load from when using /chunky worldborder, something along the lines of '/chunky worldborder vanilla' / '/chunky worldborder chunkyborder'. This would eliminate the need of an additional command.

pop4959 commented 3 years ago

It's an interesting thought, but unfortunately that doesn't work because the border addon has a dependency on chunky but not vice versa. If chunky had such a capability, then it would need to depend back on border, thus creating a cyclic dependency. The plugins would have an undefined load order, or worse, not load at all.

I don't have a problem with adding another command, though admittedly (and perhaps you'd noticed already) the implementation is a bit sloppy right now. I'll be working on improving this soon enough, so you shouldn't need to worry about it. The plan was to extend chunky's command handling once that's finalized, and currently I am still in the process of evaluating whether any changes need to be made to improve multi-platform support.

Let me know if you have any other ideas, but right now having something like a load command still seems like the best option.

pop4959 commented 3 years ago

I made a new issue to keep track of this feature request. I'm not sure if you were still planning on opening a new PR, but if not I will probably be working on this at some point.