FactionsUUID support broken by updates

[mbax]

Filling out the template for funsies:

Versions EnderContainers version: 2.2.2 Platform version: git-Paper-349 (MC: 1.17.1) [but literally anything, really]

Describe the bug FactionsUUID support fails, throwing an exception to console instead.

[02:38:38 ERROR]: Could not pass event PlayerInteractEvent to EnderContainers v2.2.2
java.lang.NoClassDefFoundError: com/massivecraft/factions/zcore/fperms/PermissableAction
        at fr.utarwyn.endercontainers.dependency.Factions1Dependency.validateBlockChestOpening(Factions1Dependency.java:52) ~[Endercontainers-2.2.2.jar:?]
        at fr.utarwyn.endercontainers.dependency.DependenciesManager.validateBlockChestOpening(DependenciesManager.java:74) ~[Endercontainers-2.2.2.jar:?]
        at fr.utarwyn.endercontainers.enderchest.EnderChestListener.onPlayerInteract(EnderChestListener.java:81) ~[Endercontainers-2.2.2.jar:?]
        at com.destroystokyo.paper.event.executor.asm.generated.GeneratedEventExecutor915.execute(Unknown Source) ~[?:?]
        at org.bukkit.plugin.EventExecutor.lambda$create$1(EventExecutor.java:69) ~[patched_1.17.1.jar:git-Paper-349]
        at co.aikar.timings.TimedEventExecutor.execute(TimedEventExecutor.java:80) ~[patched_1.17.1.jar:git-Paper-349]
        at org.bukkit.plugin.RegisteredListener.callEvent(RegisteredListener.java:70) ~[patched_1.17.1.jar:git-Paper-349]
        at org.bukkit.plugin.SimplePluginManager.callEvent(SimplePluginManager.java:628) ~[patched_1.17.1.jar:git-Paper-349]
        at org.bukkit.craftbukkit.v1_17_R1.event.CraftEventFactory.callPlayerInteractEvent(CraftEventFactory.java:543) ~[patched_1.17.1.jar:git-Paper-349]
        at net.minecraft.server.level.ServerPlayerGameMode.useItemOn(ServerPlayerGameMode.java:542) ~[app:?]
        at net.minecraft.server.network.ServerGamePacketListenerImpl.handleUseItemOn(ServerGamePacketListenerImpl.java:1815) ~[app:?]
        at net.minecraft.network.protocol.game.ServerboundUseItemOnPacket.handle(ServerboundUseItemOnPacket.java:33) ~[app:?]
        at net.minecraft.network.protocol.game.ServerboundUseItemOnPacket.handle(ServerboundUseItemOnPacket.java:9) ~[app:?]
        at net.minecraft.network.protocol.PacketUtils.lambda$ensureRunningOnSameThread$1(PacketUtils.java:56) ~[app:?]
        at net.minecraft.server.TickTask.run(TickTask.java:18) ~[patched_1.17.1.jar:git-Paper-349]
        at net.minecraft.util.thread.BlockableEventLoop.doRunTask(BlockableEventLoop.java:149) ~[app:?]
        at net.minecraft.util.thread.ReentrantBlockableEventLoop.doRunTask(ReentrantBlockableEventLoop.java:23) ~[app:?]
        at net.minecraft.server.MinecraftServer.doRunTask(MinecraftServer.java:1423) ~[patched_1.17.1.jar:git-Paper-349]
        at net.minecraft.server.MinecraftServer.shouldRun(MinecraftServer.java:192) ~[patched_1.17.1.jar:git-Paper-349]
        at net.minecraft.util.thread.BlockableEventLoop.pollTask(BlockableEventLoop.java:122) ~[app:?]
        at net.minecraft.server.MinecraftServer.pollTaskInternal(MinecraftServer.java:1401) ~[patched_1.17.1.jar:git-Paper-349]
        at net.minecraft.server.MinecraftServer.pollTask(MinecraftServer.java:1394) ~[patched_1.17.1.jar:git-Paper-349]
        at net.minecraft.util.thread.BlockableEventLoop.managedBlock(BlockableEventLoop.java:132) ~[app:?]
        at net.minecraft.server.MinecraftServer.waitUntilNextTick(MinecraftServer.java:1372) ~[patched_1.17.1.jar:git-Paper-349]
        at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1283) ~[patched_1.17.1.jar:git-Paper-349]
        at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:319) ~[patched_1.17.1.jar:git-Paper-349]
        at java.lang.Thread.run(Thread.java:831) ~[?:?]

To Reproduce

1. Interact with an enderchest
2. ???
3. Profit! Exception!

Expected behavior Not throwing an exception to console.

Screenshots Nope!

Additional context Alright, time to talk! 😁

Basically, since 2019 there have been a few changes that break the built-in support for FactionsUUID (aka a 1.x fork).

1. PermissibleAction became an interface, with PermissibleActions being an enum implementation of it, so PermissibleActions.CONTAINER would be the new referenced action.
2. Faction#hasAccess returns a boolean, because returning an enum was dumb.
3. Faction#hasAccess gained an optional, but really preferred, parameter. New method to call takes an FLocation at the end. This will be used for an upcoming feature of defining different access for different claims in a faction.

So the new line would be overall:

if (currentFac != playerFac || playerFac.getAccess(fPlayer, PermissableAction.CONTAINER, new FLocation(block))) {

(Though you could just create that FLocation once by reusing the one from the earlier use in getting the currentFac value)

However, this would break support for people inexplicably running a 2011 version of 1.x, and more importantly for folks running forks of FUUID. Those forks generally broke off in 2018 or earlier so they won't have this code. FactionsUUID forks are an absolute nightmare for managing support, so I'm sorry in advance. Some of them do hilarious things like continue to use our versioning system so you can't even easily check versions.

One thought I had was adding a matchAuthor to your DependencyResolver and just checking for me (mbaxter, not mbax - someone beat me to the username here) in the author list.

I'm happy to provide a PR for this if you want, but I wanted to bring it up first since it's your codebase.

For building, if you want to do it:

repository: https://ci.ender.zone/plugin/repository/everything/
groupId: com.massivecraft
artifactId: Factions
version: 1.6.9.5-U0.6.3

For a less formal back and forth discussion, which I welcome, I can be found on Discord as mbaxter#1592 (and we share Spigot's discord as a common ground so you should be able to find me and friend me 😄 ), but I'm happy to limit discussion to here as well.

[utarwyn]

Hello @mbax, thank you for this complete feedback, really appreciated! :+1:

I always had trouble understanding the versions and forks of all Factions plugin, it's quite a mess! But I understand your needs and it seems legit to also support your updated version of Factions plugin.

Based on plugin statistics, some server owners are still using a really old version of Factions indeed..

So, technically speaking, what I propose:

• Adding a new dependency module called "factionsuuid" with Maven dependency and changes you mentionned.

  So there will be factions1 (legacy), factions2 and factionsuuid.

• Improve DependencyResolver to also match plugin authors if provided.

  If no author matched, use module that does not check for author if provided.

• Register new "factionsuuid" dependency module in the manager.
• Maybe soft depend your plugin if its name is different?

I can start to work on it next week, as some server owners need it quickly. But this feature is also open to PR if you have already worked on it. Thank you for your time! 😁

[mbax]

it's quite a mess!

This is an excellent summary of the factions ecosystem as a whole. 🤣

I'd absolutely love to have FUUID report a different plugin name, but since around half its users run 1.8 and plugins that were abandoned half a dozen years ago, I can't drop that for compatibility reasons currently. (Looking to do a split where I operate a 'legacy' version of the plugin and a 'modern' version that is sane, and would happily PR any additional changes then)

I'm not sure if I'll be able to beat your timeline on getting started, myself, but if a week or two passes and you haven't had the chance to work on it yet I could start some work. 😄

Thanks for the quick response!