fix: default values on removeParticles logic #11374
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 9d87daf:
|
| // Default beginIndex to 0 if not provided | ||
| const start = beginIndex ?? 0; | ||
| // Default endIndex to the length of particleChildren if not provided | ||
| const end = endIndex ?? this.particleChildren.length; | ||
| // Remove the correct range | ||
| const children = this.particleChildren.splice(start, end); |
There was a problem hiding this comment.
Alternatively:
| // Default beginIndex to 0 if not provided | |
| const start = beginIndex ?? 0; | |
| // Default endIndex to the length of particleChildren if not provided | |
| const end = endIndex ?? this.particleChildren.length; | |
| // Remove the correct range | |
| const children = this.particleChildren.splice(start, end); | |
| // Default beginIndex to 0 if not provided | |
| beginIndex ??= 0; | |
| // Default endIndex to the length of particleChildren if not provided | |
| endIndex ??= this.particleChildren.length; | |
| // Remove the correct range | |
| const children = this.particleChildren.splice(beginIndex, endIndex); |
There was a problem hiding this comment.
This should be considered a breaking change because users may have removeParticles() calls in production. If their pixijs version isn't locked, their next release could exhibit unintentional behavior if they upgrade without thorough testing.
There was a problem hiding this comment.
tweaked this based on your feedback @bigtimebuddy .
Thanks for youre insight @susiwen8! , whilst it is kind of a breaking change, it also is fixing incorrect behaviour. With that in mind, I think on this occasion its ok not to consider this breaking as the use case would be quite strange!
…oveParticles method
Description of change
update
removeParticleslogic to match description provided. Set the endIndex to default to the container lengthCloses #11357
Pre-Merge Checklist
npm run lint)npm run test)