SourceWriters / SmoothTimber

A tree chopping plugin for minecraft bukkit
GNU General Public License v3.0
34 stars 16 forks source link

[BUG]: Plugin fails to load on Folia. #72

Closed pmnlla closed 10 months ago

pmnlla commented 1 year ago

Describe what happend

The latest version of the plugin, smoothtimber-legacy-1.24.7.jar fails to load on Folia. It doesn't bring down the server. There is zero output to the players and truncated error logs on stdout. This is on an aarch64 Rocky Linux server. After having changed the server to paper software, the plugins runs well.

Reproduction steps

  1. Configure folia server in Docker Compose. This is my service:
    minecraft:
        image: 'itzg/minecraft-server'
        tty: true
        stdin_open: true
        ports:
            - '25565:25565'
        environment:
            EULA: "TRUE"
            TYPE: "FOLIA"
        volumes:
            - /home/rocky/mcserver-data:/data
  2. Start the server: docker compose <-f when applicable> up -d minecraft
  3. Power off the server: docker compose down minecraft
  4. Add plugin to plugins directory (in the compose, it is /home/rocky/mcserver-data/plugins/smoothtimber.jar)
  5. Launch the server
  6. Plugin fails to load.

Expected behavior

The plugin would work without a hitch and be accessible to all with its permissions.

Media

mcserver.log

Java Version

Java 17

Server Software

Folia

Minecraft Version

1.20.1

Plugin Version

2.14.7

Additional Information

Please let me know if there is any more info necessary to debug this.

HydrolienF commented 12 months ago

Plugin should at least explicitly support folia by including folia-supported: true into plugin.yml

pmnlla commented 12 months ago

Is it safe to assume this issue was fixed by 36d601c?

HydrolienF commented 12 months ago

plugin.yml haven't been update, so it won't work out of the box. You can try to edit it in a fork.

HydrolienF commented 12 months ago

@pmnlla I found a folia compatible fork there https://github.com/ewof/SmoothTimber/tree/folia

YellowPhoenix18 commented 12 months ago

Plugin should at least explicitly support folia by including folia-supported: true into plugin.yml

So, I have now checked about the status, as far as I can see, Laura added basic support for folia. If you think that the entry is missing, please provide a Pull request, so that we can push it into the main-branch of legacy. Also make sure it is tested.

@pmnlla I found a folia compatible fork there https://github.com/ewof/SmoothTimber/tree/folia

I would highly recommend to not rely on forks. Its always good to have a option of forking (especially if main-authors are unresponsive or whatever), but its never a good choice to rely on a non-official fork. We as team will also not provide any support for issues, that come up with any fork, as we have no control over their source-code and cant guarantee for the quality of that code. Also in the original-repository we are able to make sure, that the quality of code is meet.

But as Laura mentioned: Folia is another additional fork of paper and that is therefor a fork of spigot and that is a fork of bukkit and so on. We are a pretty small team, so maintaining all forks is pretty impossible. We are open for Pull Requests, adding support, as long as they are tested. But the main goal is a working plugin on spigot (The currently nearest fork to bukkit of all the forks out here). We wont add support for any fork,, if the changes required for a specific fork do break the plugin on spigot.

pmnlla commented 12 months ago

By your writing, I assume that the bug is an issue with the folia implementation of the spigot API. I'll probably open an issue there too unless it was fixed in a recent commit on the folia repo.

Thanks a lot for your insight - I'm not a plugin developer, so I'm glad to have your opinions.

I think it might still be a good idea to figure out what changes in the spigot API were made that broke the plugin. I'll look into some logs later when I have the chance to - I've been pretty couped up right now. If the problem is caused by something trivial such as the plugin not running on the main thread of the server it's for sure something that I argue should be looked into here, but i think it's likely the fault of breaking changes folia made which kills compat with many more plugins.

0 clue if this makes sense tbh.

HydrolienF commented 12 months ago

I understand that. Folia is still in beta and require some important changes to work. Using the fork mean that there will be less update and less support.

I will try to make it work on Folia from what Laura add.

HydrolienF commented 12 months ago

By your writing, I assume that the bug is an issue with the folia implementation of the spigot API. I'll probably open an issue there too unless it was fixed in a recent commit on the folia repo.

Folia devs are aware that they broke full compatibility with spigot api. Unfortuatly, because folia is multithreaded, it won't be able to have full compatibility with spigot.

HydrolienF commented 12 months ago

It look like Laura only forget to add 1 line. I have send a pull request.

Lauriichan commented 12 months ago

Plugin should at least explicitly support folia by including folia-supported: true into plugin.yml

So, I have now checked about the status, as far as I can see, Laura added basic support for folia. If you think that the entry is missing, please provide a Pull request, so that we can push it into the main-branch of legacy. Also make sure it is tested.

@pmnlla I found a folia compatible fork there https://github.com/ewof/SmoothTimber/tree/folia

I would highly recommend to not rely on forks. Its always good to have a option of forking (especially if main-authors are unresponsive or whatever), but its never a good choice to rely on a non-official fork. We as team will also not provide any support for issues, that come up with any fork, as we have no control over their source-code and cant guarantee for the quality of that code. Also in the original-repository we are able to make sure, that the quality of code is meet.

But as Laura mentioned: Folia is another additional fork of paper and that is therefor a fork of spigot and that is a fork of bukkit and so on. We are a pretty small team, so maintaining all forks is pretty impossible. We are open for Pull Requests, adding support, as long as they are tested. But the main goal is a working plugin on spigot (The currently nearest fork to bukkit of all the forks out here). We wont add support for any fork,, if the changes required for a specific fork do break the plugin on spigot.

The mentioned fork was reimplemented by me (see https://github.com/SourceWriters/SmoothTimber/pull/69) So yeah I guess I forgot that one line x)

HydrolienF commented 11 months ago

Can I help somehow to have the pull request merged?

Lauriichan commented 11 months ago

I'll merge it once I got time to publish it as well :)

Lauriichan commented 10 months ago

Was added in da7b9c3