But it’ll work on a pre-generated database yeah? I think Nucleus’s default perms were messing up too with LuckPerms, but I assume this build will fix that too.
Yes. Default permissions do not interact with the database in any way. They are never saved or read from storage.
Defaults can be changed in PEX. The constants I have in GP are the starting values. An admin can change a default with command in game or directly in json. In PEX, defaults represent the default user that all users inherit from and is always persisted. The documentation definitely needs some work.
To add onto this,
So it’s definitely implied that parent nodes should be granting the child nodes.
I’m aware of that. Trust me, I’ve read the JavaDocs for this stuff over and over.
My point was, it’s not clear where that should be resolved. It’s a common thing throughout PermissionService. When you call a #hasPermission method on a Subject (for example), should it include inherited permissions? Should it resolve wildcards? Should it just include the permissions the Subject explicitly has? Whose job is it to apply wildcard rules? Is that down to the plugin who implements PermissionService, or the plugins checking for permissions?
That’s my point. The JavaDocs are very unclear and unspecific in some places, and you’re forced to just guess. People are going to keep having issues with this part of the API unless the documentation is improved. I’m not even expecting other people to do it, I’ll happily do it myself, once we’re agreed on what is what. You can’t have cases where what a method does is defined by how PEX implements it. It should be very clear.
You can do that with LuckPerms too. However, instead of removing the default permissions directly, you just override them.
I think that’s a much better approach because, some developers might not respect that admins might change the Default permissions, and just think that they need to apply their Defaults to the default subject each time the server starts.
I know you haven’t done that in GP, but it’s certainly not the logical thing to do. It’s also not documented!
1 Like
Fair point. but to answer the question,
Yes, Yes, No, The permission service.
Most plugins querying the service arn’t even going to be aware of permissionnode structure.
I’m aware there are other issues in the API though, Here is an issue to track them.
2 Likes
May you rise to the challenge of this ancient mystery
SpongeDocs in the balance, information must be free!
Thanks for getting some focus onto this @Luck and @ryantheleach
So any default changes in game will be persisted? Because if not, GP will break. Also GP doesn’t wipe default perms, it just adds to it. It also verifies that all defaults exist on startup and will reapply anything that does not. A feature I requested to PEX in the past was to allow plugins to store any defaults they have into a separate file to not clutter the original (assuming file mode).
Props to you for actually doing something about it. <3
So I guess the general rule is that methods querying if a Subject has a permission/option should always be checking the parents, applying “wildcards” and such, and then the methods in SubjectData query a user’s own data, and do not resolve inheritance? If that’s the case, getting it into the JavaDocs would be a great help.
I’m aware of that, but again, by point was, where is this documented? I said in my post, I know GP doesn’t wipe or override default perms, but what’s to stop other plugins from doing it? It’s not documented.
Changes made to defaults are not persisted, no. You told me this was ok. Again, the JavaDocs has no mention of this.
Currently nothing stops plugins from messing with defaults but that is an issue with the API. Easiest fix would be to require a plugin container to set defaults. With a plugin container you could isolate plugin defaults and avoid what you stated above. Feel free to PR a fix.
As for my fake user comment. I was referring to GP’s usage of it. I don’t persist any fake user info on startup. I merely use them as dummy users for protection.
So for now, how about simply isolating default permissions if set via API by the node. For example, all my defaults start with “griefprevention” so you could simply store this in a file called griefprention.json. Any calls to modify GP defaults in game would be persisted in this file. Thoughts?
1 Like
I’d really rather see that defaults get assigned to a role using custom PermissionDescriptions, then the role subject assigned as a parent to the default.
It makes managing and overriding it in the permissions plugin a lot easier, as inheriting from a role means that the plugins defaults will always come second to what the server operator has set as the defaults.
Also how are you setting the defaults? getting the users default subject(for a context), and setting the values directly?
Uhh idk.
My thoughts would be to keep “defaults” completely separated from the “default” group. At least in LuckPerms, every user is placed in the default group when they join for the first time, but they can be removed from it.
However, “defaults” are applied to all users, whether they’re in the “default” group or not. I don’t see why “defaults” can’t just be transient, allowing admins to override them by setting permissions to parent group subjects.
The issue is especially true when you have per-server permissions and whole networks of linked instances. I’d rather not bloat the “default” group with pre-set permissions from plugins. I just don’t see the need.
If plugins want to allow users to modify what’s assigned by default, then admins should either override the defaults in their permissions plugin, or plugin authors can provide some sort of configuration option.
You raise very good points.
As far as I know, (again, this part of the API is very confusing), there’s literally no use to PermissionDescriptions currently? There is absolutely no documentation about how plugins implementing PermissionService should use PermissionDescriptions. They seem to overlap with Defaults, without any real explanation of their purpose.
They have no affect on permission checks, so what’s the point in them?
https://github.com/SpongePowered/SpongeAPI/blob/bleeding/src/main/java/org/spongepowered/api/service/permission/PermissionDescription.java#L33-L36
The whole point of handling defaults for users is to make it user friendly. If GP didn’t use contexts then I would agree but with contexts, permissions are not as easy to manage for end-users. There is also no way for me to provide a working “default” file due to contexts. Transient is fine but I still have an issue when an admin wants to override these defaults. Requiring them to add it manually is cumbersome with contexts.
The only other alternative would be GP keeps track of defaults in its config and force applies them each startup. Since defaults are transient in LuckPerms, this shouldn’t be an issue. I’ll make this change in GP soon which should solve the defaults issue.
I believe that the latter would be the best approach. (I’m even happy to attempt to PR it into GP myself.)
And that’s exactly why, in my option, they shouldn’t be persisted. If I merge “defaults” with the default group, then the end user will see all of those permissions set by plugins.
Actually persisting them locally is no big deal, it’s just writing out some json when the server loads/starts. Either way, this should be clarified within the SpongeAPI / javadocs. I see both options, and can see the logic behind each one. However, they should either be transient or persist, so plugin authors know how they should register their defaults. 
So I started to work on changing how I handle default permissions in GP and came across a stumbling block. Currently, whenever a user uses the /cf command, the permission/value/context get stored in the global subject(defaults).
Here is how it would show up in PEX if a user typed the command
/cf entity-spawn pixelmon:trader false
“contexts”: {
“gp_claim”: “e7ae9257-cea4-455e-adec-2c42d2d19269”
},
“permissions”: {
“griefprevention.claim.flag.entity-spawn.pixelmon.trader”: -1
}
If I store this in a config, it could get messy fast. I also didn’t want to have defaults in config and everything else stored in perms. I would rather all permissions be stored in one place if possible.
Here are 2 alternatives
-
As I mentioned before was for LuckPerms to isolate any defaults set by a plugin until Sponge adds support for it in API. This feature could be flipped on in LuckPerms config. For example, you could have a config option “isolate-plugin-defaults” which could be a list that accepts plugin ids. Any plugin id specified in this config option would store all default permissions containing the plugin id in an isolated location to avoid bloating the global default subject.
-
GP adds a config option to specify the default group to use then reference that instead of the default subject. So if a user typed /cf command, I would use the default group set in GP’s config and set it in permissions. The only issue with this option is it would assume the user followed directions and made sure GP’s default group was inherited properly. Option 1 requires no user intervention.
Understood, however, this all stems back to whether defaults should be modified or not. Again, sorry to keep repeating it, but I’ll say it again. It is my opinion that defaults should not be persisted, as it leads to issues with plugin authors overriding any changes made to defaults. However, it is clear that whatever the decision is needs to be documented. It is trivial for me to implement persisting defaults in LuckPerms, however we need to agree on a standard.
In most plugins, I do not see the use case for overriding them at all, which is why IMO, it probably makes sense to make the SpongeAPI easier for the majority, who just want to set defaults every start up and forget about it, rather than to
I can do the same as in PEX, and keep the global default subject completely separate from the default group, it’s not an issue at all. That completely gets rid of the “bloating” issue.
So, what needs to happen is:
- Sponge as a collective decides whether Defaults should be persisted. It needs to be added to the JavaDocs either way, with a warning to plugin authors to not override them, if it is decided that they should persist.
- If they should persist, I’ll implement it in LuckPerms, and you won’t have to make any changes to GP. However, if we decide they shouldn’t, you just make one of the changes above.
To add on, you storing default overrides in a file is going to be less messy than me storing all defaults in a file. You will only have to persist overrides, I would have to save everything.
I agree, it would make sense to use your Option 1 above - less work for the end user, however, depending on what’s agreed in the API, I can just implement saving / loading defaults, and you don’t have to do anything. 
I’m just going to wait on an answer for now, and I think you should probably do the same, to avoid wasted time for both of us. Again, I’m just pushing for a standard, not trying to be purposely difficult. 
I mean, another idea, is just to properly document, within the #getDefaults method, that plugins should choose carefully as to whether they choose to modify the transient subject data or the normal subject data. That would elegantly solve this whole issue, however it still needs to be documented, either way.
Why not use the #getTransientSubjectData call here?
https://github.com/MinecraftPortCentral/GriefPrevention/blob/master/src/me/ryanhamshire/griefprevention/FlatFileDataStore.java#L220
… and then save any modifications made by admins with commands to the permanent subject data? Seems like a good solution, just needs to be documented.
@zml wrote our permissions API and persists defaults when calling getSubjectData in PEX. Due to how he handles it in PEX, I’ll assume he intended any subject to persist if calling this method. The getTransientSubjectData was added to guarantee it would NOT persist. However, the docs do state it is up to the service provider to persist the data when calling getSubjectData. I’ll have zml verify when he is around because it is confusing. As for PermissionService#getDefaults, how does this sound
/** * Get the subject holding data that is applied by default to all subjects. This * subject is at the root of all inheritance trees, above even subject type-specific * defaults, meaning it has the lowest priority when all other weighting is equal. * * <p>Note: Plugins that add permissions to this subject need to be careful to not * override permissions already set. It is also recommended to use * {@link Subject#getTransientSubjectData()} where possible to avoid persisting * unneeded data.</p> * * @return The default subject data */ Subject getDefaults();
As for MinecraftPortCentral/GriefPrevention/blob/master/src/me/ryanhamshire/griefprevention/FlatFileDataStore.java#L220
I’ll change this to transient.
Also, feel free to PR any API changes you think are needed. We can look it over.
Ok. I’ve PR’d some changes. Clarify some parts of the PermissionService documentation by lucko · Pull Request #1387 · SpongePowered/SpongeAPI · GitHub
I’ll start work on persisting defaults in LuckPerms now.
Defaults should be persisted in the latest build, which will hopefully solve the remaining issues with GriefPrevention.
https://github.com/lucko/LuckPerms/commit/5b4975f34c6b6bcaff47aa57d59cbc316fd10296
It’s somewhat untested, but should work fine.
2 Likes
Hey do you have a command for setting how many homes a group can set? I messaged Hassan about it, one of the Dev’s for Nucleus. He said this was an issue with your plugin.