Added group nesting (super groups and sub groups)

[goeppes]

Lightly tested group nesting, it seems to work fine. I was unable to effectively test inherited members by myself. Also, I ended up refactoring some things when I wasn't thinking. What do I need to do now?

[CivcraftBot]

Can one of the admins verify this patch? Type 'ok to test' to test.

[Maxopoly]

Havent looked at the code yet, but you'll need to resolve the merge conflicts first and then we can have it build a test jar.

[Maxopoly]

ok to test

[Maxopoly]

Overall it looks pretty good, I'll test it once I get home. While it's appreciated that you refactor some of the methods to make them easier to read, could you not just reformat whole files? It makes it unnecessarily hard to find out what you actually changed.

[rourke750]

@Maxopoly I'll leave it to your discretion to merge.

[rourke750]

@tyrothalos When you get the chance and finalize and before @Maxopoly merges it, can you squash this so its only one commit.

Also thank you for doing this.

[Maxopoly]

Sorry, I forgot about this completly. Just tested it and linking/unlinking etc. seems to work fine. Only problem is that you renamed some of the permission methods, so all other plugins, which use NameLayer permissions (Citadel, Bastion, JukeAlert) are now broken and I couldnt really test the permission inheritation there. Could you fix that?

2 example stacks:

Caused by: java.lang.NoSuchMethodError: vg.civcraft.mc.namelayer.group.Group.get PlayerType(Ljava/util/UUID;)Lvg/civcraft/mc/namelayer/GroupManager$PlayerType; at com.untamedears.JukeAlert.util.Utility.isPartialOwnerOfSnitch(Utility .java:84) ~[?:?]

Caused by: java.lang.NoSuchMethodError: vg.civcraft.mc.namelayer.group.Group.getPlayerType(Ljava/util/UUID;)Lvg/civcraft/mc/namelayer/GroupManager$PlayerType; at vg.civcraft.mc.citadel.command.commands.Fortification.execute(Fortification.java:64) ~[?:?]

[Maxopoly]

So all the linking etc. seems to work, but it doesnt seem to inherit permissions. Is that intentional?

[goeppes]

No, it's just that I completely forgot to change the isMembers methods to check the inherited member list instead of the current members list. I'll fix it in a few hours.

[Maxopoly]

The permissions still seem to be broken. I reinforced blocks on a supergroup, added someone as a mod to a subgroup and he still couldn't bypass the reinforcement. Also the player type shown when running /nllm or /nllsg is wrong, it showed everyone as admin, even though noone actually was an admin.

When someone is on a supergroup and I want to invite him to a subgroup to effectively give him more permissions I also can't do that, because it thinks the player is already on the group.

[goeppes]

First, players on a subgroup but not the supergroup should not be able to access anything from the supergroup. Only members and their permission rank are inherited. Do you mean you added someone to a supergroup and they could not access stuff on a subgroup?

Second, owners of subgroups are demoted to admin while the subgroup has a supergroup. This is to prevent subgroup owners from messing with groups further down the subgroup tree without the owners of the top supergroup saying so. If other players are getting demoted or promoted, I can just remove this since its not really necessary.

For the last thing, I'll just need to make some separate functions to check the members of the immediate group instead of all members. Will probably need to add another command so you can see the two different member lists. Also, after this is fixed the permission of the higher rank for a player will override the lower rank.

Since I don't really have a way of testing using multiple players, I was expecting bugs. Sorry about this.

[Maxopoly]

Yeah, nothing to apologize for, it's completly fine.

So what I did was I created 2 groups, one was called Super and one Sub. I made Sub a subgroup of Super, reinforced stuff on the group Super, then added someone to the group Sub and he could not access the reinforcements. Thats not intended right?

And the problem with the admin rank was that someone I added as a mod was also being displayed as an admin (at least on the stat commands, I didnt try whether he actually has the permissions).

[goeppes]

So it sounds like first case is working as intended. Players on the Super group can access the Sub groups as if they were extensions of the Super group. However, players on the Sub group cannot access the Super group.

I've removed the admin rank thing. If it's needed later then it can be taken care of then.

I've also added a new command "nllcm", which lists the members of the immediate group. "nllm" should list all of the members that have access to the group.

[Maxopoly]

So it seems to work now, but for some groups it would tell me that I dont have permissions to do that for the subgroup. Both groups had me as the only member and owner and no other links.

[goeppes]

Hmm... What were you trying to do with the subgroup?

[Maxopoly]

Just linking it, I ran /nllink ab abc and it told me I dont have permission for the subgroup, even though I owned it and it wasn't linked in any way. Might be a persistence issue, because that was for a group which already existed before, when I created 2 new ones it worked.

[goeppes]

Could you check the list of owner permissions for the subgroup and see if LINKING is there? I had trouble following how permissions are managed so it might be that they are not being saved properly.

[Maxopoly]

The commands for that don't work in your current version, because you forgot to update the plugin.yml with the new commands. You need to do that every time you add/remove a command

[Maxopoly]

Yep LINKING is missing for those groups, even the owner group doesn't have it.

[goeppes]

Hmm. Well, I'm not quite sure what's going on. Unless something goes wrong when the permissions string is tokenized, I'm sure the default perms including LINKING are being saved to the database on group creation, but I don't know when or why it's being removed. I'll try and figure it out within the next week.

[Maxopoly]

Oh and one other thing, commands that use permission levels (owner, mods, etc.) now need the full capitalized spelling so it works, could you change that back? Probably not an intended change, just noticed that.

For example "/nlip Tyrothalos SecretGroup mods" wont work now, but previously did.

Also again thanks for your work so far.

[goeppes]

So I can't seem to replicate this issue myself. Do you remember if you made the groups before testing linking? More specifically, before I squashed the commits together? Old groups from before then won't have the new permission, while newer groups should.

[Maxopoly]

Now that you make me think about it, I might have made those groups on another shard, where the NameLayer version wasn't updated, so it doesn't have the permission. My bad.

There might still be bugs here, but we're not gonna find those by testing it on our own. I'm just gonna merge this and we let any testing happen on our test server for 3.0, which should go up in the next few days. Thanks for taking the time to add this feature.

Do you want to make a writeup explaining how this works or should I make it? I'm fine with both, but we need some sort of docu to explain it to people.

[goeppes]

I can make something. Where should it go?

[Maxopoly]

Add it to the wiki here on git