add /restrictsubclaim #135
Conversation
|
Just wanted to say thanks for working on this feature, not being able to restrict access in subclaims has definitely been something players on my own server have talked about having a problem with. |
|
Great, I might have time this weekend to finish it. I've been using it on my server for months, I just finally found the motivation to PR it and finish the other database type. |
|
I added database storage support, and decided to only allow owners to use /rsc. This should be completed now. By the way, the source code is a mess of tabs and spaces. It makes it hard to understand when you have tabs set to 8 spaces (I usually do 4 but the editor I was using just happened to default to 8). I used tabs here to match the code around what I was editing, but you should probably make sure there is some standard about what to use. |
|
This looks good and sane to me. It'll want careful testing, but the fact you're already using it yourself is reassuring :) Thanks for your work on this! Also, regarding the mix of tabs and spaces (and coding style in general), yes, it's a mess, I believe that's all going to be sorted during the ongoing refactoring job. |
|
I know you're using it, but I'm wondering if you can find some other servers (or use a backup) to test the conversion process, since you do convert both databases to a new schema... |
|
I created a test server to test the conversion process when I wrote it. It seemed to work fine. There were only a few claims to convert but it created the new column and set it to the correct value for the restricted subclaim. There is no conversion process for .yml, I think because of the way that works it will default to false if it isn't already there. I don't remember having any issues when my server was converted months ago. |
|
Yea, will need to have some flatfile testing - not sure how it's going to react with a mix of old and new flatfiles... |
|
I just checked my flat files, most of them don't have "inheritNothing: false" in them. Even though it works completely fine without that there, I will add a conversion process to them for completeness (sometime later this week). If anyone else wants to test this or try it out, I put a compiled .jar here: http://mc.starcatcher.us/GriefPrevention.jar |
| TooMuchIpOverlap, | ||
| StandInSubclaim, | ||
| SubclaimRestricted, | ||
| SubclaimUnrestricted, |
There was a problem hiding this comment.
Trailing comma here, does this compile?
|
Turns out it was already fine, I just did some testing. When upgrading the flatfile schema from 2 to 3, it already regenerates all the .yml files. This adds the inheritNothing = false to all of them So I don't see any issues, everything worked fine in my testing. |
|
Is this going to be looked at or merged soon? It is complete. |
|
Mind adding documentation of this command to the wiki? |
|
OK, added to the Commands page |
|
As soon as I have a few moments to spare, I'll build & deploy to my test server after backing up the claims data flatfiles, and check the end result seems sane. If I feel particularly brave, I may even try out a proper DB-backed store migration, too (I'd like to switch to that eventually, anyway). So, sorry for the delay, but it's not being ignored! |
|
(I should add that I don't speak for @RoboMWM - whether it's merged or not isn't my call, I'm just offering to do some additional testing so I can confirm that it worked well for me - the more testing the better :) ) |
|
I've been busy and will be for the summer, hopefully I'll be able to look over this on the weekend (though from a brief look, it seems ok). Either way, it will be looked over before the next release. |
Will update databases - warn server owners to backup when releasing (they should be backing up anyways...)
Right now you can add extra permissions to small areas in claims using subclaims, but there is no way to make more restrictive permissions. If you want to limit access to a certain chest on a shared base, for example, there is no way to do that.
This adds /restrictsubclaim (/rsc), which can be used to make a subclaim inherit no permissions from the parent claim. You can then add in the players you want to have access to the subclaim. It has been very useful on my server, for large team bases with 10+ members, and also for locking specific chests in public farms.
Even though this PR has been in use on my server for a while, it is unfinished. It doesn't support database storage, support for that would need to be added before this is merged. There's also some unanswered questions about who exactly should be able to use /restrictsubclaim, right now anyone with permissiontrust can use it, even though only owners have any permissions on newly created restricted subclaims (this is probably a bug)