search

akdukaan / GPFlags

GriefPreventionFlags is a Minecraft plugin to allow players to customize their GriefPrevention claims with claimflags.
GNU General Public License v3.0
17 stars 20 forks source link

Players are still able to fly after respawning #83

Closed IAISI closed 1 month ago

IAISI commented 2 months ago

Describe the bug

Players are still able to fly after respawning.

To reproduce

Steps to reproduce the behavior:

  1. Make claim
  2. Add fly flag
  3. Die
  4. After respawning you will be able to fly at spawn (world guard protected region in our case) and probably everywhere else until you enter another claim...

Expected behavior

https://github.com/akdukaan/GPFlags/commit/c62edfb438309b96004689fc1ba392583d972cb2#diff-6894a9e68d70c208d06d417a2f6ca2c86e1f210139f127fce0f7bedad0377fbfR273

Removed check on PlayerRespawnEvent? I think that might be the issue?

Screenshots, videos, or logs

If applicable, add screenshots, videos, or console logs to show the issue. You can add console logs at binflop and send the link here

Configs and console errors

If you think it might help, paste configs into binflop and send the links here

Versions

gpflags debug [19:13:37 INFO]: [GPFlags] Server version: Sakura 1.21.1-DEV-190206c (MC: 1.21.1) [19:13:37 INFO]: [GPFlags] GP version: 17.0.0-49-gc7151e4-dirty [19:13:37 INFO]: [GPFlags] GPF version: 5.13.5.107

IAISI commented 2 months ago

Actually it's a bit odd, not 100% how to replicate, here's a video of a player who did it: https://www.youtube.com/watch?v=T8PpQI5cExs

akdukaan commented 2 months ago

PlayerRespawnEvent is still listened to in PlayerListener. It then creates a PlayerPostClaimBorderEvent which gets listened to in FlightManager. My best guess is it might just be that I need to delay the call of PlayerPostClaimBorder event by a tick so that the player can finish respawning before I turn off their fly. Will look into this when I get home.

akdukaan commented 2 months ago

Try 109 https://jenkins.luminescent.dev/job/GPFlags/

IAISI commented 1 month ago

Still testing latest commit, there's this tho...


[21:25:03 ERROR]: Could not pass event PlayerPostClaimBorderEvent to GPFlags v5.13.5.109
java.lang.NullPointerException: Cannot invoke "org.bukkit.Location.getWorld()" because "location" is null
        at GPFlags.jar/me.ryanhamshire.GPFlags.FlagManager.getEffectiveFlag(FlagManager.java:199) ~[GPFlags.jar:?]
        at GPFlags.jar/me.ryanhamshire.GPFlags.flags.FlagDef_NoFlight.letPlayerFly(FlagDef_NoFlight.java:20) ~[GPFlags.jar:?]
        at GPFlags.jar/me.ryanhamshire.GPFlags.FlightManager.gpfAllowsFlight(FlightManager.java:173) ~[GPFlags.jar:?]
        at GPFlags.jar/me.ryanhamshire.GPFlags.FlightManager.onEnterNewClaim(FlightManager.java:81) ~[GPFlags.jar:?]
        at com.destroystokyo.paper.event.executor.asm.generated.GeneratedEventExecutor506.execute(Unknown Source) ~[?:?]
        at org.bukkit.plugin.EventExecutor$2.execute(EventExecutor.java:77) ~[sakura-api-1.21.1-R0.1-SNAPSHOT.jar:?]
        at org.bukkit.plugin.RegisteredListener.callEvent(RegisteredListener.java:70) ~[sakura-api-1.21.1-R0.1-SNAPSHOT.jar:?]
        at io.papermc.paper.plugin.manager.PaperEventManager.callEvent(PaperEventManager.java:53) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at io.papermc.paper.plugin.manager.PaperPluginManagerImpl.callEvent(PaperPluginManagerImpl.java:131) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at org.bukkit.plugin.SimplePluginManager.callEvent(SimplePluginManager.java:628) ~[sakura-api-1.21.1-R0.1-SNAPSHOT.jar:?]
        at GPFlags.jar/me.ryanhamshire.GPFlags.listener.PlayerListener.onLogin(PlayerListener.java:86) ~[GPFlags.jar:?]
        at com.destroystokyo.paper.event.executor.MethodHandleEventExecutor.execute(MethodHandleEventExecutor.java:40) ~[sakura-api-1.21.1-R0.1-SNAPSHOT.jar:?]
        at org.bukkit.plugin.RegisteredListener.callEvent(RegisteredListener.java:70) ~[sakura-api-1.21.1-R0.1-SNAPSHOT.jar:?]
        at io.papermc.paper.plugin.manager.PaperEventManager.callEvent(PaperEventManager.java:53) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at io.papermc.paper.plugin.manager.PaperPluginManagerImpl.callEvent(PaperPluginManagerImpl.java:131) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at org.bukkit.plugin.SimplePluginManager.callEvent(SimplePluginManager.java:628) ~[sakura-api-1.21.1-R0.1-SNAPSHOT.jar:?]
        at net.minecraft.server.players.PlayerList.placeNewPlayer(PlayerList.java:346) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at net.minecraft.server.network.ServerConfigurationPacketListenerImpl.handleConfigurationFinished(ServerConfigurationPacketListenerImpl.java:173) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at net.minecraft.network.protocol.configuration.ServerboundFinishConfigurationPacket.handle(ServerboundFinishConfigurationPacket.java:22) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at net.minecraft.network.protocol.configuration.ServerboundFinishConfigurationPacket.handle(ServerboundFinishConfigurationPacket.java:13) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at net.minecraft.network.protocol.PacketUtils.lambda$ensureRunningOnSameThread$0(PacketUtils.java:55) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at net.minecraft.server.TickTask.run(TickTask.java:18) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at net.minecraft.util.thread.BlockableEventLoop.doRunTask(BlockableEventLoop.java:151) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at net.minecraft.util.thread.ReentrantBlockableEventLoop.doRunTask(ReentrantBlockableEventLoop.java:24) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at net.minecraft.server.MinecraftServer.doRunTask(MinecraftServer.java:1540) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at net.minecraft.server.MinecraftServer.doRunTask(MinecraftServer.java:198) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at net.minecraft.util.thread.BlockableEventLoop.pollTask(BlockableEventLoop.java:125) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at net.minecraft.server.MinecraftServer.pollTaskInternal(MinecraftServer.java:1517) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at net.minecraft.server.MinecraftServer.pollTask(MinecraftServer.java:1510) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at net.minecraft.util.thread.BlockableEventLoop.managedBlock(BlockableEventLoop.java:135) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at net.minecraft.server.MinecraftServer.managedBlock(MinecraftServer.java:1470) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at net.minecraft.server.MinecraftServer.waitUntilNextTick(MinecraftServer.java:1476) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1322) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:328) ~[sakura-1.21.1.jar:1.21.1-DEV-190206c]
        at java.base/java.lang.Thread.run(Thread.java:1583) ~[?:?]
[21:25:03 WARN]: com.destroystokyo.paper.exception.ServerEventException: Could not pass event PlayerPostClaimBorderEvent to GPFlags v5.13.5.109
[21:25:03 WARN]:        at io.papermc.paper.plugin.manager.PaperEventManager.callEvent(PaperEventManager.java:71)
[21:25:03 WARN]:        at io.papermc.paper.plugin.manager.PaperPluginManagerImpl.callEvent(PaperPluginManagerImpl.java:131)
[21:25:03 WARN]:        at org.bukkit.plugin.SimplePluginManager.callEvent(SimplePluginManager.java:628)
[21:25:03 WARN]:        at GPFlags.jar//me.ryanhamshire.GPFlags.listener.PlayerListener.onLogin(PlayerListener.java:86)
[21:25:03 WARN]:        at com.destroystokyo.paper.event.executor.MethodHandleEventExecutor.execute(MethodHandleEventExecutor.java:40)
[21:25:03 WARN]:        at org.bukkit.plugin.RegisteredListener.callEvent(RegisteredListener.java:70)
[21:25:03 WARN]:        at io.papermc.paper.plugin.manager.PaperEventManager.callEvent(PaperEventManager.java:53)
[21:25:03 WARN]:        at io.papermc.paper.plugin.manager.PaperPluginManagerImpl.callEvent(PaperPluginManagerImpl.java:131)
[21:25:03 WARN]:        at org.bukkit.plugin.SimplePluginManager.callEvent(SimplePluginManager.java:628)
[21:25:03 WARN]:        at net.minecraft.server.players.PlayerList.placeNewPlayer(PlayerList.java:346)
[21:25:03 WARN]:        at net.minecraft.server.network.ServerConfigurationPacketListenerImpl.handleConfigurationFinished(ServerConfigurationPacketListenerImpl.java:173)
[21:25:03 WARN]:        at net.minecraft.network.protocol.configuration.ServerboundFinishConfigurationPacket.handle(ServerboundFinishConfigurationPacket.java:22)
[21:25:03 WARN]:        at net.minecraft.network.protocol.configuration.ServerboundFinishConfigurationPacket.handle(ServerboundFinishConfigurationPacket.java:13)
[21:25:03 WARN]:        at net.minecraft.network.protocol.PacketUtils.lambda$ensureRunningOnSameThread$0(PacketUtils.java:55)
[21:25:03 WARN]:        at net.minecraft.server.TickTask.run(TickTask.java:18)
[21:25:03 WARN]:        at net.minecraft.util.thread.BlockableEventLoop.doRunTask(BlockableEventLoop.java:151)
[21:25:03 WARN]:        at net.minecraft.util.thread.ReentrantBlockableEventLoop.doRunTask(ReentrantBlockableEventLoop.java:24)
[21:25:03 WARN]:        at net.minecraft.server.MinecraftServer.doRunTask(MinecraftServer.java:1540)
[21:25:03 WARN]:        at net.minecraft.server.MinecraftServer.doRunTask(MinecraftServer.java:198)
[21:25:03 WARN]:        at net.minecraft.util.thread.BlockableEventLoop.pollTask(BlockableEventLoop.java:125)
[21:25:03 WARN]:        at net.minecraft.server.MinecraftServer.pollTaskInternal(MinecraftServer.java:1517)
[21:25:03 WARN]:        at net.minecraft.server.MinecraftServer.pollTask(MinecraftServer.java:1510)
[21:25:03 WARN]:        at net.minecraft.util.thread.BlockableEventLoop.managedBlock(BlockableEventLoop.java:135)
[21:25:03 WARN]:        at net.minecraft.server.MinecraftServer.managedBlock(MinecraftServer.java:1470)
[21:25:03 WARN]:        at net.minecraft.server.MinecraftServer.waitUntilNextTick(MinecraftServer.java:1476)
[21:25:03 WARN]:        at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1322)
[21:25:03 WARN]:        at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:328)
[21:25:03 WARN]:        at java.base/java.lang.Thread.run(Thread.java:1583)
[21:25:03 WARN]: Caused by: java.lang.NullPointerException: Cannot invoke "org.bukkit.Location.getWorld()" because "location" is null
[21:25:03 WARN]:        at GPFlags.jar//me.ryanhamshire.GPFlags.FlagManager.getEffectiveFlag(FlagManager.java:199)
[21:25:03 WARN]:        at GPFlags.jar//me.ryanhamshire.GPFlags.flags.FlagDef_NoFlight.letPlayerFly(FlagDef_NoFlight.java:20)
[21:25:03 WARN]:        at GPFlags.jar//me.ryanhamshire.GPFlags.FlightManager.gpfAllowsFlight(FlightManager.java:173)
[21:25:03 WARN]:        at GPFlags.jar//me.ryanhamshire.GPFlags.FlightManager.onEnterNewClaim(FlightManager.java:81)
[21:25:03 WARN]:        at com.destroystokyo.paper.event.executor.asm.generated.GeneratedEventExecutor506.execute(Unknown Source)
[21:25:03 WARN]:        at org.bukkit.plugin.EventExecutor$2.execute(EventExecutor.java:77)
[21:25:03 WARN]:        at org.bukkit.plugin.RegisteredListener.callEvent(RegisteredListener.java:70)
[21:25:03 WARN]:        at io.papermc.paper.plugin.manager.PaperEventManager.callEvent(PaperEventManager.java:53)
[21:25:03 WARN]:        ... 27 more

which made me look into this...

    /**
     * Checks if a flag is the reason that the player allows flight at a location
     * @param player
     * @param location
     * @param claim Claim at location. Helpful in case a claim is being deleted
     * @return
     */
    private static Boolean gpfAllowsFlight(Player player, Location location, Claim claim) {
        boolean manageFlight = gpfManagesFlight(player);
        if (manageFlight) {
            if (FlagDef_OwnerMemberFly.letPlayerFly(player, location, claim)) {
                return true;
            }
            if (FlagDef_OwnerFly.letPlayerFly(player, location, claim)) {
                return true;
            }
        }
        if (!FlagDef_NoFlight.letPlayerFly(player, location, claim)) {
            return false;
        }
        if (manageFlight) {
            if (FlagDef_PermissionFly.letPlayerFly(player, location, claim)) {
                return true;
            }
        }
        // we have no flight context set, so we need to compare the response for this method from both claims to determine if they should fly
        return null;
    }

it would be safe to implement it like this?

    /**
     * Checks if a flag is the reason that the player allows flight at a location
     * @param player
     * @param location
     * @param claim Claim at location. Helpful in case a claim is being deleted
     * @return
     */
    private static Boolean gpfAllowsFlight(Player player, Location location, Claim claim) {
        boolean manageFlight = gpfManagesFlight(player);
        if (manageFlight) {
            if (FlagDef_OwnerMemberFly.letPlayerFly(player, location, claim)) {
                return true;
            }
            if (FlagDef_OwnerFly.letPlayerFly(player, location, claim)) {
                return true;
            }
            if (!FlagDef_NoFlight.letPlayerFly(player, location, claim)) {
                return false;
            }
            if (FlagDef_PermissionFly.letPlayerFly(player, location, claim)) {
                return true;
            }
        }
        // we have no flight context set, so we need to compare the response for this method from both claims to determine if they should fly
        return null;
    }

after all no flight flag would also mean GP is managing flying? This does "hide" exception but I'm not sure about stuff that this might break.

IAISI commented 1 month ago

Actually my bad... FlagDef_NoFlight should be updated...

    public static boolean letPlayerFly(Player player, Location location, Claim claim) {
        if (claim == null) return true;
        Flag flag = GPFlags.getInstance().getFlagManager().getEffectiveFlag(location, "NoFlight", claim);
        if (flag == null) return true;
        return Util.shouldBypass(player, claim, flag);
    }

That way it would return false if claim is null before actually trying to get world from null location?

akdukaan commented 1 month ago

Fixed in 110 by updating getEffectiveFlag.

IAISI commented 1 month ago

Still an issue and still having hard time replicating.

Had at least types of reports, some players reported glitching/toggling elytra somehow activated fly, others reported TP is activated every time they die and get TPed to spawn. None of which I can replicate.

But now I'm thinking, is it possible that this is caused by anticheat/other plugins canceling events/teleporting player?

IAISI commented 1 month ago

I think we've got it. It's unset flag cmd.

make claim /claimflag OwnerFly /removeclaimflag OwnerFly You can now fly everywhere, in own claim, outside of claims, after TPing to spawn...

works as long as some other plugin doesn't disable it... say for example combat tag or whatever

akdukaan commented 1 month ago

Ok! I see what’s wrong. Will get you a fix in about 8-10 hours.

IAISI commented 1 month ago

Also there's one other way apparently, that involves setting spawn using bed within claim that has fly enabled, then dying, not sure why that's an issue tho.

akdukaan commented 1 month ago

I think the removeclaimflag issue was caused by me trying to check their new permissions while the claim was still existing. And I think the bed issue could be that the spawn point was being changed by another plugin after GPFlags calculates their new flight status based on the old location.

I've given both issues a shot. Try 116 https://jenkins.luminescent.dev/job/GPFlags/

akdukaan commented 1 month ago

I’m taking your thumbs up reactions to mean it worked. Closing as completed. Reopen this if I’m misinterpreting