GitBrincie212 / Apel-Mod

Apel is a library that brings particle animations to the table with flexible behaviour and a clean developer interface. It promises also lots of predefined shapes & paths to help the developer on their particle scene
Other
2 stars 1 forks source link

`ParticleCuboid.Builder` needs to handle `#amount(int)` correctly #46

Closed DarthSharkie closed 2 months ago

DarthSharkie commented 2 months ago

With the move to builders, most objects can set their particle counts by calling .amount(int). ParticleCuboid has special handling for amount, and setting a single integer cannot work correctly because that method is (and should/must be) final so that ParticleObject can maintain its invariants. It is documented that #setAmount(int) doesn't work on Cuboid, but the docs should also describe the correct way to use the builder.

At the same time, the builder can be made to work without sacrificing the invariants because the #build() method allows for logic prior to constructor, a key benefit of the builder pattern.

Example that breaks with existing code:

ParticleCuboid particleCuboid = ParticleCuboid.builder()
        .particleEffect(ParticleTypes.END_ROD).amount(50).size(new Vector3f(1f)).build();

Correct usage:

ParticleCuboid particleCuboid = ParticleCuboid.builder()
        .particleEffect(ParticleTypes.END_ROD).amount(new Vector3i(50)).size(new Vector3f(1f)).build();
DarthSharkie commented 2 months ago

Another way to fix this would be to create a CuboidAmount record and use a proper structure for it. Then the builder check gets simpler.

A possible way to force the correct method would be throw IllegalStateException if the amount vector isn't set. This is reasonable, per Effective Java's Item on "Favor the use of standard exceptions" and its discussion of a class instance in an illegal state to perform an action.

GitBrincie212 commented 2 months ago

Yup i think that closes this issues