aromaa / WorldGuardExtraFlags

Extension for the WorldGuard plugin.
https://www.spigotmc.org/resources/worldguard-extra-flags.4823
MIT License
90 stars 71 forks source link

Option to not force glide when glide: ALLOW #94

Closed mdigit5 closed 4 years ago

mdigit5 commented 4 years ago

The "glide: ALLOW" flag forces users to glide, even when on the ground. It prevents users from walking at all.

The use case where this becomes a problem is as follows:

Global region (standard global region): No gliding allowed. Glide flag is set to DENY Inner region (no parent / no inheritance): Gliding allowed. I need to override the glide DENY on the global region.

When I use the glide: ALLOW flag in my inner region everybody ends up flat on the ground in "glide position". Walking is no longer possible. The flag forces glide rather than allowing it. To allow normal game play I had to remove this flag.

When I set my inner region (with no glide flags set) to a higher priority than the global region, glide still isn't allowed in the inner region. This is because even though the inner region has a higher priority, a glide flag actually needs to be set (allowed or denied - not "unset") in the higher priority region for it to take precedence.

Is this a bug in WGEP? Hoping you can take a look.

Thanks!

aromaa commented 4 years ago

The original suggestion was to allow gliding without having elytra equipped, so the ALLOW flag was supposed to do that. Tbh most of the flags would benefit from having "DEFAULT" option

mdigit5 commented 4 years ago

Hi. Thanks for finding time to take a look at this. I can see how it would be cool to allow glide without an Elytra, but even when equipped with an Elytra (for real) you can still walk. With the glide: ALLOW flag set you can't walk anymore! ;) That seems a bit buggy. Even without addressing my particular use case, maybe there is a way to allow a player to walk when glide: ALLOW is set?

However, In my case I really wouldn't want to allow people to glide without an elytra. I just want to deny it's use in a region.

Having a "DEFAULT" option would be nice, but I'm pretty sure you mean that WG itself needs to provide this option so that your plugin can use it. i.e. "DEFAULT" is different than "unset" - "DEFAULT" is a real value for the flag. The priority on the flag values from low to high could be 0-unset, 1-allow, 2-deny, 3-default. In my scenario, the global region would be set as priority 0 / glide: DENY. The inner region could be set to priority 0 / glide: DEFAULT. In this case glide would be allowed in the inner region without forcing it with glide: ALLOW. You could set a higher priority on the inner region but you wouldn't need to. I suppose you could also set the priorities as 0-unset, 1-allow, 2-default, 3-deny, and then use a higher priority on the inner region to override DENY in the global region, etc... anyway, all of this can work in different ways if there was a real "DEFAULT" value for the flag.

EDIT: Reading this again, I suppose the proper priority for the flags (from low to high) would be 0-unset, 1-default, 2-allow, 3-deny. Then you would just set priority levels on the region to override a higher priority flag.

Anyway, is that what you mean by having a "DEFAULT" option? and, If yes... I guess that means without a major update to WG, there isn't much you can do to fix things in WGEF. Is that correct?

aromaa commented 4 years ago

I could resolve this in WGEF itself by changing it to custom state flag which has three options, ALLOW, DENY and DEFAULT/NONE/UNSET. I have seen this being suggested for other types of flags also to allow overriding the option in child regions.

Regarding the issue of not being able to walk depends a lot whatever the client sends is it on the ground, I have yet to test this. If thats not the case however it would most likely need another state flag just for the glide flag, FORCED. Meaning allow allows gliding on the area and FORCED, as the name suggest, forces you to glide like the current behavior does.

mdigit5 commented 4 years ago

It would be great if you could handle it in WGEF with a custom state flag. I'm very happy to test / help any way I can if you are able to some find time to do the dev work.

As for the inability to walk with glide: ALLOW set, I have observed the "forced glide" at various elevations (if that's what you mean) - not on purpose. Just because I happened to be at different elevations when testing it. If there is something specific you want me to test here, I can do that too.

I appreciate you taking time to look at this. Thanks!

Bobcat00 commented 4 years ago

I could resolve this in WGEF itself by changing it to custom state flag which has three options, ALLOW, DENY and DEFAULT/NONE/UNSET. I have seen this being suggested for other types of flags also to allow overriding the option in child regions.

But wouldn't DEFAULT (or whatever) be the same as not setting the flag? Why does this need a custom state?

mdigit5 commented 4 years ago

But wouldn't DEFAULT (or whatever) be the same as not setting the flag? Why does this need a custom state?

My understanding is that DEFAULT would actually be a flag value, so not the same as not setting it / "UNSET". The problem is that WG region priority handling only comes in to play if a region flag has an actual value (not "UNSET"). However, for WGEF to set a new flag value of "DEFAULT" it means that WG would need to accept/respect 3 flag values: ALLOW, DENY, DEFAULT and then UNSET / no value set at all.

I'm realizing how confusing what I just wrote is!! ;) Here is how I think it would work:

Global region: DENY Glide Inner Region: ALLOW Glide + higher priority than global region Result: Glide is denied in the global region but forced in the inner region because ALLOW == FORCE (and nobody can walk).

Global Region: DENY Glide Inner Region: "UNSET"/no value for the Glide flag + higher priority than global region Result: Glide is denied in the global region and Glide is still denied in the inner region. Why? - Even though the inner region has a higher priority, no actual Glide flag value is set (UNSET is not a value) and WG will only apply priority if a flag value is set. So, the inner region's higher priority "UNSET" is ignored by WG and the global region's DENY Glide flag still takes precedence over the inner region.

Global Region: DENY Glide Inner Region: DEFAULT Glide + higher priority than global region Expected Result: Glide is denied in the global region but allowed (without forcing) in the inner region. Why? - If we can set an actual value of "DEFAULT" for the glide flag in the inner region (a value that WG will accept/respect) then WG will consider the higher priority on this region for the Glide flag and override the global Glide DENY / returning the inner region to the "default" state without forcing Glide using ALLOW.

Of course, I may have no idea what I'm taking about (highly likely actually), so please correct me as needed!! ;)

Assuming this is what isokissa3 had in mind, it seems to me that WG would need to accept DEFAULT as a valid flag value, and I'm not sure if it does?

aromaa commented 4 years ago

This was duplicate of #23 which is now closed

mdigit5 commented 4 years ago

This is AWESOME! Thank-you! I've tested it in Paper MC v1.15.2 Build 87 with the latest version of WorldGuard (bukkit-7.0.2). It works great!

The only problem I have is that the flags don't persist through a server restart. I have Glide:DENY in the global region and Glide:ALLOW in my inner region. I've tried it several times. After a server restart the glide flags disappear.

Once in the server log I saw a WorldGuard error saying something like "cannot enumerate glide flag...". Although I can consistently reproduce the state after restart where the flags do not persist, I cant reproduce these errors/warnings in log. No idea why.

If you have time, could you help me to troubleshoot?

Thanks again for making this update. I think it will be really useful to others using the plugin too!

P.s. - Not sure if this is the right place to handle this. Let me know.

aromaa commented 4 years ago

Ah yeah, this was small mistake on my part, fixed now

mdigit5 commented 4 years ago

That's great. Glad it was something simple! Can I download the new plugin jar now from spigotmc or does it take time for it to update? Seems like it's the same jar (same file hashes).

aromaa commented 4 years ago

No, I need to put it manually on spigot, you can grap the dev build which is always up to date with the Github's sources.

mdigit5 commented 4 years ago

Ok, got it. Working great! Thanks again for taking time to do this. I really appreciate it. I had to disable elytra flight on the server, and now it's back (in designated regions). The kids are VERY happy. I showed them this thread so they know that a young guy like you is making great Minecraft plugins and helping everybody out. Very cool. Thank-you!