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

[B+C] Add missing effects. Adds BUKKIT-5652. #1074

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[B+C] Add missing effects. Adds BUKKIT-5652. #1074

wants to merge 2 commits into from

Conversation

socram8888
Copy link

The Issue:

Adds new effects introduced on Minecraft.

Justification for this PR:

There is no direct way to employ the new effects, as Bukkit does not provide a way to send effects packets with a certain ID.

PR Breakdown:

Add new members to the org.bukkit.Effect enumeration.

Relevant PR(s):

CB-1391 - Bukkit/CraftBukkit#1391

JIRA Ticket:

BUKKIT-4349 - https://bukkit.atlassian.net/browse/BUKKIT-4349
BUKKIT-5652 - https://bukkit.atlassian.net/browse/BUKKIT-5652

* and is used to choose the appropiate texture based on the block
* material. The particles are rendered on top of that.
*/
LAND_DUST(2006, Type.VISUAL, Integer.class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is being touched anyway, it should end the line with comma and the semicolon on the next line.

@Wolvereness
Copy link
Contributor

I'd like to see all of these effects audited with links to the associated mc-dev code.

@socram8888
Copy link
Author

The code I've extracted the ids from isn't available on mc-dev, as the class is only available on the client binary.

Class name on 1.7.9 is "bmo", method "a(Lyy;IIIII)V".

@socram8888
Copy link
Author

Here is the said method: http://pastebin.com/CN9wHaMF

Since Searge isn't updating Minecraft Coder Pack anymore I've been working with the original obfuscated code.

@feildmaster
Copy link
Member

@socram8888
Copy link
Author

IMHO the first (bmo.java) is more clear when it comes to their effect, as you can see the file they play or the particle they spawn.

@turt2live
Copy link
Contributor

There are some similarities between this and #1025. What are the (dis)advantages of either PR? Is there a need for both or can they be combined/adapted?

I also found the Sound enum, which is not updated in this PR. I haven't had a chance to look, but are there any further mismatched/missing cases from not only other parts of the API, but from the game itself?

@turt2live turt2live self-assigned this Jul 19, 2014
@socram8888
Copy link
Author

Well IMHO #1025 and this one they are both mutually compatible. This adds new visual and sound effects while the other adds currently-unimplemented particle effects, which are the basis for visual effects.

However I consider that adding another class for Particle effects is redundant. I wanted to get this pulled first, and then add particle effects as is currently implemented in SportBukkit (https://github.com/OvercastNetwork/SportBukkit/blob/master/Bukkit/0011-Allow-playEffect-to-spawn-particles.patch): as a new Effect type (Type.PARTICLE), without actually making distinction based on how are they implemented by the current network protocol (ie integers for VISUAL and sound, and URIs for particles), allowing programmers to use the very same playEffect methods currently defined in World.

@turt2live turt2live removed their assignment Feb 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants