Skip to content

[B+C] Adds methods for displaying particle effects. Adds BUKKIT-3792 - #1025

Closed
SpaceManiac wants to merge 3 commits into
Bukkit:masterfrom
SpaceManiac:particles
Closed

SpaceManiac wants to merge 3 commits into
Bukkit:masterfrom
SpaceManiac:particles

Conversation

@SpaceManiac

Copy link
Copy Markdown
Contributor

The Issue:

There is no method to display specific particle effects to the users like there are to play individual sounds (Sound) or special effects (Effect).

Justification for this PR:

A wide variety particle effects are not currently possible to display to players, and it would be useful to be able to display them.

PR Breakdown:

A new Particle enum is added to Bukkit, containing the known valid particle effect types. Two showParticles methods each are added to Player and World, to show a particle effect to a single player or nearby players, with or without material data.

The reason for making Particle its own class separate from Effect is twofold. First, the kind of data you need to show a Particle is largely different from that of an Effect. Second, to be parallel to Sound, which is also separate. Sounds and Particles are two kinds of effect to play to the player. The Effect enum contains several special predefined (by Minecraft) combinations of sounds and/or particles.

While this PR could be considered complete, there are a few issues that should be addressed:

  • How to document specific details about what each argument to showParticle does and to which particles (caveats, so on).
  • Potentially adding range-limited methods (NMS's default limit is 16 blocks).
  • Unit tests? Sound has no tests and Effect has a trivial, non-applicable test.
  • Maybe better validation of MaterialData passed (ITEM_BREAK expects item, other two expect block). No crashes involved, just particles looking silly.
  • More descriptive commit message. The current one is just basic info, the real goods are in this PR description.

This is kind of a long list, I know, but I wanted to at least get this initial version up and in the pipeline.

Testing Results and Materials:

Testing plugin code: https://gist.github.com/SpaceManiac/8837964
Testing plugin download: http://wombat.platymuus.com/dl/ParticleTest.jar

How to use:
/particles list - displays Particle.values()
/particles all - shows all particle types in sequence to only you
/particles [name] - shows one particle type to all players
All particles are spawned above the player's head (use third person view)

Relevant PRs:

CB-1334 - Bukkit/CraftBukkit#1334 - Accompanying CraftBukkit PR

B-802 - #802 - Past effort to address this ticket
CB-1066 - Bukkit/CraftBukkit#1066 - Past effort to address this ticket

JIRA Ticket:

BUKKIT-3792 - https://bukkit.atlassian.net/browse/BUKKIT-3792

@Wolvereness

Copy link
Copy Markdown
Contributor

Biggest burning question without reading the PR description: https://github.com/Bukkit/Bukkit/blob/master/src/main/java/org/bukkit/Effect.java

Response (as provided from description):

The reason for making Particle its own class separate from Effect is twofold. First, the kind of data you need to show a Particle is largely different from that of an Effect. Second, to be parallel to Sound, which is also separate. Sounds and Particles are two kinds of effect to play to the player. The Effect enum contains several special predefined (by Minecraft) combinations of sounds and/or particles.

  • It's worth making a response containing this breakdown

@amaranth

amaranth commented Feb 8, 2014

Copy link
Copy Markdown
Contributor

We could bikeshed the name all day (sendParticle, sendParticles, etc) but what you have works so whatever. I think you should add the string variants now though since we know there will shortly be a use. While 1.8 will probably only have the command in the past you couldn't play arbitrary sounds but now can via resource packs so we may as well plan for the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants