search

OvercastNetwork / SportBukkit

CraftBukkit and Bukkit modifications that improve stability and add new features
100 stars 85 forks source link

Redo particle effects API and add new effects #89

Closed socram8888 closed 10 years ago

socram8888 commented 10 years ago

I've redone the particle effects API.

Patches originally used to add the particle stuff have been repurposed to add some missing effects. I've intentionally done this so early in the patch process because I've submitted a PR to Bukkit with the same diff (see https://github.com/Bukkit/Bukkit/pull/1074), so if it gets pulled this century updating would be a matter of removing them both.

Patches whose purpose is to add the actual particle stuff have been added last. I've attempted to keep the diffs at a minimum, so they may look ugly or something, but work well otherwise and with very little overhead.

On the plus side this fixes https://github.com/OvercastNetwork/SportBukkit/issues/88

Yukon commented 10 years ago

The renamed effects are going to cause issues with plugins not compiled with this patch and vise versa.

socram8888 commented 10 years ago

I could add some @Deprecated for that, but otherwise most whose name is different matches more closely the official Bukkit naming.

I want to get this if possible pulled on official Bukkit, so I thought it would make more sense to use the proper names, so if it gets pulled it would not break, even though it breaks now.

tonybruess commented 10 years ago

I'm fine breaking plugins/names

tonybruess commented 10 years ago

@socram8888 care to rebase on master?

socram8888 commented 10 years ago

Done

tonybruess commented 10 years ago

I re-reviewed this PR and it looks good code wise. Have you done any testing? Even if you have I probably will before we merge this.

socram8888 commented 10 years ago

All I tested is that vanilla Bukkit effects (SOUND and VISUAL) work properly (while in the original code they don't). I haven't tested the particle things since I have no plugin which employs them.

Maybe it's desirable to merge only the first two patches (the ones that add the new effects and fixes the original ones from Bukkit), and after that, pull the particle patch.