Review Pore's patches to Bukkit API

GlowstoneMC / Glowstone

A fast, customizable and compatible open source server for Minecraft: Java Edition

https://glowstone.net

Other

1.9k stars 273 forks source link

Review Pore's patches to Bukkit API #32

Closed deathcap closed 7 years ago

deathcap commented 9 years ago

The Pore project (an implementation of Spigot's 1.8.3 update to the Bukkit API) has a handful of its own patches to the API, some might make sense to pull into Glowstone++'s:

https://github.com/LapisBlue/Pore/tree/master/patches

[x] 0001-Disable-Spigot-version-check.patch
[ ] 0002-Remove-final-from-event-getters.patch
[ ] 0003-Add-HandlerList-adapters.patch
[ ] 0004-Make-events-properly-initialisable.patch
[ ] 0005-Add-method-to-get-EntityType-from-Class.patch
[ ] 0006-Disable-reload-command.patch
[ ] 0007-Remove-plugin-prefixes-and-use-normal-logger-instead.patch
[ ] 0008-Add-validity-check-supermethod-to-top-Event-class.patch

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

mastercoms commented 9 years ago

First one is useless for a server implementation, we need a version command. I think the second one was only done so they could override it for Pore's use. Though plugins could possibly use this to make their own custom APIs suited for their plugins on top of Bukkit. Third one seems good. 4th one seems only applicable to Pore. 5th is useful.

In my opinion, we should use only 3 and 5.

deathcap commented 9 years ago

I took the first patch (committed in https://github.com/deathcap/Glowkit/commit/18d92a429a99d64e814858d59e444361fbeb20e3), it only disables Spigot's online API version check (queries https://hub.spigotmc.org/stash/rest/api/1.0/projects/SPIGOT/repos/) in /version, which with Glowstone++ always fails (but we could re-enable it if we have a similar stash API for this project, at some point)

mastercoms commented 9 years ago

Oh, I didn't see that part, sorry.

deathcap commented 8 years ago

Pore has since added three additional patches:

0006-Disable-reload-command.patch - I believe this was added because of issues with mods when reloading (something with resetting recipes mod recipes?), does not apply to us
0007-Remove-plugin-prefixes-and-use-normal-logger-instead.patch - seems like we wouldn't want this one either, plugin prefixes in logging are useful..
0008-Add-validity-check-supermethod-to-top-Event-class.patch - unsure, added for Sponge compatibility, might be useful later for SpongeAPI support?

gdude2002 commented 8 years ago

This ticket needs categorization on the waffle board.

Has this ticket seen any movement or has there been any action expected since the previous comment?

mastercoms commented 7 years ago

Closing due to Pore being dead, and the patches don't seem to add much for Bukkit plugins.