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

Particles per second parameter apparently does not change anything #20

Closed
ANPez opened this issue Aug 23, 2017 · 4 comments
Closed

Particles per second parameter apparently does not change anything #20

ANPez opened this issue Aug 23, 2017 · 4 comments
Labels

Comments

@ANPez
Copy link

ANPez commented Aug 23, 2017

I see the same result when passing "1" as parameters as when 1000.

@DanielMartinus
Copy link
Owner

DanielMartinus commented Aug 23, 2017

Thanks for reporting @ANPez

I did a quick look in the code and as far as I can see is that amountPerMs is calculated when the StreamEmitter is created here:

amountPerMs = TimeUnit.NANOSECONDS.toMillis(1000L / particlesPerSecond)

But not used when the particles are being created here

/**
* If timer isn't started yet, set initial start time
* Create the first confetti immediately and update the last emit time
*/
override fun createConfetti(deltaTime: Float) {
// if maxParticles is set and particlesCreated not within
// range of maxParticles stop emitting
if(maxParticles in 1..(particlesCreated - 1)) {
return
}
// Check if particle should be created
if (deltaTime >= amountPerMs && elapsedTime < emittingTime) {
this.addConfetti()
}
elapsedTime += deltaTime * 1000
}

I'll make some time to implement this properly and see if I can cover this with tests. But I have not much time until the weekend. A Pull request is welcome ofcourse if there is no time to wait.

In the meantime you might be able to achieve the same by creating more than one particle stream if you need a lot of particles creations happening at the same time. For a slower rate there needs to be a fix.

@DanielMartinus
Copy link
Owner

Hi @ANPez,

To fix the particles per second bug I've refactored the StreamEmitter. I've opened a pull request which also contains a couple of other fixes such as rewriting the RenderSystem to make it all more testable. You'll also find tests in the pull request referenced in this issue.

I'm not exactly sure when I'll push an update for the library, for now the pull request isn't merged yet. I'll update you on the progress.

@DanielMartinus
Copy link
Owner

@ANPez
Copy link
Author

ANPez commented Sep 4, 2017

Thanks! Really appreciate your work 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants