Fix effect properties ignored

CHANGELOG.md

@@ -42,6 +42,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Bind groups for effect rendering are now created in a separate system in the `EffectSystems::PrepareBindGroups` set, itself part of Bevy's `RenderSet::PrepareBindGroups`. They're also cached, which increases the performance of rendering many effects.
 - Merged the init and update pass bind groups for the particle buffer and associated resources in `EffectBfufer`. The new unified resources use the `_sim` (simulation) suffix.
 - `CompiledParticleEffect` now holds a strong handle to the same `EffectAsset` as the `ParticleEffect` it's compiled from. This ensures the asset is not unloaded while in use during the frame. To allow an `EffectAsset` to unload, clear the handle of the `ParticleEffect`, then allow the `CompiledParticleEffect` to observe the change and clear its own handle too.
+- The `EffectProperties` component is now mandatory, and has been added to the `ParticleEffectBundle`. (#309)
 
 ### Removed
 
@@ -61,6 +62,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fixed invalid WGSL being generated for large `u32` values.
 - Fixed particle flickering, only for particles rendered through the `AlphaMask3d` render pass (`EffectAsset::alpha_mode == AlphaMode::Mask`). (#183)
 - Fixed invalid layout of all `mat3xR<f32>` returned by `MatrixValue::as_bytes()`, which was missing padding. (#310)
+- Fixed a regression where declaring properties but not adding an `EffectProperties` component would prevent properties from being uploaded to GPU. The `EffectProperties` component is now mandatory, even if the effect doesn't use any property. However there's still no GPU resource allocated if no property is used. (#309)
 
 ## [0.10.0] 2024-02-24
 

examples/force_field.rs

@@ -59,7 +59,10 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         .init_resource::<inspector::Configuration>()
         .register_type::<inspector::Configuration>()
         .add_systems(Update, inspector::inspector_ui)
-        .add_systems(PostUpdate, inspector::apply_tweaks);
+        .add_systems(
+            PostUpdate,
+            inspector::apply_tweaks.after(EffectSystems::UpdatePropertiesFromAsset),
+        );
 
     app.add_systems(Startup, setup)
         .add_systems(
@@ -357,28 +360,7 @@ fn setup(
             .render(ColorOverLifetimeModifier { gradient }),
     );
 
-    commands.spawn((
-        ParticleEffectBundle::new(effect),
-        // Note: regression as of 0.10, we need to explicitly add this component with the correct
-        // properties.
-        EffectProperties::default().with_properties([
-            (
-                "repulsor_position".to_string(),
-                Value::Vector(REPULSOR_POS.into()),
-            ),
-            ("attraction_accel".to_string(), Value::Scalar(20.0.into())),
-            (
-                "max_attraction_speed".to_string(),
-                Value::Scalar(5.0.into()),
-            ),
-            ("sticky_factor".to_string(), Value::Scalar(2.0.into())),
-            (
-                "shell_half_thickness".to_string(),
-                Value::Scalar(0.1.into()),
-            ),
-            ("repulsor_accel".to_string(), Value::Scalar((-15.0).into())),
-        ]),
-    ));
+    commands.spawn(ParticleEffectBundle::new(effect));
 }
 
 fn spawn_on_click(

examples/spawn.rs

@@ -161,10 +161,6 @@ fn setup(
                     .with_rotation(Quat::from_rotation_z(1.)),
                 ..Default::default()
             },
-            // Note: We don't need to manually insert an EffectProperties here, because Hanabi will
-            // take care of it on next update (since the effect has a property). Since we don't
-            // really use that property here, we don't access the EffectProperties so don't care
-            // when it's spawned. See also effect3 below for a different approach.
         ))
         .with_children(|p| {
             // Reference cube to visualize the emit origin
@@ -294,12 +290,6 @@ fn setup(
                 transform: Transform::from_translation(Vec3::new(30., 0., 0.)),
                 ..Default::default()
             },
-            // Note: We manually insert EffectProperties so update_accel() can immediately set a
-            // new value to the property, without having to deal with one-frame delays. If we let
-            // Hanabi create the component, it will do so *before* Update, so on the first frame
-            // after spawning it, update_accel() will not find it (it's spawned on next frame) and
-            // will panic. See also effect1 above.
-            EffectProperties::default(),
             DynamicRuntimeAccel,
         ))
         .with_children(|p| {

examples/spawn_on_command.rs

@@ -186,10 +186,7 @@ fn setup(
     );
 
     commands
-        .spawn((
-            ParticleEffectBundle::new(effect),
-            EffectProperties::default(),
-        ))
+        .spawn(ParticleEffectBundle::new(effect))
         .insert(Name::new("effect"));
 }
 

src/bundle.rs

@@ -1,4 +1,4 @@
-use crate::{CompiledParticleEffect, EffectAsset, ParticleEffect};
+use crate::{CompiledParticleEffect, EffectAsset, EffectProperties, ParticleEffect};
 use bevy::prelude::*;
 
 /// A component bundle for a particle effect.
@@ -7,11 +7,6 @@ use bevy::prelude::*;
 /// function correctly, and is the preferred method for spawning a new
 /// [`ParticleEffect`].
 ///
-/// If the effect uses properties, you can additionally spawn an
-/// [`EffectProperties`] component and insert initial values for some or all its
-/// properties. This bundle however doesn't add that component by default, to
-/// allow skipping effects without properties where possible.
-///
 /// [`EffectProperties`]: crate::EffectProperties
 #[derive(Bundle, Clone)]
 pub struct ParticleEffectBundle {
@@ -23,6 +18,11 @@ pub struct ParticleEffectBundle {
     /// for the effect to work. This is split from the [`ParticleEffect`] itself
     /// mainly for change detection reasons, as well as for semantic.
     pub compiled_effect: CompiledParticleEffect,
+    /// Runtime storage for effect properties.
+    ///
+    /// Stores the current property values, before they're uploaded to GPU. Can
+    /// be modified manually to set a new property value.
+    pub effect_properties: EffectProperties,
     /// Transform of the entity, representing the frame of reference for the
     /// particle emission.
     ///
@@ -87,6 +87,7 @@ impl ParticleEffectBundle {
         Self {
             effect: ParticleEffect::new(handle),
             compiled_effect: CompiledParticleEffect::default(),
+            effect_properties: EffectProperties::default(),
             transform: Default::default(),
             global_transform: Default::default(),
             visibility: Default::default(),

src/lib.rs

@@ -197,9 +197,9 @@ pub use bundle::ParticleEffectBundle;
 pub use gradient::{Gradient, GradientKey};
 pub use graph::*;
 pub use modifier::*;
-pub use plugin::HanabiPlugin;
+pub use plugin::{EffectSystems, HanabiPlugin};
 pub use properties::*;
-pub use render::{EffectSystems, LayoutFlags, ShaderCache};
+pub use render::{LayoutFlags, ShaderCache};
 pub use spawn::{tick_spawners, CpuValue, EffectSpawner, Random, Spawner};
 pub use time::{EffectSimulation, EffectSimulationTime};
 

src/plugin.rs

@@ -22,16 +22,69 @@ use crate::{
     render::{
         extract_effect_events, extract_effects, prepare_bind_groups, prepare_effects,
         prepare_resources, queue_effects, DispatchIndirectPipeline, DrawEffects, EffectAssetEvents,
-        EffectBindGroups, EffectCache, EffectSystems, EffectsMeta, ExtractedEffects,
-        GpuSpawnerParams, ParticlesInitPipeline, ParticlesRenderPipeline, ParticlesUpdatePipeline,
-        ShaderCache, SimParams, VfxSimulateDriverNode, VfxSimulateNode,
+        EffectBindGroups, EffectCache, EffectsMeta, ExtractedEffects, GpuSpawnerParams,
+        ParticlesInitPipeline, ParticlesRenderPipeline, ParticlesUpdatePipeline, ShaderCache,
+        SimParams, VfxSimulateDriverNode, VfxSimulateNode,
     },
     spawn::{self, Random},
     tick_spawners,
     time::effect_simulation_time_system,
     update_properties_from_asset, EffectSimulation, ParticleEffect, RemovedEffectsEvent, Spawner,
 };
 
+/// Labels for the Hanabi systems.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, SystemSet)]
+pub enum EffectSystems {
+    /// Tick all effect instances to generate particle spawn counts.
+    ///
+    /// This system runs during the [`PostUpdate`] schedule. Any system which
+    /// modifies an effect spawner should run before this set to ensure the
+    /// spawner takes into account the newly set values during its ticking.
+    TickSpawners,
+
+    /// Compile the effect instances, updating the [`CompiledParticleEffect`]
+    /// components.
+    ///
+    /// This system runs during the [`PostUpdate`] schedule. This is largely an
+    /// internal task which can be ignored by most users.
+    CompileEffects,
+
+    /// Update the properties of the effect instance based on the declared
+    /// properties in the [`EffectAsset`], updating the associated
+    /// [`EffectProperties`] component.
+    ///
+    /// This system runs during Bevy's own [`UpdateAssets`] schedule, after the
+    /// assets have been updated. Any system which modifies an
+    /// [`EffectAsset`]'s declared properties should run before [`UpdateAssets`]
+    /// in order for changes to be taken into account in the same frame.
+    ///
+    /// [`UpdateAssets`]: bevy::asset::UpdateAssets
+    UpdatePropertiesFromAsset,
+
+    /// Gather all removed [`ParticleEffect`] components during the
+    /// [`PostUpdate`] set, to clean-up unused GPU resources.
+    ///
+    /// Systems deleting entities with a [`ParticleEffect`] component should run
+    /// before this set if they want the particle effect is cleaned-up during
+    /// the same frame.
+    ///
+    /// [`ParticleEffect`]: crate::ParticleEffect
+    GatherRemovedEffects,
+
+    /// Prepare effect assets for the extracted effects.
+    PrepareEffectAssets,
+
+    /// Queue the GPU commands for the extracted effects.
+    QueueEffects,
+
+    /// Prepare GPU data for the queued effects.
+    PrepareEffectGpuResources,
+
+    /// Prepare the GPU bind groups once all buffers have been (re-)allocated
+    /// and won't change this frame.
+    PrepareBindGroups,
+}
+
 pub mod main_graph {
     pub mod node {
         use bevy::render::render_graph::RenderLabel;

src/render/mod.rs

@@ -68,58 +68,6 @@ use self::batch::EffectBatches;
 // bytes.
 const INDIRECT_INDEX_SIZE: u32 = 12;
 
-/// Labels for the Hanabi systems.
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, SystemSet)]
-pub enum EffectSystems {
-    /// Tick all effect instances to generate particle spawn counts.
-    ///
-    /// This system runs during the [`PostUpdate`] schedule. Any system which
-    /// modifies an effect spawner should run before this set to ensure the
-    /// spawner takes into account the newly set values during its ticking.
-    TickSpawners,
-
-    /// Compile the effect instances, updating the [`CompiledParticleEffect`]
-    /// components.
-    ///
-    /// This system runs during the [`PostUpdate`] schedule. This is largely an
-    /// internal task which can be ignored by most users.
-    CompileEffects,
-
-    /// Update the properties of the effect instance based on the declared
-    /// properties in the [`EffectAsset`], updating the associated
-    /// [`EffectProperties`] component.
-    ///
-    /// This system runs during Bevy's own [`UpdateAssets`] schedule, after the
-    /// assets have been updated. Any system which modifies an
-    /// [`EffectAsset`]'s declared properties should run before [`UpdateAssets`]
-    /// in order for changes to be taken into account in the same frame.
-    ///
-    /// [`UpdateAssets`]: bevy::asset::UpdateAssets
-    UpdatePropertiesFromAsset,
-
-    /// Gather all removed [`ParticleEffect`] components during the
-    /// [`PostUpdate`] set, to be able to clean-up GPU resources.
-    ///
-    /// Systems deleting entities with a [`ParticleEffect`] component should run
-    /// before this set if they want the particle effect is cleaned-up during
-    /// the same frame.
-    ///
-    /// [`ParticleEffect`]: crate::ParticleEffect
-    GatherRemovedEffects,
-
-    /// Prepare effect assets for the extracted effects.
-    PrepareEffectAssets,
-
-    /// Queue the GPU commands for the extracted effects.
-    QueueEffects,
-
-    /// Prepare GPU data for the queued effects.
-    PrepareEffectGpuResources,
-
-    /// Prepare the bind groups.
-    PrepareBindGroups,
-}
-
 /// Simulation parameters, available to all shaders of all effects.
 #[derive(Debug, Default, Clone, Copy, Resource)]
 pub(crate) struct SimParams {
@@ -1503,7 +1451,11 @@ pub(crate) fn extract_effects(
         let property_layout = asset.property_layout();
 
         let property_data = if let Some(properties) = maybe_properties {
-            if properties.is_changed() {
+            // Note: must check that property layout is not empty, because the
+            // EffectProperties component is marked as changed when added but contains an
+            // empty Vec if there's no property, which would later raise an error if we
+            // don't return None here.
+            if properties.is_changed() && !property_layout.is_empty() {
                 trace!("Detected property change, re-serializing...");
                 Some(properties.serialize(&property_layout))
             } else {