search

GeyserMC / Floodgate

Hybrid mode plugin to allow for connections from Geyser to join online mode servers.
https://geysermc.org
MIT License
575 stars 170 forks source link

Add Mojmap support #502

Closed ghost closed 6 months ago

ghost commented 6 months ago

for paper 😄

onebeastchris commented 6 months ago

What error are you getting?

ghost commented 6 months ago

What error are you getting?

haven't tested but i assume its still using spigot mapping from looking at the source

onebeastchris commented 6 months ago

Paper 1.20.5 is supported, and paper does remap spigot-mapped calls to mojang ones. The latest version of floodgate already works on 1.20.5 Spigot, and also on 1.20.5 Paper

ghost commented 6 months ago

Paper 1.20.5 is supported, and paper does remap spigot-mapped calls to mojang ones. The latest version of floodgate already works on 1.20.5 Spigot, and also on 1.20.5 Paper

I disabled this feature because its lowkey bloat, if I pr proper mojang mappings will u accept it?

onebeastchris commented 6 months ago

I wouldn't call that feature bloat as it allows (e.g. us) to make a plugin using internals compatible with both spigot and paper at the same time.

ghost commented 6 months ago

yes but unfortunately its opt out, paper has to manage whole seperate directory of remapped plugin copies hashes etc, everytime plugin is changed it needs to be "recompiled" even if it has 0 reflection in it

onebeastchris commented 6 months ago

https://github.com/GeyserMC/Floodgate/commit/343023fdaf542cbcac6bb31865d01fef598afded The dev branch will add support for mojang mappings. If you feel that strongly about them, feel free to backport that to the current master branch and create a PR - imo, given that paper's remapping feature is opt-out instead of opt-in, i think it's fine to rely on that for that time being

ghost commented 6 months ago

343023f The dev branch will add support for mojang mappings. If you feel that strongly about them, feel free to backport that to the current master branch and create a PR - imo, given that paper's remapping feature is opt-out instead of opt-in, i think it's fine to rely on that for that time being

Thank you 🙏

Tim203 commented 6 months ago

Note that just that commit is not enough to support the latest Paper changes. All new features are added to the dev branch and master only receives updates to support the latest versions. Since Paper offers Spigot compatibility by default we won't be adding it ourselves, but yeah I don't think we'd reject a PR because of it.