LuckPerms | An advanced permissions plugin

Plugins Plugin Releases
116

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

  1. 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.

  2. 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.

117

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:

  1. 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.
  2. 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. :stuck_out_tongue: 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. :slight_smile:

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.

118

@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.

119

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.

120

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
121

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.

122

Excellent! Thanks for the support. I’ll make the transient change tomorrow.

123

I’ve reported it in an issue to try to explain what we need.

https://github.com/lucko/LuckPerms/issues/23

124

GP flag defaults will no longer persist.

https://github.com/MinecraftPortCentral/GriefPrevention/commit/cd98d9ca29a5370f017921c0358772b8e2d4bbf3

Hopefully that helps your end =) Thanks for the tip.

125

I seem to be having a memory leak? Or maybe it’s something else. But it seems that player data inside luckperms, is randomly being deleted. Causing a player to lose anything attached to them and any permission they used to have. It’s been around once a day this seems to happen, no error no nothing. Just the player data being deleted. They still remain in the uuidcache

126

I’ve just updated to the latest for Luckperms(2.11.21) SF(1798) Forge(2105) and GP(2.1.0.82). Talking with blood about some default issues, I can apply the changes in flags and it changes the default permissions file and the flag looks like it changes. But nothing get’s applied. I’m using the file format.

I’ll let you know if the player data issue happens again using the latest LuckPerms
^ It just happened again and deleted my player data for the second time today. I was doing nothing but chatting on the server.

127

Very odd - I suggest you roll back to a build from a few days ago.

I’ve disabled CI builds for the time being, as I’m currently rewriting a large proportion of the plugin, and the resultant builds are very unstable. I’ll look into those issues as a matter of priority once I’ve finished.

(just to add that I’ve had no reports of similar issues, so I suspect that it’s an issue that was introduced in one of the more recent builds.)

Additionally regarding the Defaults, I maybe didn’t stress it enough, but it was completely untested.

I guess it would be sensible if I linked to a stable build on this page instead of the CI server. :stuck_out_tongue: I’ll change that when I have a chance to as well.

128

@Luck The issue seems to be here

https://github.com/MinecraftPortCentral/GriefPrevention/blob/master/src/me/ryanhamshire/griefprevention/GPPermissionHandler.java#L257

PEX returns the proper value but LuckPerms doesn’t seem to be checking transient perms correctly. LuckPerms always returns UNDEFINED. Should I be using a different method?

129

Yeah, I see now. typo on this line…
https://github.com/lucko/LuckPerms/blob/master/sponge/src/main/java/me/lucko/luckperms/api/sponge/simple/persisted/SimplePersistedSubject.java#L92
should be “res = …”

I’ll sort that soon. Currently tied up in a huge rewrite… :frowning:

Use this build for now: https://ci.lucko.me/job/LuckPerms/221/

It won’t persist defaults but should take account of the transient ones correctly.

130

Ok, so uh, I’ve finished most of my major changes, the latest CI build seems stable & working. I did some brief testing with GP and all seems to be ok.

I have no clue where to start regarding your issues with data loss. If you have any ideas please let me know. I’ll take a look myself, but it sounds like it’s a race condition, so is therefore somewhat harder to diagnose.

131

Yeah I haven’t heard anyone else having this issue, Nucleus keeps wiping my kit data and warps and such to. So I’m not sure how to pinpoint this without an error or anything to go off of. But I’ll test out your changes for GP and defaults once you put out a good release and test that for you as well

132

Ah, if other data is being wiped too, then that would suggest a wider issue, probably not related to LuckPerms. Either way, feel free to drop me any ideas or theories about how said issues could be occurring and I’ll happily look into it.

The latest builds should be stable, certainly more stable than the ones released shortly before them. Make sure you download the latest GP too. :slight_smile:

133

Of course haha can’t report bugs if your not using the latest of everything, but some people will never understand xD

134

As per Support permission options other than prefix/suffix · Issue #23 · LuckPerms/LuckPerms · GitHub, that should now be working. Please use the latest CI build.

The command you need is: /luckperms user <user> meta set <key> <value> [server] [world].

135

Excellent =) I’m starting to send users your way so you may start to see more questions/tickets.