Add disable-general-commands to Bukkit config

[codeHusky]

As an alternative, or supplement, to PR #2041, this Pull Request introduces disable-general-commands, which can be used to disable the basic commands WorldGuard adds.

Functionality of this change was verified on Paper 1.20.2 git-Paper-280

Resolves most of issue #2013, albeit not terribly elegantly on its own. Still good to have IMO.

I should note that this PR may contradict the messaging in the README.md of "Enable only features you want! Everything is off by default" - can be revised to address this as needed.

[Jjman739]

Is there any particular reason this never got merged or reviewed? I know it's not completely necessary when namespaced commands and commands.yml (if it worked with autocomplete) exist, but it would still be good to let server owners choose whether to have these potentially unwanted features that aren't part of the plugin's core purpose.

[wizjany]

because it was discussed at length in the previous (linked) PR, and neither PR fully addressed the problems.

[codeHusky]

@wizjany It's never been explained how this PR or any other PR like it could be reasonably changed to be merged. If the issue is simply to just leave this config option off by default, that's a less than a line change reasonably to do. Without proper communication neither PR can ever move forward.

[Jjman739]

I reviewed the discussion in the other PR and it's unclear what the issue is. If codeHusky's code works as stated, then there's no problem for existing servers to update without breaking anything, and even if it should be on by default, it's still good to have the option to turn it off.

If the issue is the interpretation of "everything is off by default" and the fact that commands like /god are an opt-in to non-vanilla behaviors, I can see that logic, but it doesn't mean that having a config toggle for the commands is antithetical to the plugin. And I'd also like to bring up the case of /locate, which is why I care about this. It's overriding a vanilla command, so it's not a matter of accommodating other plugins even. If WorldGuard's /locate is overriding vanilla's /locate until the player goes into commands.yml to change it, then that promise of "everything is off by default" is not being kept.

If there's an issue with the execution or the philosophy behind it, please share. Though if I may, I'd like to suggest that problems with the philosophy behind it should be discussed under the issue, not the pull requests to fix it. Unfortunately, that's not possible because the issue was locked as spam.

[codeHusky]

Considering this hasn't gone anywhere in coming up on 2 years, it would probably be a better use of time to simply fork the project or use a different plugin if you'd like a way to turn these commands off.
Disabling the GeneralCommands class's registration just be a singular config option and added to the hasCommandBookGodMode condition check in WorldGuardPlugin, but that solution could've been implemented in less time than it'd take to read this message by anyone with push access by now, so I'm doubtful it'll happen.

[me4502]

My understanding of the specific issue wizjany had with these PRs is that it changes the default behaviour of the plugin. Having it only apply to a "fresh install" doesn't mean it's not changing how the plugin behaves by default.

[codeHusky]

@me4502 The original discussion was about various changes in how commands were handled in Paper / WorldGuard meant that previous behavior (WorldGuard's commands being overridden automatically by Essentials, just go back to older versions of server software and try) was no longer the case. It was being argued continuously that reverting back to the way it was would be changing behavior (which is only technically true), and in general, the conversation did not go anywhere.

If a simple PR just adding a config check would be accepted then IDK why the contributors involved didn't (1) see a demand for it and (2) do it without forcing someone to do a pr (3) without waiting years for it to happen. I'm happy to do it myself if it's actually desired and will be merged, but I'm not going to do it if its just going to sit and never be merged for over a year without any direction or constructive feedback.

I'd prefer something like this is merged because most admins (IMO) aren't expecting WorldGuard to add random commands unrelated to region protection, but that's again up to the contributors to decide and if anyone ever just said "We just want them on by default, but having the config option is fine" then this could've moved along months ago.

[Jjman739]

If they're not going to answer, I'm in support of a fork if anyone wants to do it, but I'm not going to do so myself because I don't have the time and experience with this codebase to maintain it. I only found out about this project a week ago, and I'm only here because "everything is off by default" was a big part of the reason I chose this one, so I don't like that a vanilla command is being overridden by default without a way to disable it.

[aurorasmiles]

If a simple PR just adding a config check would be accepted then IDK why the contributors involved didn't (1) see a demand for it and (2) do it without forcing someone to do a pr (3) without waiting years for it to happen. I'm happy to do it myself if it's actually desired and will be merged, but I'm not going to do it if its just going to sit and never be merged for over a year without any direction or constructive feedback.

We do see a demand, and I (personally) would also be okay with merging a PR that just adds a simple config check. The reason why no one has touched this yet is because ideally we would want to deprecate those commands, actually announce that they will be removed in a future version, and then remove them at some point, potentially with a bigger update like WG 8. That requires actually working on a future version - everyone is just burnt out - as well as actually announcing it