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

ParticleEffect is badly named, and easily confused with Particle #3836

Open
enjarai opened this issue Apr 5, 2024 · 2 comments
Open

ParticleEffect is badly named, and easily confused with Particle #3836

enjarai opened this issue Apr 5, 2024 · 2 comments

Comments

@enjarai
Copy link
Contributor

enjarai commented Apr 5, 2024

I've recently been doing some stuff with particles, and found I had a really hard time figuring out the difference between the three main involved classes, Particle, ParticleEffect and ParticleType.

After actually digging into the code, It seems ParticleEffect is more like a configured instance of ParticleType than an instance of a "particle effect". It is also used for network transfer and command argument parsing, while Particle is purely client-side, and represents an actual particle in the world.

I don't know what a good alternative name would be, but I do think ParticleEffect should be renamed to reflect it's actual purpose, be less generic, and be more distinct from Particle.

@Linguardium
Copy link

having run into this recently, i agree. Its made even more confusing because you have EntityType which is to Entity as BlockEntityType is to BlockEntity, but ParticleType defines more of a serializer registry for ParticleEffect which is a configuration instance for the actual client rendered particle

@haykam821
Copy link
Contributor

haykam821 commented Apr 7, 2024

The cause of particle confusion could be helped with a few renames. Unfortunately, I think part of the issue is with how the particle classes are designed. For example, DefaultParticleType is both a particle type and particle effect. Some particle effects support various types, but others are tied to exactly one type. There isn't any type safety tying particle types to effects.

However, I think a few renames could be made to clarify the relationship between the particle classes:

  • DefaultParticleType class to SimpleParticleType
  • *ParticleEffect classes to *ParticleParameters (this suffix is already used in parameter names)
  • ParticleEffect.Factory class to Reader

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

No branches or pull requests

3 participants