diff --git a/fxgl-core/src/main/kotlin/com/almasb/fxgl/particle/ParticleSystem.kt b/fxgl-core/src/main/kotlin/com/almasb/fxgl/particle/ParticleSystem.kt index 121a49fc2..9b72ac61c 100644 --- a/fxgl-core/src/main/kotlin/com/almasb/fxgl/particle/ParticleSystem.kt +++ b/fxgl-core/src/main/kotlin/com/almasb/fxgl/particle/ParticleSystem.kt @@ -35,10 +35,9 @@ class ParticleSystem : Updatable { } override fun onUpdate(tpf: Double) { - emitters.forEach { (emitter, p) -> + emitters.forEach { (emitter, position) -> val particlesList = particles[emitter]!! - - particlesList.addAll(emitter.emit(p.x, p.y)) + particlesList.addAll(emitter.emit(position.x, position.y)) val iter = particlesList.iterator() while (iter.hasNext()) { @@ -46,9 +45,8 @@ class ParticleSystem : Updatable { if (particle.update(tpf)) { iter.remove() - pane.children.remove(particle.view) - Pools.free(p) + Pools.free(particle) } else { if (particle.view.parent == null) pane.children.add(particle.view) @@ -56,4 +54,4 @@ class ParticleSystem : Updatable { } } } -} \ No newline at end of file +} diff --git a/fxgl-core/src/test/kotlin/com/almasb/fxgl/particle/ParticleSystemTest.kt b/fxgl-core/src/test/kotlin/com/almasb/fxgl/particle/ParticleSystemTest.kt index 0aed9f7d8..d4f95de67 100644 --- a/fxgl-core/src/test/kotlin/com/almasb/fxgl/particle/ParticleSystemTest.kt +++ b/fxgl-core/src/test/kotlin/com/almasb/fxgl/particle/ParticleSystemTest.kt @@ -6,9 +6,11 @@ package com.almasb.fxgl.particle +import com.almasb.fxgl.core.pool.Pools import javafx.util.Duration import org.hamcrest.CoreMatchers.`is` import org.hamcrest.MatcherAssert.assertThat +import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.Test /** @@ -51,4 +53,48 @@ class ParticleSystemTest { system.removeParticleEmitter(emitter) } -} \ No newline at end of file + + /** + * Regression test for issue #1417. + * + * Before the fix, ParticleSystem.onUpdate called Pools.free(p) where `p` + * was the emitter's Point2D position (shadowed by the destructured lambda + * parameter), not the expired Particle. Because Pools.free is a no-op for + * unregistered types, expired Particles were never returned to the pool. + * + * This test asserts that, after a full spawn/expire cycle, a Particle pool + * has been registered in Pools.typePools — confirming that Pools.free was + * actually invoked with a Particle instance. + */ + @Test + fun `Expired particles are returned to the pool`() { + val system = ParticleSystem() + + val emitter = ParticleEmitter() + emitter.emissionRate = 1.0 + emitter.numParticles = 50 + emitter.maxEmissions = 5 + emitter.setExpireFunction { Duration.seconds(0.5) } + + system.addParticleEmitter(emitter, 0.0, 0.0) + + // Drive the system long enough that all 5 * 50 = 250 particles spawn and die + repeat(10) { system.onUpdate(0.5) } + + // After the run, no live particles should remain in the scene + assertThat(system.pane.children.size, `is`(0)) + + // Reflectively inspect Pools.typePools to confirm a Particle pool exists. + // Pre-fix, the pool stayed empty because Pools.free(Point2D) was a no-op. + val typePoolsField = Pools::class.java.getDeclaredField("typePools") + typePoolsField.isAccessible = true + + @Suppress("UNCHECKED_CAST") + val typePools = typePoolsField.get(null) as Map, *> + + val particlePool = typePools[Particle::class.java] + assertNotNull(particlePool, "Particle pool should exist after free()") + + system.removeParticleEmitter(emitter) + } +}