Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
pmnlla opened this issue Aug 9, 2023 · 13 comments · Fixed by #76
Closed

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

pmnlla opened this issue Aug 9, 2023 · 13 comments · Fixed by #76
Labels
Bug A unintentional behavior that needs to be fixed Platform: Paper

Comments

@pmnlla
Copy link

pmnlla commented Aug 9, 2023

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
  1. Start the server: docker compose <-f when applicable> up -d minecraft
  2. Power off the server: docker compose down minecraft
  3. Add plugin to plugins directory (in the compose, it is /home/rocky/mcserver-data/plugins/smoothtimber.jar)
  4. Launch the server
  5. 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.

@pmnlla pmnlla added the Bug A unintentional behavior that needs to be fixed label Aug 9, 2023
@HydrolienF
Copy link
Contributor

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

@pmnlla
Copy link
Author

pmnlla commented Nov 12, 2023

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

@HydrolienF
Copy link
Contributor

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
Copy link
Contributor

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

@YellowPhoenix18
Copy link
Member

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
Copy link
Author

pmnlla commented Nov 12, 2023

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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

@Lauriichan
Copy link
Member

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 #69)
So yeah I guess I forgot that one line x)

@HydrolienF
Copy link
Contributor

Can I help somehow to have the pull request merged?

@Lauriichan
Copy link
Member

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

@Lauriichan
Copy link
Member

Was added in da7b9c3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A unintentional behavior that needs to be fixed Platform: Paper
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants