From 8117a672ca4275f82a00badec036920cf9a54c01 Mon Sep 17 00:00:00 2001 From: databasedav <31483365+databasedav@users.noreply.github.com> Date: Thu, 25 Dec 2025 20:14:13 -0800 Subject: [PATCH 01/15] more --- src/builder.rs | 96 ++++++- src/lib.rs | 4 +- src/signal.rs | 665 +++++++++++++++++++++++++++++++++++++++++++--- src/signal_map.rs | 140 ++++++++-- src/signal_vec.rs | 462 +++++++++++++++++++++++++++----- src/utils.rs | 7 +- 6 files changed, 1239 insertions(+), 135 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index 81db7c6..601cb3c 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -2,6 +2,7 @@ use super::{ graph::{SignalHandle, SignalHandles}, signal::{Signal, SignalBuilder, SignalExt}, + signal_map::{SignalMap, SignalMapExt}, signal_vec::{SignalVec, SignalVecExt, VecDiff}, utils::{LazyEntity, SSs, ancestor_map}, }; @@ -17,6 +18,78 @@ use bevy_platform::{ sync::{Arc, Mutex}, }; +/// A type-erased signal that can be registered with a [`World`] at spawn time. +/// +/// This trait enables [`JonmoBuilder::hold_signals`] to accept signals that haven't +/// been registered yet, deferring their registration until the entity is spawned. +/// +/// Use the [`.hold()`](SignalHoldExt::hold) extension method to convert a [`Signal`], +/// [`SignalVec`], or [`SignalMap`] into a `Box`. +pub trait Holdable: Send + Sync + 'static { + /// Register this signal with the world and return its handle. + fn register_holdable(self: Box, world: &mut World) -> SignalHandle; +} + +// Signal wrapper +struct HoldableSignal(S); + +impl Holdable for HoldableSignal { + fn register_holdable(self: Box, world: &mut World) -> SignalHandle { + self.0.register(world) + } +} + +/// Extension trait for converting a [`Signal`] into a [`Holdable`]. +pub trait SignalHoldExt: Signal + Sized + SSs { + /// Convert this signal into a type-erased [`Holdable`] for use with + /// [`JonmoBuilder::hold_signals`]. + fn hold(self) -> Box { + Box::new(HoldableSignal(self)) + } +} + +impl SignalHoldExt for S {} + +// SignalVec wrapper +struct HoldableSignalVec(S); + +impl Holdable for HoldableSignalVec { + fn register_holdable(self: Box, world: &mut World) -> SignalHandle { + self.0.register(world) + } +} + +/// Extension trait for converting a [`SignalVec`] into a [`Holdable`]. +pub trait SignalVecHoldExt: SignalVec + Sized + SSs { + /// Convert this signal vec into a type-erased [`Holdable`] for use with + /// [`JonmoBuilder::hold_signals`]. + fn hold(self) -> Box { + Box::new(HoldableSignalVec(self)) + } +} + +impl SignalVecHoldExt for S {} + +// SignalMap wrapper +struct HoldableSignalMap(S); + +impl Holdable for HoldableSignalMap { + fn register_holdable(self: Box, world: &mut World) -> SignalHandle { + self.0.register(world) + } +} + +/// Extension trait for converting a [`SignalMap`] into a [`Holdable`]. +pub trait SignalMapHoldExt: SignalMap + Sized + SSs { + /// Convert this signal map into a type-erased [`Holdable`] for use with + /// [`JonmoBuilder::hold_signals`]. + fn hold(self) -> Box { + Box::new(HoldableSignalMap(self)) + } +} + +impl SignalMapHoldExt for S {} + fn on_despawn_hook(mut world: DeferredWorld, ctx: HookContext) { let entity = ctx.entity; let fs = world @@ -100,9 +173,25 @@ impl JonmoBuilder { }) } - /// Attach registered [`SignalHandle`]s to this entity for automatic cleanup on despawn. - pub fn hold_signals(self, handles: impl IntoIterator + SSs) -> Self { - self.on_spawn(move |world, entity| add_handles(world, entity, handles)) + /// Attach [`Holdable`] signals to this entity for automatic cleanup on despawn. + /// + /// Use [`.hold()`](SignalHoldExt::hold) to convert a [`Signal`], [`SignalVec`], or + /// [`SignalMap`] into a [`Holdable`]. + /// + /// # Example + /// + /// ```ignore + /// let my_signal = SignalBuilder::from_resource::().map_in(|r| r.value).hold(); + /// let my_signal_vec = my_mutable_vec.signal_vec().hold(); + /// + /// JonmoBuilder::new() + /// .hold_signals([my_signal, my_signal_vec]) + /// ``` + pub fn hold_signals(self, holdables: impl IntoIterator> + SSs) -> Self { + self.on_spawn(move |world, entity| { + let handles: Vec<_> = holdables.into_iter().map(|h| h.register_holdable(world)).collect(); + add_handles(world, entity, handles); + }) } /// Run a function with this builder's [`EntityWorldMut`]. @@ -158,6 +247,7 @@ impl JonmoBuilder { } /// Set the [`LazyEntity`] to this builder's [`Entity`]. + #[track_caller] pub fn lazy_entity(self, entity: LazyEntity) -> Self { self.on_spawn(move |_, e| entity.set(e)) } diff --git a/src/lib.rs b/src/lib.rs index 39b9feb..603ca90 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -115,7 +115,9 @@ impl Plugin for JonmoPlugin { /// `use jonmo::prelude::*;` imports everything one needs to use start using [jonmo](crate). pub mod prelude { #[cfg(feature = "builder")] - pub use crate::builder::JonmoBuilder; + pub use crate::builder::{ + Holdable, JonmoBuilder, SignalHoldExt, SignalMapHoldExt, SignalVecHoldExt, + }; pub use crate::{ JonmoPlugin, graph::SignalHandles, diff --git a/src/signal.rs b/src/signal.rs index 1d996ef..672b5ed 100644 --- a/src/signal.rs +++ b/src/signal.rs @@ -75,12 +75,20 @@ impl Signal for Box + Send + Sync> { /// Signal graph node which takes an input of [`In<()>`] and has no upstreams. See [`SignalBuilder`] /// methods for examples. -#[derive(Clone)] pub struct Source { signal: LazySignal, _marker: PhantomData O>, } +impl Clone for Source { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl Signal for Source where O: 'static, @@ -93,13 +101,25 @@ where } /// Signal graph node which applies a [`System`] to its upstream, see [`.map`](SignalExt::map). -#[derive(Clone)] pub struct Map { upstream: Upstream, signal: LazySignal, _marker: PhantomData O>, } +impl Clone for Map +where + Upstream: Clone, +{ + fn clone(&self) -> Self { + Self { + upstream: self.upstream.clone(), + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl Signal for Map where Upstream: Signal, @@ -117,11 +137,21 @@ where /// Signal graph node which maps its upstream [`Entity`] to a corresponding [`Component`], see /// [.component](SignalExt::component). -#[derive(Clone)] pub struct MapComponent { signal: Map, } +impl Clone for MapComponent +where + Upstream: Clone, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } +} + impl Signal for MapComponent where Upstream: Signal, @@ -136,11 +166,21 @@ where /// Signal graph node which maps its upstream [`Entity`] to a corresponding [`Option`], /// see [.component_option](SignalExt::component_option). -#[derive(Clone)] pub struct ComponentOption { signal: Map>, } +impl Clone for ComponentOption +where + Upstream: Clone, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } +} + impl Signal for ComponentOption where Upstream: Signal, @@ -155,11 +195,21 @@ where /// Signal graph node which maps its upstream [`Entity`] to its [`Changed`] `C` [`Component`], /// see [.component_changed](SignalExt::component_changed). -#[derive(Clone)] pub struct ComponentChanged { signal: Map, } +impl Clone for ComponentChanged +where + Upstream: Clone, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } +} + impl Signal for ComponentChanged where Upstream: Signal, @@ -174,12 +224,23 @@ where /// Signal graph node with maps its upstream [`Entity`] to whether it has some [`Component`], see /// [`.has_component`](SignalExt::has_component). -#[derive(Clone)] pub struct HasComponent { signal: Map, _marker: PhantomData C>, } +impl Clone for HasComponent +where + Upstream: Clone, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl Signal for HasComponent where Upstream: Signal, @@ -194,7 +255,6 @@ where /// Signal graph node which does not forward upstream duplicates, see /// [`.dedupe`](SignalExt::dedupe). -#[derive(Clone)] pub struct Dedupe where Upstream: Signal, @@ -202,6 +262,17 @@ where signal: Map, } +impl Clone for Dedupe +where + Upstream: Signal + Clone, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } +} + impl Signal for Dedupe where Upstream: Signal, @@ -215,12 +286,22 @@ where /// Signal graph node that terminates forever after forwarding a single upstream value, see /// [`.first`](SignalExt::first). -#[derive(Clone)] pub struct First where Upstream: Signal, { - signal: Map, + signal: Take, +} + +impl Clone for First +where + Upstream: Signal + Clone, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } } impl Signal for First @@ -234,8 +315,38 @@ where } } +/// Signal graph node that terminates forever after forwarding `count` upstream values, see +/// [`.take`](SignalExt::take). +pub struct Take +where + Upstream: Signal, +{ + signal: Map, +} + +impl Clone for Take +where + Upstream: Signal + Clone, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } +} + +impl Signal for Take +where + Upstream: Signal, +{ + type Item = Upstream::Item; + + fn register_boxed_signal(self: Box, world: &mut World) -> SignalHandle { + self.signal.register(world) + } +} + /// Signal graph node which combines two upstreams, see [`.combine`](SignalExt::combine). -#[derive(Clone)] pub struct Combine where Left: Signal, @@ -248,6 +359,20 @@ where signal: LazySignal, } +impl Clone for Combine +where + Left: Signal + Clone, + Right: Signal + Clone, +{ + fn clone(&self) -> Self { + Self { + left_wrapper: self.left_wrapper.clone(), + right_wrapper: self.right_wrapper.clone(), + signal: self.signal.clone(), + } + } +} + impl Signal for Combine where Left: Signal, @@ -267,11 +392,21 @@ where /// Signal graph node which outputs equality between its upstream and a fixed value, see /// [`.eq`](SignalExt::eq). -#[derive(Clone)] pub struct Eq { signal: Map, } +impl Clone for Eq +where + Upstream: Clone, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } +} + impl Signal for Eq where Upstream: Signal, @@ -285,11 +420,21 @@ where /// Signal graph node which outputs inequality between its upstream and a fixed value, see /// [`.neq`](SignalExt::neq). -#[derive(Clone)] pub struct Neq { signal: Map, } +impl Clone for Neq +where + Upstream: Clone, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } +} + impl Signal for Neq where Upstream: Signal, @@ -303,7 +448,6 @@ where } /// Signal graph node which applies [`ops::Not`] to its upstream, see [`.not`](SignalExt::not). -#[derive(Clone)] pub struct Not where Upstream: Signal, @@ -313,6 +457,19 @@ where signal: Map::Output>, } +impl Clone for Not +where + Upstream: Signal + Clone, + ::Item: ops::Not, + <::Item as ops::Not>::Output: Clone, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } +} + impl Signal for Not where Upstream: Signal, @@ -327,12 +484,20 @@ where } /// Signal graph node which selectively terminates propagation, see [`.filter`](SignalExt::filter). -#[derive(Clone)] pub struct Filter { signal: LazySignal, _marker: PhantomData Upstream>, } +impl Clone for Filter { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl Signal for Filter where Upstream: Signal, @@ -351,7 +516,6 @@ struct FlattenState { /// Signal graph node which forwards the upstream output [`Signal`]'s output, see /// [`.flatten`](SignalExt::flatten). -#[derive(Clone)] pub struct Flatten where Upstream: Signal, @@ -362,6 +526,19 @@ where _marker: PhantomData (Upstream, Upstream::Item)>, } +impl Clone for Flatten +where + Upstream: Signal, + Upstream::Item: Signal, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl Signal for Flatten where Upstream: Signal, @@ -377,7 +554,6 @@ where /// Signal graph node which maps its upstream to a [`Signal`], forwarding its output, see /// [`.switch`](SignalExt::switch). -#[derive(Clone)] pub struct Switch where Upstream: Signal, @@ -387,6 +563,19 @@ where signal: Flatten>, } +impl Clone for Switch +where + Upstream: Signal, + Other: Signal, + Other::Item: Clone, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } +} + impl Signal for Switch where Upstream: Signal, @@ -402,7 +591,6 @@ where /// Signal graph node which maps its upstream to a [`SignalVec`], see /// [`.switch_signal_vec`](SignalExt::switch_signal_vec). -#[derive(Clone)] pub struct SwitchSignalVec where Upstream: Signal, @@ -412,6 +600,19 @@ where _marker: PhantomData (Upstream, Switched)>, } +impl Clone for SwitchSignalVec +where + Upstream: Signal, + Switched: SignalVec, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalVec for SwitchSignalVec where Upstream: Signal, @@ -427,7 +628,6 @@ where /// Signal graph node which maps its upstream to a [`SignalMap`], see /// [`.switch_signal_map`](SignalExt::switch_signal_map). -#[derive(Clone)] pub struct SwitchSignalMap where Upstream: Signal, @@ -437,6 +637,19 @@ where _marker: PhantomData (Upstream, Switched)>, } +impl Clone for SwitchSignalMap +where + Upstream: Signal, + Switched: SignalMap, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalMap for SwitchSignalMap where Upstream: Signal, @@ -456,7 +669,6 @@ cfg_if::cfg_if! { if #[cfg(feature = "time")] { /// Signal graph node which delays the propagation of subsequent upstream outputs, see /// [`.throttle`](SignalExt::throttle). - #[derive(Clone)] pub struct Throttle where Upstream: Signal, @@ -464,6 +676,17 @@ cfg_if::cfg_if! { signal: Map, } + impl Clone for Throttle + where + Upstream: Signal + Clone, + { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } + } + impl Signal for Throttle where Upstream: Signal, @@ -479,12 +702,20 @@ cfg_if::cfg_if! { /// Signal graph node whose [`System`] depends on its upstream [`bool`] value, see /// [`.map_bool`](SignalExt::map_bool). -#[derive(Clone)] pub struct MapBool { signal: LazySignal, _marker: PhantomData (Upstream, O)>, } +impl Clone for MapBool { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl Signal for MapBool where Upstream: Signal, @@ -499,12 +730,20 @@ where /// Signal graph node whose system only runs when its upstream outputs [`true`], see /// [`.map_true`](SignalExt::map_true). -#[derive(Clone)] pub struct MapTrue { signal: LazySignal, _marker: PhantomData (Upstream, O)>, } +impl Clone for MapTrue { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl Signal for MapTrue where Upstream: Signal, @@ -519,12 +758,20 @@ where /// Signal graph node whose system only runs when its upstream outputs [`false`], see /// [`.map_false`](SignalExt::map_false). -#[derive(Clone)] pub struct MapFalse { signal: LazySignal, _marker: PhantomData (Upstream, O)>, } +impl Clone for MapFalse { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl Signal for MapFalse where Upstream: Signal, @@ -539,12 +786,20 @@ where /// Signal graph node whose [`System`] depends on its upstream [`Option`] value, see /// [`.map_option`](SignalExt::map_option). -#[derive(Clone)] pub struct MapOption { signal: LazySignal, _marker: PhantomData (Upstream, O)>, } +impl Clone for MapOption { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl Signal for MapOption where Upstream: Signal, @@ -559,12 +814,20 @@ where /// Signal graph node whose system only runs when its upstream outputs [`Some`], see /// [`.map_some`](SignalExt::map_some). -#[derive(Clone)] pub struct MapSome { signal: LazySignal, _marker: PhantomData (Upstream, O)>, } +impl Clone for MapSome { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl Signal for MapSome where Upstream: Signal, @@ -579,12 +842,20 @@ where /// Signal graph node whose system only runs when its upstream outputs [`None`], see /// [`.map_none`](SignalExt::map_none). -#[derive(Clone)] pub struct MapNone { signal: LazySignal, _marker: PhantomData (Upstream, O)>, } +impl Clone for MapNone { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl Signal for MapNone where Upstream: Signal, @@ -599,7 +870,6 @@ where /// Signal graph node which maps its upstream [`Vec`] to a [`SignalVec`], see /// [`.to_signal_vec`](SignalExt::to_signal_vec). -#[derive(Clone)] pub struct ToSignalVec where Upstream: Signal, @@ -608,6 +878,18 @@ where _marker: PhantomData Upstream>, } +impl Clone for ToSignalVec +where + Upstream: Signal, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalVec for ToSignalVec where Upstream: Signal>, @@ -623,7 +905,6 @@ where cfg_if::cfg_if! { if #[cfg(feature = "tracing")] { /// Signal graph node that debug logs its upstream's output, see [`.debug`](SignalExt::debug). - #[derive(Clone)] pub struct Debug where Upstream: Signal, @@ -631,6 +912,17 @@ cfg_if::cfg_if! { signal: Map, } + impl Clone for Debug + where + Upstream: Signal + Clone, + { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } + } + impl Signal for Debug where Upstream: Signal, @@ -822,7 +1114,6 @@ where /// although note that all [`Signal`]s are boxed internally regardless. /// /// Inspired by . -#[derive(Clone)] #[allow(missing_docs)] pub enum SignalEither where @@ -833,6 +1124,19 @@ where Right(R), } +impl Clone for SignalEither +where + L: Signal + Clone, + R: Signal + Clone, +{ + fn clone(&self) -> Self { + match self { + Self::Left(left) => Self::Left(left.clone()), + Self::Right(right) => Self::Right(right.clone()), + } + } +} + impl, R: Signal> Signal for SignalEither where L: Signal, @@ -1178,9 +1482,12 @@ pub trait SignalExt: Signal { } } - /// Outputs this [`Signal`]'s first value and then terminates for all subsequent frames. + /// Outputs up to the first `count` values from this [`Signal`], and then terminates for all + /// subsequent frames. /// - /// After the first value is emitted, this signal will stop propagating any further values. + /// After `count` values are emitted, this signal will stop propagating any further values. + /// + /// If `count` is `0`, this will never propagate any values. /// /// # Example /// @@ -1191,28 +1498,54 @@ pub trait SignalExt: Signal { /// SignalBuilder::from_system({ /// move |_: In<()>, mut state: Local| { /// *state += 1; - /// *state / 2 + /// *state /// } /// }) - /// .first(); // outputs `0` then terminates forever + /// .take(2); // outputs `1`, `2`, then terminates forever /// ``` - fn first(self) -> First + fn take(self, count: usize) -> Take where Self: Sized, Self::Item: Clone + 'static, { - First { - signal: self.map(|In(item): In, mut first: Local| { - if *first { + Take { + signal: self.map(move |In(item): In, mut emitted: Local| { + if *emitted >= count { None } else { - *first = true; + *emitted += 1; Some(item) } }), } } + /// Outputs this [`Signal`]'s first value and then terminates for all subsequent frames. + /// + /// After the first value is emitted, this signal will stop propagating any further values. + /// + /// # Example + /// + /// ``` + /// use bevy_ecs::prelude::*; + /// use jonmo::prelude::*; + /// + /// SignalBuilder::from_system({ + /// move |_: In<()>, mut state: Local| { + /// *state += 1; + /// *state / 2 + /// } + /// }) + /// .first(); // outputs `0` then terminates forever + /// ``` + fn first(self) -> First + where + Self: Sized, + Self::Item: Clone + 'static, + { + First { signal: self.take(1) } + } + /// Output this [`Signal`]'s equality with a fixed value. /// /// # Example @@ -2873,7 +3206,7 @@ mod tests { graph::{LazySignalHolder, SignalRegistrationCount}, prelude::{IntoSignalVecEither, SignalVecExt, clone}, signal::{SignalBuilder, SignalExt, Upstream}, - signal_vec::{MutableVecBuilder, VecDiff}, + signal_vec::{MutableVec, MutableVecBuilder, VecDiff}, utils::{LazyEntity, SSs}, }; use core::{convert::identity, fmt}; @@ -3314,6 +3647,36 @@ mod tests { signal.cleanup(app.world_mut()); } + #[test] + fn test_take() { + let mut app = create_test_app(); + app.init_resource::>(); + let counter = Arc::new(Mutex::new(0)); + let values = Arc::new(Mutex::new(vec![10, 20, 30])); + let signal = SignalBuilder::from_system(clone!((values) move |_: In<()>| { + let mut values_lock = values.lock().unwrap(); + if values_lock.is_empty() { + None + } else { + Some(values_lock.remove(0)) + } + })) + .take(2) + .map(clone!((counter) move |In(val): In| { + *counter.lock().unwrap() += 1; + val + })) + .map(capture_output) + .register(app.world_mut()); + app.update(); + app.update(); + app.update(); + assert_eq!(get_output::(app.world()), Some(20)); + assert_eq!(*counter.lock().unwrap(), 2); + assert_eq!(values.lock().unwrap().len(), 0); + signal.cleanup(app.world_mut()); + } + #[test] fn test_combine() { let mut app = create_test_app(); @@ -3604,6 +3967,110 @@ mod tests { crate::signal_vec::tests::cleanup(true); } + #[test] + fn test_switch_signal_vec_initially_empty() { + { + // --- Setup --- + let mut app = create_test_app(); + app.init_resource::>(); + + // Start with an EMPTY list - this tests the `was_initially_empty` code path + let list_a: MutableVec = app.world_mut().into(); + let list_b = MutableVecBuilder::from([100, 200, 300]).spawn(app.world_mut()); + let signal_a = list_a.signal_vec().map_in(identity); + let signal_b = list_b.signal_vec(); + + // A resource to control which list is active. + #[derive(Resource, Clone, Copy, PartialEq, Debug)] + enum ListSelector { + A, + B, + } + + app.insert_resource(ListSelector::A); + + // The control signal reads the resource. + let control_signal = + SignalBuilder::from_system(|_: In<()>, selector: Res| *selector).dedupe(); + + // The signal chain under test. + let switched_signal = control_signal.dedupe().switch_signal_vec( + clone!((signal_a, signal_b) move |In(selector): In| match selector { + ListSelector::A => signal_a.clone().left_either(), + ListSelector::B => signal_b.clone().right_either(), + }), + ); + + // Register the final signal to a capture system. + let handle = switched_signal.for_each(capture_vec_output).register(app.world_mut()); + + // --- Test 1: Initial State with Empty List --- + // The first update should select List A which is empty. + // With an empty initial list, no Replace diff should be emitted. + app.update(); + let diffs = get_and_clear_vec_output::(app.world_mut()); + assert!( + diffs.is_empty(), + "Initial update with empty list should produce no diffs, got: {:?}", + diffs + ); + + // --- Test 2: Push to Empty Active List --- + // A push to the initially empty List A should be forwarded as a Push. + list_a.write(app.world_mut()).push(10); + app.update(); + let diffs = get_and_clear_vec_output::(app.world_mut()); + assert_eq!(diffs.len(), 1, "Push to active list should produce one diff."); + assert_eq!( + diffs[0], + VecDiff::Push { value: 10 }, + "Should forward Push diff from List A." + ); + + // --- Test 3: Another Push --- + list_a.write(app.world_mut()).push(20); + app.update(); + let diffs = get_and_clear_vec_output::(app.world_mut()); + assert_eq!(diffs.len(), 1, "Second push should produce one diff."); + assert_eq!( + diffs[0], + VecDiff::Push { value: 20 }, + "Should forward second Push diff from List A." + ); + + // --- Test 4: Switch to List B (non-empty) --- + *app.world_mut().resource_mut::() = ListSelector::B; + app.update(); + let diffs = get_and_clear_vec_output::(app.world_mut()); + assert_eq!(diffs.len(), 1, "Switching to non-empty list should produce one diff."); + assert_eq!( + diffs[0], + VecDiff::Replace { + values: vec![100, 200, 300] + }, + "Switch should emit a Replace with List B's contents." + ); + + // --- Test 5: Switch Back to List A (now non-empty) --- + // When switching back, List A now has items, so it should emit Replace. + *app.world_mut().resource_mut::() = ListSelector::A; + app.update(); + let diffs = get_and_clear_vec_output::(app.world_mut()); + assert_eq!(diffs.len(), 1, "Switching back to A should produce one diff."); + assert_eq!( + diffs[0], + VecDiff::Replace { + values: vec![10, 20], + }, + "Switching back should Replace with the current state of List A." + ); + + handle.cleanup(app.world_mut()); + } + + crate::signal_vec::tests::cleanup(true); + } + #[test] fn test_switch_signal_map() { use crate::signal_map::{IntoSignalMapEither, MapDiff, MutableBTreeMapBuilder, SignalMapExt}; @@ -3744,6 +4211,128 @@ mod tests { crate::signal_map::tests::cleanup(true); } + #[test] + fn test_switch_signal_map_initially_empty() { + use crate::signal_map::{IntoSignalMapEither, MapDiff, MutableBTreeMap, MutableBTreeMapBuilder, SignalMapExt}; + + #[derive(Resource, Default, core::fmt::Debug)] + struct SignalMapOutput(Vec>) + where + K: SSs + Clone + core::fmt::Debug, + V: SSs + Clone + core::fmt::Debug; + + fn capture_map_output(In(diffs): In>>, mut output: ResMut>) + where + K: SSs + Clone + core::fmt::Debug, + V: SSs + Clone + core::fmt::Debug, + { + output.0.extend(diffs); + } + + fn get_and_clear_map_output(world: &mut World) -> Vec> + where + K: SSs + Clone + core::fmt::Debug, + V: SSs + Clone + core::fmt::Debug, + { + world + .get_resource_mut::>() + .map(|mut res| core::mem::take(&mut res.0)) + .unwrap_or_default() + } + + { + // --- Setup --- + let mut app = create_test_app(); + app.init_resource::>(); + + // Start with an EMPTY map - this tests the `was_initially_empty` code path + let map_a: MutableBTreeMap = app.world_mut().into(); + let map_b = MutableBTreeMapBuilder::from([(1, 100), (2, 200), (3, 300)]).spawn(app.world_mut()); + let signal_a = map_a.signal_map().map_value(|In(x): In| x); + let signal_b = map_b.signal_map(); + + // A resource to control which map is active. + #[derive(Resource, Clone, Copy, PartialEq, Debug)] + enum MapSelector { + A, + B, + } + + app.insert_resource(MapSelector::A); + + // The control signal reads the resource. + let control_signal = SignalBuilder::from_system(|_: In<()>, selector: Res| *selector).dedupe(); + + // The signal chain under test. + let switched_signal = control_signal.dedupe().switch_signal_map( + clone!((signal_a, signal_b) move |In(selector): In| match selector { + MapSelector::A => signal_a.clone().left_either(), + MapSelector::B => signal_b.clone().right_either(), + }), + ); + + // Register the final signal to a capture system. + let handle = switched_signal.for_each(capture_map_output).register(app.world_mut()); + + // --- Test 1: Initial State with Empty Map --- + // The first update should select Map A which is empty. + // With an empty initial map, no Replace diff should be emitted. + app.update(); + let diffs = get_and_clear_map_output::(app.world_mut()); + assert!( + diffs.is_empty(), + "Initial update with empty map should produce no diffs, got: {:?}", + diffs + ); + + // --- Test 2: Insert to Empty Active Map --- + // An insert to the initially empty Map A should be forwarded as an Insert. + map_a.write(app.world_mut()).insert(1, 10); + app.update(); + let diffs = get_and_clear_map_output::(app.world_mut()); + assert_eq!(diffs.len(), 1, "Insert to active map should produce one diff."); + assert!( + matches!(&diffs[0], MapDiff::Insert { key: 1, value: 10 }), + "Should forward Insert diff from Map A." + ); + + // --- Test 3: Another Insert --- + map_a.write(app.world_mut()).insert(2, 20); + app.update(); + let diffs = get_and_clear_map_output::(app.world_mut()); + assert_eq!(diffs.len(), 1, "Second insert should produce one diff."); + assert!( + matches!(&diffs[0], MapDiff::Insert { key: 2, value: 20 }), + "Should forward second Insert diff from Map A." + ); + + // --- Test 4: Switch to Map B (non-empty) --- + *app.world_mut().resource_mut::() = MapSelector::B; + app.update(); + let diffs = get_and_clear_map_output::(app.world_mut()); + assert_eq!(diffs.len(), 1, "Switching to non-empty map should produce one diff."); + assert!( + matches!(&diffs[0], MapDiff::Replace { entries } if entries == &vec![(1, 100), (2, 200), (3, 300)]), + "Switch should emit a Replace with Map B's contents." + ); + + // --- Test 5: Switch Back to Map A (now non-empty) --- + // When switching back, Map A now has items, so it should emit Replace. + *app.world_mut().resource_mut::() = MapSelector::A; + app.update(); + let diffs = get_and_clear_map_output::(app.world_mut()); + assert_eq!(diffs.len(), 1, "Switching back to A should produce one diff."); + assert!( + matches!(&diffs[0], MapDiff::Replace { entries } if entries == &vec![(1, 10), (2, 20)]), + "Switching back should Replace with the current state of Map A." + ); + + handle.cleanup(app.world_mut()); + } + + crate::signal_map::tests::cleanup(true); + } + #[test] fn test_throttle() { let mut app = App::new(); diff --git a/src/signal_map.rs b/src/signal_map.rs index 86fa757..2880fab 100644 --- a/src/signal_map.rs +++ b/src/signal_map.rs @@ -18,7 +18,7 @@ use bevy_platform::{ prelude::*, sync::{Arc, LazyLock, Mutex}, }; -use core::{fmt, marker::PhantomData, ops::Deref, sync::atomic::AtomicUsize}; +use core::{fmt, marker::PhantomData, ops::Deref, sync::atomic::{AtomicBool, AtomicUsize, Ordering as AtomicOrdering}}; use dyn_clone::{DynClone, clone_trait_object}; /// Describes the mutations made to the underlying [`MutableBTreeMap`] that are piped to downstream @@ -158,13 +158,25 @@ impl SignalMap for Box`]s of its /// upstream, see [.for_each](SignalMapExt::for_each). -#[derive(Clone)] pub struct ForEach { upstream: Upstream, signal: LazySignal, _marker: PhantomData O>, } +impl Clone for ForEach +where + Upstream: Clone, +{ + fn clone(&self) -> Self { + Self { + upstream: self.upstream.clone(), + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl Signal for ForEach where Upstream: SignalMap, @@ -182,12 +194,20 @@ where /// Signal graph node which applies a [`System`] to each [`Value`](SignalMap::Value) of its /// upstream, see [`.map_value`](SignalMapExt::map_value). -#[derive(Clone)] pub struct MapValue { signal: LazySignal, _marker: PhantomData (Upstream, O)>, } +impl Clone for MapValue { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalMap for MapValue where Upstream: SignalMap, @@ -204,12 +224,20 @@ where /// Signal graph node which applies a [`System`] to each [`Value`](SignalMap::Value) of its /// upstream, forwarding the output of each resulting [`Signal`], see /// [`.map_value`](SignalMapExt::map_value). -#[derive(Clone)] pub struct MapValueSignal { signal: LazySignal, _marker: PhantomData (Upstream, S)>, } +impl Clone for MapValueSignal { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalMap for MapValueSignal where Upstream: SignalMap, @@ -226,7 +254,6 @@ where /// Signal graph node which maps its upstream [`SignalVec`] to a [`Key`](SignalMap::Key)-lookup /// [`Signal`], see [`.key`](SignalMapExt::key). -#[derive(Clone)] pub struct Key where Upstream: SignalMap, @@ -234,6 +261,17 @@ where inner: ForEach>, } +impl Clone for Key +where + Upstream: SignalMap + Clone, +{ + fn clone(&self) -> Self { + Self { + inner: self.inner.clone(), + } + } +} + impl Signal for Key where Upstream: SignalMap, @@ -251,7 +289,6 @@ cfg_if::cfg_if! { if #[cfg(feature = "tracing")] { /// Signal graph node that debug logs its upstream's "raw" [`Vec`]s, see /// [`.debug`](SignalMapExt::debug). - #[derive(Clone)] pub struct Debug where Upstream: SignalMap, @@ -260,6 +297,17 @@ cfg_if::cfg_if! { signal: ForEach>>, } + impl Clone for Debug + where + Upstream: SignalMap + Clone, + { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } + } + impl SignalMap for Debug where Upstream: SignalMap, @@ -277,12 +325,20 @@ cfg_if::cfg_if! { /// Signal graph node with no upstreams which outputs some [`MutableBTreeMap`]'s sorted /// [`Key`](SignalMap::Key)s as a [`SignalVec`], see /// [`.signal_vec_keys`](MutableBTreeMap::signal_vec_keys). -#[derive(Clone)] pub struct SignalVecKeys { signal: LazySignal, _marker: PhantomData K>, } +impl Clone for SignalVecKeys { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalVec for SignalVecKeys where K: 'static, @@ -296,12 +352,20 @@ where /// Signal graph node which maps its upstream [`MutableBTreeMap`] to a [`SignalVec`] of its sorted /// `(key, value)`s, see [`.signal_vec_entries`](MutableBTreeMap::signal_vec_entries). -#[derive(Clone)] pub struct SignalVecEntries { signal: LazySignal, _marker: PhantomData (K, V)>, } +impl Clone for SignalVecEntries { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalVec for SignalVecEntries where K: 'static, @@ -318,7 +382,6 @@ where /// although note that all [`SignalMap`]s are boxed internally regardless. /// /// Inspired by . -#[derive(Clone)] #[allow(missing_docs)] pub enum SignalMapEither where @@ -329,6 +392,19 @@ where Right(R), } +impl Clone for SignalMapEither +where + L: SignalMap + Clone, + R: SignalMap + Clone, +{ + fn clone(&self) -> Self { + match self { + Self::Left(left) => Self::Left(left.clone()), + Self::Right(right) => Self::Right(right.clone()), + } + } +} + impl SignalMap for SignalMapEither where L: SignalMap, @@ -736,7 +812,7 @@ pub trait SignalMapExt: SignalMap { fn key(self, key: Self::Key) -> Key where Self: Sized, - Self::Key: PartialEq + Clone + SSs, + Self::Key: PartialEq + SSs, Self::Value: Clone + Send + 'static, { Key { @@ -780,6 +856,7 @@ pub trait SignalMapExt: SignalMap { } #[cfg(feature = "tracing")] + #[track_caller] /// Adds debug logging to this [`SignalMap`]'s raw [`MapDiff`] outputs. /// /// # Example @@ -894,8 +971,8 @@ impl SignalMapExt for T where T: SignalMap {} static STALE_MUTABLE_BTREE_MAPS: LazyLock>> = LazyLock::new(Mutex::default); pub(crate) fn despawn_stale_mutable_btree_maps(world: &mut World) { - let mut queue = STALE_MUTABLE_BTREE_MAPS.lock().unwrap(); - for entity in queue.drain(..) { + let queue = STALE_MUTABLE_BTREE_MAPS.lock().unwrap().drain(..).collect::>(); + for entity in queue { world.despawn(entity); } } @@ -1006,7 +1083,7 @@ pub struct MutableBTreeMap { impl Clone for MutableBTreeMap { fn clone(&self) -> Self { - self.references.fetch_add(1, core::sync::atomic::Ordering::SeqCst); + self.references.fetch_add(1, AtomicOrdering::SeqCst); Self { entity: self.entity, references: self.references.clone(), @@ -1017,7 +1094,7 @@ impl Clone for MutableBTreeMap { impl Drop for MutableBTreeMap { fn drop(&mut self) { - if self.references.fetch_sub(1, core::sync::atomic::Ordering::SeqCst) == 1 { + if self.references.fetch_sub(1, AtomicOrdering::SeqCst) == 1 { STALE_MUTABLE_BTREE_MAPS.lock().unwrap().push(self.entity); } } @@ -1025,12 +1102,20 @@ impl Drop for MutableBTreeMap { /// Signal graph node with no upstreams which forwards [`Vec>`]s flushed from some /// source [`MutableBTreeMap`], see [`MutableBTreeMap::signal_map`]. -#[derive(Clone)] pub struct Source { signal: LazySignal, _marker: PhantomData (K, V)>, } +impl Clone for Source { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalMap for Source where K: 'static, @@ -1124,15 +1209,26 @@ impl MutableBTreeMap { let was_initially_empty = self_.read(&*world).is_empty(); let replay_entity = LazyEntity::new(); - let replay_system = clone!((self_, replay_entity) move |In(upstream_diffs): In>>, replay_onces: Query<&ReplayOnce, Allow>, mutable_btree_map_datas: Query<&MutableBTreeMapData>| { + let is_first_replay = Arc::new(AtomicBool::new(true)); + let replay_system = clone!((self_, replay_entity, is_first_replay) move |In(upstream_diffs): In>>, replay_onces: Query<&ReplayOnce, Allow>, mutable_btree_map_datas: Query<&MutableBTreeMapData>| { if replay_onces.contains(*replay_entity) { - if !was_initially_empty { - let initial_map = self_.read(&mutable_btree_map_datas); - Some(vec![MapDiff::Replace { entries: initial_map.iter().map(|(k, v)| (k.clone(), v.clone())).collect() }]) - } else if upstream_diffs.is_empty() { - None + let first_replay = is_first_replay.swap(false, AtomicOrdering::SeqCst); + if first_replay && was_initially_empty { + // Initial replay with empty map - just forward upstream diffs + if upstream_diffs.is_empty() { + None + } else { + Some(upstream_diffs) + } } else { - Some(upstream_diffs) + // Either non-empty initial state, or subsequent replay (e.g., switch_signal_map) + // Always emit Replace with current state + let current_map = self_.read(&mutable_btree_map_datas); + if current_map.is_empty() { + None + } else { + Some(vec![MapDiff::Replace { entries: current_map.iter().map(|(k, v)| (k.clone(), v.clone())).collect() }]) + } } } else if upstream_diffs.is_empty() { None } else { Some(upstream_diffs) } }); diff --git a/src/signal_vec.rs b/src/signal_vec.rs index 7fadd11..84b7440 100644 --- a/src/signal_vec.rs +++ b/src/signal_vec.rs @@ -18,7 +18,7 @@ use bevy_platform::{ prelude::*, sync::{Arc, LazyLock, Mutex}, }; -use core::{cmp::Ordering, fmt, marker::PhantomData, ops::Deref, sync::atomic::AtomicUsize}; +use core::{cmp::Ordering, fmt, marker::PhantomData, ops::Deref, sync::atomic::{AtomicBool, AtomicUsize, Ordering as AtomicOrdering}}; use dyn_clone::{DynClone, clone_trait_object}; /// Describes the mutations made to the underlying [`MutableVec`] that are piped to downstream @@ -173,12 +173,20 @@ impl SignalVec for Box + Send + Sync /// Signal graph node with no upstreams which forwards [`Vec>`]s flushed from some source /// [`MutableVec`], see [`MutableVec::signal_vec`]. -#[derive(Clone)] pub struct Source { signal: LazySignal, _marker: PhantomData T>, } +impl Clone for Source { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalVec for Source where T: 'static, @@ -192,13 +200,25 @@ where /// Signal graph node which applies a [`System`] directly to the "raw" [`Vec`]s of its /// upstream, see [`.for_each`](SignalVecExt::for_each). -#[derive(Clone)] pub struct ForEach { upstream: Upstream, signal: LazySignal, _marker: PhantomData O>, } +impl Clone for ForEach +where + Upstream: Clone, +{ + fn clone(&self) -> Self { + Self { + upstream: self.upstream.clone(), + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl Signal for ForEach where Upstream: SignalVec, @@ -216,12 +236,20 @@ where /// Signal graph node which applies a [`System`] to each [`Item`](SignalVec::Item) of its upstream, /// see [`.map`](SignalVecExt::map). -#[derive(Clone)] pub struct Map { signal: LazySignal, _marker: PhantomData (Upstream, O)>, } +impl Clone for Map { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalVec for Map where Upstream: SignalVec, @@ -240,12 +268,20 @@ struct ItemIndex(usize); /// Signal graph node which applies a [`System`] to each [`Item`](SignalVec::Item) of its upstream, /// forwarding the output of each resulting [`Signal`], see /// [`.map_signal`](SignalVecExt::map_signal). -#[derive(Clone)] pub struct MapSignal { signal: LazySignal, _marker: PhantomData (Upstream, S)>, } +impl Clone for MapSignal { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalVec for MapSignal where Upstream: SignalVec, @@ -263,17 +299,17 @@ fn find_index<'a>(indices: impl Iterator, index: usize) -> usiz indices.take(index).filter(|x| **x).count() } -fn filter_helper( +/// Helper for `filter` - clones input to pass to system, returns original if predicate passes. +#[track_caller] +fn filter_helper( world: &mut World, diffs: Vec>>, - system: SystemId, - f: impl Fn(T::Inner<'static>, O) -> Option, + system: SystemId, indices: &mut Vec, -) -> Option>> +) -> Option>>> where T: SystemInput + 'static, T::Inner<'static>: Clone, - O: 'static, { let mut output = vec![]; for diff in diffs { @@ -282,23 +318,21 @@ where *indices = Vec::with_capacity(values.len()); let mut output = Vec::with_capacity(values.len()); for input in values { - let value = world + let include = world .run_system_with(system, input.clone()) - .ok() - .and_then(|output| f(input, output)); - indices.push(value.is_some()); - if let Some(value) = value { - output.push(value); + .unwrap_or(false); + indices.push(include); + if include { + output.push(input); } } Some(VecDiff::Replace { values: output }) } VecDiff::InsertAt { index, value } => { - if let Some(value) = world + let include = world .run_system_with(system, value.clone()) - .ok() - .and_then(|output| f(value, output)) - { + .unwrap_or(false); + if include { indices.insert(index, true); Some(VecDiff::InsertAt { index: find_index(indices.iter(), index), @@ -310,11 +344,10 @@ where } } VecDiff::UpdateAt { index, value } => { - if let Some(value) = world + let include = world .run_system_with(system, value.clone()) - .ok() - .and_then(|output| f(value, output)) - { + .unwrap_or(false); + if include { if indices[index] { Some(VecDiff::UpdateAt { index: find_index(indices.iter(), index), @@ -365,11 +398,128 @@ where } } VecDiff::Push { value } => { - if let Some(value) = world + let include = world .run_system_with(system, value.clone()) - .ok() - .and_then(|output| f(value, output)) - { + .unwrap_or(false); + if include { + indices.push(true); + Some(VecDiff::Push { value }) + } else { + indices.push(false); + None + } + } + VecDiff::Pop => { + if indices.pop().expect("can't pop from empty vec") { + Some(VecDiff::Pop) + } else { + None + } + } + VecDiff::Clear => { + indices.clear(); + Some(VecDiff::Clear) + } + }; + if let Some(diff) = diff_option { + output.push(diff); + } + } + if output.is_empty() { None } else { Some(output) } +} + +/// Helper for `filter_map` - does NOT clone input, system takes ownership and returns mapped value. +#[track_caller] +fn filter_map_helper( + world: &mut World, + diffs: Vec>>, + system: SystemId>, + indices: &mut Vec, +) -> Option>> +where + T: SystemInput + 'static, + O: 'static, +{ + let mut output = vec![]; + for diff in diffs { + let diff_option = match diff { + VecDiff::Replace { values } => { + *indices = Vec::with_capacity(values.len()); + let mut output = Vec::with_capacity(values.len()); + for input in values { + let mapped = world.run_system_with(system, input).ok().flatten(); + indices.push(mapped.is_some()); + if let Some(value) = mapped { + output.push(value); + } + } + Some(VecDiff::Replace { values: output }) + } + VecDiff::InsertAt { index, value } => { + if let Some(value) = world.run_system_with(system, value).ok().flatten() { + indices.insert(index, true); + Some(VecDiff::InsertAt { + index: find_index(indices.iter(), index), + value, + }) + } else { + indices.insert(index, false); + None + } + } + VecDiff::UpdateAt { index, value } => { + if let Some(value) = world.run_system_with(system, value).ok().flatten() { + if indices[index] { + Some(VecDiff::UpdateAt { + index: find_index(indices.iter(), index), + value, + }) + } else { + indices[index] = true; + Some(VecDiff::InsertAt { + index: find_index(indices.iter(), index), + value, + }) + } + } else if indices[index] { + indices[index] = false; + Some(VecDiff::RemoveAt { + index: find_index(indices.iter(), index), + }) + } else { + None + } + } + VecDiff::Move { old_index, new_index } => { + // Determine if the item being moved is part of the filtered set. + let was_included = indices[old_index]; + + // Calculate the filtered old and new indices BEFORE and AFTER mutating `indices`. + let filtered_old_index = find_index(indices.iter(), old_index); + let temp_val = indices.remove(old_index); + indices.insert(new_index, temp_val); + let filtered_new_index = find_index(indices.iter(), new_index); + + if was_included { + Some(VecDiff::Move { + old_index: filtered_old_index, + new_index: filtered_new_index, + }) + } else { + None + } + } + VecDiff::RemoveAt { index } => { + if indices.remove(index) { + Some(VecDiff::RemoveAt { + index: find_index(indices.iter(), index), + }) + } else { + None + } + } + VecDiff::Push { value } => { + if let Some(value) = world.run_system_with(system, value).ok().flatten() { indices.push(true); Some(VecDiff::Push { value }) } else { @@ -398,12 +548,20 @@ where /// Signal graph node which selectively forwards upstream [`Item`](SignalVec::Item)s, see /// [`.filter`](SignalVecExt::filter). -#[derive(Clone)] pub struct Filter { signal: LazySignal, _marker: PhantomData Upstream>, } +impl Clone for Filter { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalVec for Filter where Upstream: SignalVec, @@ -417,7 +575,6 @@ where /// Signal graph node which transforms and selectively forwards upstream [`Item`](SignalVec::Item)s, /// see [`.filter_map`](SignalVecExt::filter_map). -#[derive(Clone)] pub struct FilterMap where Upstream: SignalVec, @@ -426,6 +583,18 @@ where _marker: PhantomData (Upstream, O)>, } +impl Clone for FilterMap +where + Upstream: SignalVec, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalVec for FilterMap where Upstream: SignalVec, @@ -543,12 +712,20 @@ fn spawn_filter_signal( /// Signal graph node which selectively forwards upstream [`Item`](SignalVec::Item)s depending on a /// [`Signal`], see [`.filter_signal`](SignalVecExt::filter_signal). -#[derive(Clone)] pub struct FilterSignal { signal: LazySignal, _marker: PhantomData Upstream>, } +impl Clone for FilterSignal { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalVec for FilterSignal where Upstream: SignalVec, @@ -569,7 +746,6 @@ struct EnumerateState { /// Signal graph node which prepends an index signal to each upstream [`Item`](SignalVec::Item), see /// [`.enumerate`](SignalVecExt::enumerate). -#[derive(Clone)] pub struct Enumerate where Upstream: SignalVec, @@ -579,6 +755,18 @@ where _marker: PhantomData Upstream>, } +impl Clone for Enumerate +where + Upstream: SignalVec, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalVec for Enumerate where Upstream: SignalVec, @@ -592,7 +780,6 @@ where /// Signal graph node which collects upstream [`Item`](SignalVec::Item)s into a single [`Vec`], see /// [`.to_signal`](SignalVecExt::to_signal). -#[derive(Clone)] pub struct ToSignal where Upstream: SignalVec, @@ -600,6 +787,17 @@ where signal: ForEach>, } +impl Clone for ToSignal +where + Upstream: SignalVec + Clone, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } +} + impl Signal for ToSignal where Upstream: SignalVec, @@ -613,7 +811,6 @@ where /// Signal graph node which outputs whether its upstream is populated, see /// [`.is_empty`](SignalVecExt::is_empty). -#[derive(Clone)] pub struct IsEmpty where Upstream: SignalVec, @@ -622,6 +819,18 @@ where signal: super::signal::Map, bool>, } +impl Clone for IsEmpty +where + Upstream: SignalVec + Clone, + Upstream::Item: Clone, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } +} + impl Signal for IsEmpty where Upstream: SignalVec, @@ -635,7 +844,6 @@ where } /// Signal graph node which outputs its upstream's length, see [`.len`](SignalVecExt::len). -#[derive(Clone)] pub struct Len where Upstream: SignalVec, @@ -644,6 +852,18 @@ where signal: super::signal::Map, usize>, } +impl Clone for Len +where + Upstream: SignalVec + Clone, + Upstream::Item: Clone, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } +} + impl Signal for Len where Upstream: SignalVec, @@ -657,7 +877,6 @@ where } /// Signal graph node which outputs its upstream's sum, see [`.sum`](SignalVecExt::sum). -#[derive(Clone)] pub struct Sum where Upstream: SignalVec, @@ -665,6 +884,17 @@ where signal: super::signal::Map, Upstream::Item>, } +impl Clone for Sum +where + Upstream: SignalVec + Clone, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } +} + impl Signal for Sum where Upstream: SignalVec, @@ -676,14 +906,24 @@ where } } -#[derive(Clone)] enum LrDiff { Left(Vec>), Right(Vec>), } +impl Clone for LrDiff +where + T: Clone, +{ + fn clone(&self) -> Self { + match self { + Self::Left(left) => Self::Left(left.clone()), + Self::Right(right) => Self::Right(right.clone()), + } + } +} + /// Signal graph node which concatenates its upstreams, see [`.chain`](SignalVecExt::chain). -#[derive(Clone)] pub struct Chain where Left: SignalVec, @@ -694,6 +934,20 @@ where signal: LazySignal, } +impl Clone for Chain +where + Left: SignalVec + Clone, + Right: SignalVec + Clone, +{ + fn clone(&self) -> Self { + Self { + left_wrapper: self.left_wrapper.clone(), + right_wrapper: self.right_wrapper.clone(), + signal: self.signal.clone(), + } + } +} + impl SignalVec for Chain where Left: SignalVec, @@ -713,7 +967,6 @@ where /// Signal graph node that places a separator between each upstream [`Item`](SignalVec::Item), see /// [`.intersperse`](SignalVecExt::intersperse). -#[derive(Clone)] pub struct Intersperse where Upstream: SignalVec, @@ -721,6 +974,17 @@ where signal: ForEach>>, } +impl Clone for Intersperse +where + Upstream: SignalVec + Clone, +{ + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } +} + impl SignalVec for Intersperse where Upstream: SignalVec, @@ -758,12 +1022,20 @@ impl Default for IntersperseState { /// Signal graph node that places a [`System`]-dependent separator between each upstream /// [`Item`](SignalVec::Item), see [`.intersperse_with`](SignalVecExt::intersperse_with). -#[derive(Clone)] pub struct IntersperseWith { signal: LazySignal, _marker: PhantomData Upstream>, } +impl Clone for IntersperseWith { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalVec for IntersperseWith where Upstream: SignalVec, @@ -777,12 +1049,20 @@ where /// Signal graph node that sorts each upstream [`Item`](SignalVec::Item) based on a comparison /// [`System`], see [`.sort_by`](SignalVecExt::sort_by). -#[derive(Clone)] pub struct SortBy { signal: LazySignal, _marker: PhantomData Upstream>, } +impl Clone for SortBy { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalVec for SortBy where Upstream: SignalVec, @@ -796,12 +1076,20 @@ where /// Signal graph node that sorts each upstream [`Item`](SignalVec::Item) based on a key extraction /// [`System`], see [`.sort_by_key`](SignalVecExt::sort_by_key). -#[derive(Clone)] pub struct SortByKey { signal: LazySignal, _marker: PhantomData Upstream>, } +impl Clone for SortByKey { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + impl SignalVec for SortByKey where Upstream: SignalVec, @@ -813,7 +1101,7 @@ where } } -/* #[derive(Clone)] +/* struct FlattenItem { processor_handle: SignalHandle, values: Vec, @@ -994,7 +1282,6 @@ cfg_if::cfg_if! { if #[cfg(feature = "tracing")] { /// Signal graph node that debug logs its upstream's "raw" [`Vec`]s, see /// [`.debug`](SignalVecExt::debug). - #[derive(Clone)] pub struct Debug where Upstream: SignalVec, @@ -1002,6 +1289,17 @@ cfg_if::cfg_if! { signal: ForEach>>, } + impl Clone for Debug + where + Upstream: SignalVec + Clone, + { + fn clone(&self) -> Self { + Self { + signal: self.signal.clone(), + } + } + } + impl SignalVec for Debug where Upstream: SignalVec, @@ -1019,7 +1317,6 @@ cfg_if::cfg_if! { /// although note that all [`SignalVec`]s are boxed internally regardless. /// /// Inspired by . -#[derive(Clone)] #[allow(missing_docs)] pub enum SignalVecEither where @@ -1030,6 +1327,19 @@ where Right(R), } +impl Clone for SignalVecEither +where + L: SignalVec + Clone, + R: SignalVec + Clone, +{ + fn clone(&self) -> Self { + match self { + Self::Left(left) => Self::Left(left.clone()), + Self::Right(right) => Self::Right(right.clone()), + } + } +} + impl, R: SignalVec> SignalVec for SignalVecEither where L: SignalVec, @@ -1140,6 +1450,7 @@ pub trait SignalVecExt: SignalVec { /// [`.for_each`](SignalVecExt::for_each), returns a [`Signal`], not a [`SignalVec`], since the /// output type need not be an [`Option>`]. If the [`System`] logic is infallible, /// wrapping the result in an option is unnecessary. + #[track_caller] fn for_each(self, system: F) -> ForEach where Self: Sized, @@ -1310,7 +1621,7 @@ pub trait SignalVecExt: SignalVec { fn map_signal(self, system: F) -> MapSignal where Self: Sized, - Self::Item: Clone + SSs, + Self::Item: SSs, S: Signal + 'static + Clone, S::Item: Clone + Send + Sync, F: IntoSystem, S, M> + SSs, @@ -1593,15 +1904,7 @@ pub trait SignalVecExt: SignalVec { let SignalHandle(signal) = self .for_each::>, _, _, _>( move |In(diffs): In>>, world: &mut World, mut indices: Local>| { - filter_helper( - world, - diffs, - system, - |item, include| { - if include { Some(item) } else { None } - }, - &mut indices, - ) + filter_helper(world, diffs, system, &mut indices) }, ) .register(world); @@ -1631,10 +1934,11 @@ pub trait SignalVecExt: SignalVec { /// .signal_vec() /// .filter_map(|In(s): In<&'static str>| s.parse::().ok()); // outputs `SignalVec -> [1, 5]` /// ``` + #[track_caller] fn filter_map(self, system: F) -> FilterMap where Self: Sized, - Self::Item: Clone + 'static, + Self::Item: 'static, O: Clone + 'static, F: IntoSystem, Option, M> + SSs, { @@ -1643,7 +1947,7 @@ pub trait SignalVecExt: SignalVec { let SignalHandle(signal) = self .for_each::>, _, _, _>( move |In(diffs): In>>, world: &mut World, mut indices: Local>| { - filter_helper(world, diffs, system, |_, mapped| mapped, &mut indices) + filter_map_helper(world, diffs, system, &mut indices) }, ) .register(world); @@ -3490,6 +3794,7 @@ pub trait SignalVecExt: SignalVec { } */ #[cfg(feature = "tracing")] + #[track_caller] /// Adds debug logging to this [`SignalVec`]'s raw [`VecDiff`] outputs. /// /// # Example @@ -3747,7 +4052,7 @@ pub struct MutableVec { impl Clone for MutableVec { fn clone(&self) -> Self { - self.references.fetch_add(1, core::sync::atomic::Ordering::SeqCst); + self.references.fetch_add(1, AtomicOrdering::SeqCst); Self { entity: self.entity, references: self.references.clone(), @@ -3758,7 +4063,7 @@ impl Clone for MutableVec { impl Drop for MutableVec { fn drop(&mut self) { - if self.references.fetch_sub(1, core::sync::atomic::Ordering::SeqCst) == 1 { + if self.references.fetch_sub(1, AtomicOrdering::SeqCst) == 1 { STALE_MUTABLE_VECS.lock().unwrap().push(self.entity); } } @@ -3797,15 +4102,26 @@ impl MutableVec { let was_initially_empty = self_.read(&*world).is_empty(); let replay_entity = LazyEntity::new(); - let replay_system = clone!((self_, replay_entity) move |In(upstream_diffs): In>>, replay_onces: Query<&ReplayOnce, Allow>, mutable_vec_datas: Query<&MutableVecData>| { + let is_first_replay = Arc::new(AtomicBool::new(true)); + let replay_system = clone!((self_, replay_entity, is_first_replay) move |In(upstream_diffs): In>>, replay_onces: Query<&ReplayOnce, Allow>, mutable_vec_datas: Query<&MutableVecData>| { if replay_onces.contains(*replay_entity) { - if !was_initially_empty { - let initial_vec = self_.read(&mutable_vec_datas).to_vec(); - Some(vec![VecDiff::Replace { values: initial_vec }]) - } else if upstream_diffs.is_empty() { - None + let first_replay = is_first_replay.swap(false, AtomicOrdering::SeqCst); + if first_replay && was_initially_empty { + // Initial replay with empty vec - just forward upstream diffs + if upstream_diffs.is_empty() { + None + } else { + Some(upstream_diffs) + } } else { - Some(upstream_diffs) + // Either non-empty initial state, or subsequent replay (e.g., switch_signal_vec) + // Always emit Replace with current state + let current_vec = self_.read(&mutable_vec_datas).to_vec(); + if current_vec.is_empty() { + None + } else { + Some(vec![VecDiff::Replace { values: current_vec }]) + } } } else if upstream_diffs.is_empty() { None } else { Some(upstream_diffs) } }); @@ -3831,8 +4147,8 @@ impl MutableVec { } pub(crate) fn despawn_stale_mutable_vecs(world: &mut World) { - let mut queue = STALE_MUTABLE_VECS.lock().unwrap(); - for entity in queue.drain(..) { + let queue = STALE_MUTABLE_VECS.lock().unwrap().drain(..).collect::>(); + for entity in queue { world.despawn(entity); } } @@ -6568,13 +6884,21 @@ pub(crate) mod tests { /// A helper enum to represent the items in our final interspersed vector, /// allowing us to distinguish between original items and generated separators. - #[derive(Clone)] enum ItemOrSep { Item(u32), // The separator variant holds the *unregistered* index signal. Sep(crate::signal::Source>), } + impl Clone for ItemOrSep { + fn clone(&self) -> Self { + match self { + Self::Item(i) => Self::Item(*i), + Self::Sep(s) => Self::Sep(s.clone()), + } + } + } + // Manual `Debug` implementation is required because `Source` does not implement it. impl fmt::Debug for ItemOrSep { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { diff --git a/src/utils.rs b/src/utils.rs index dc820ab..86ee50e 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -11,7 +11,7 @@ pub use enclose::enclose as clone; #[derive(Default, Clone)] pub struct LazyEntity(Arc>); -const LAZY_ENTITY_GET_ERROR: &str = "EntityHolder does not contain an Entity"; +const LAZY_ENTITY_GET_ERROR: &str = "LazyEntity does not contain an Entity"; impl LazyEntity { #[allow(missing_docs)] @@ -20,11 +20,13 @@ impl LazyEntity { } /// Set the [`Entity`], panicking if it was already set. + #[track_caller] pub fn set(&self, entity: Entity) { - self.0.set(entity).expect("EntityHolder already contains an Entity"); + self.0.set(entity).expect("LazyEntity already contains an Entity"); } /// Get the [`Entity`], panicking if it was not set. + #[track_caller] pub fn get(&self) -> Entity { self.0.get().copied().expect(LAZY_ENTITY_GET_ERROR) } @@ -33,6 +35,7 @@ impl LazyEntity { impl Deref for LazyEntity { type Target = Entity; + #[track_caller] fn deref(&self) -> &Self::Target { self.0.get().expect(LAZY_ENTITY_GET_ERROR) } From 31f8de10489e2c29ed3e1d1ba74a939a79c32324 Mon Sep 17 00:00:00 2001 From: databasedav <31483365+databasedav@users.noreply.github.com> Date: Thu, 25 Dec 2025 22:12:26 -0800 Subject: [PATCH 02/15] clean --- src/builder.rs | 236 ++++++++++++++++++++++++---------------------- src/lib.rs | 4 +- src/signal.rs | 9 +- src/signal_map.rs | 7 +- src/signal_vec.rs | 33 +++---- 5 files changed, 147 insertions(+), 142 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index 601cb3c..9ac0480 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -18,109 +18,6 @@ use bevy_platform::{ sync::{Arc, Mutex}, }; -/// A type-erased signal that can be registered with a [`World`] at spawn time. -/// -/// This trait enables [`JonmoBuilder::hold_signals`] to accept signals that haven't -/// been registered yet, deferring their registration until the entity is spawned. -/// -/// Use the [`.hold()`](SignalHoldExt::hold) extension method to convert a [`Signal`], -/// [`SignalVec`], or [`SignalMap`] into a `Box`. -pub trait Holdable: Send + Sync + 'static { - /// Register this signal with the world and return its handle. - fn register_holdable(self: Box, world: &mut World) -> SignalHandle; -} - -// Signal wrapper -struct HoldableSignal(S); - -impl Holdable for HoldableSignal { - fn register_holdable(self: Box, world: &mut World) -> SignalHandle { - self.0.register(world) - } -} - -/// Extension trait for converting a [`Signal`] into a [`Holdable`]. -pub trait SignalHoldExt: Signal + Sized + SSs { - /// Convert this signal into a type-erased [`Holdable`] for use with - /// [`JonmoBuilder::hold_signals`]. - fn hold(self) -> Box { - Box::new(HoldableSignal(self)) - } -} - -impl SignalHoldExt for S {} - -// SignalVec wrapper -struct HoldableSignalVec(S); - -impl Holdable for HoldableSignalVec { - fn register_holdable(self: Box, world: &mut World) -> SignalHandle { - self.0.register(world) - } -} - -/// Extension trait for converting a [`SignalVec`] into a [`Holdable`]. -pub trait SignalVecHoldExt: SignalVec + Sized + SSs { - /// Convert this signal vec into a type-erased [`Holdable`] for use with - /// [`JonmoBuilder::hold_signals`]. - fn hold(self) -> Box { - Box::new(HoldableSignalVec(self)) - } -} - -impl SignalVecHoldExt for S {} - -// SignalMap wrapper -struct HoldableSignalMap(S); - -impl Holdable for HoldableSignalMap { - fn register_holdable(self: Box, world: &mut World) -> SignalHandle { - self.0.register(world) - } -} - -/// Extension trait for converting a [`SignalMap`] into a [`Holdable`]. -pub trait SignalMapHoldExt: SignalMap + Sized + SSs { - /// Convert this signal map into a type-erased [`Holdable`] for use with - /// [`JonmoBuilder::hold_signals`]. - fn hold(self) -> Box { - Box::new(HoldableSignalMap(self)) - } -} - -impl SignalMapHoldExt for S {} - -fn on_despawn_hook(mut world: DeferredWorld, ctx: HookContext) { - let entity = ctx.entity; - let fs = world - .get_mut::(entity) - .unwrap() - .0 - .drain(..) - .collect::>(); - for f in fs { - f(&mut world, entity); - } -} - -#[allow(clippy::type_complexity)] -#[derive(Component)] -#[component(on_remove = on_despawn_hook)] -struct OnDespawnCallbacks(Vec>); - -fn add_handles(world: &mut World, entity: Entity, handles: I) -where - I: IntoIterator, -{ - if let Ok(mut entity) = world.get_entity_mut(entity) - && let Some(mut existing_handles) = entity.get_mut::() - { - for handle in handles { - existing_handles.add(handle); - } - } -} - // TODO: the fluent interface link breaks cargo fmt ?? /// A thin facade over a Bevy [`Entity`] enabling the ergonomic registration of /// reactive components and children using a declarative @@ -174,18 +71,33 @@ impl JonmoBuilder { } /// Attach [`Holdable`] signals to this entity for automatic cleanup on despawn. - /// + /// /// Use [`.hold()`](SignalHoldExt::hold) to convert a [`Signal`], [`SignalVec`], or /// [`SignalMap`] into a [`Holdable`]. - /// + /// /// # Example - /// - /// ```ignore - /// let my_signal = SignalBuilder::from_resource::().map_in(|r| r.value).hold(); - /// let my_signal_vec = my_mutable_vec.signal_vec().hold(); - /// - /// JonmoBuilder::new() - /// .hold_signals([my_signal, my_signal_vec]) + /// + /// ``` + /// use bevy_ecs::prelude::*; + /// use jonmo::prelude::*; + /// + /// #[derive(Resource, Clone)] + /// struct MyResource { + /// value: i32, + /// } + /// + /// let my_signal = SignalBuilder::from_resource::() + /// .map_in(|r: MyResource| println!("{}", r.value)) + /// .hold(); + /// + /// let mut world = World::new(); + /// let my_mutable_vec = MutableVecBuilder::from([1, 2, 3]).spawn(&mut world); + /// let my_signal_vec = my_mutable_vec + /// .signal_vec() + /// .map_in(|item: i32| println!("{}", item)) + /// .hold(); + /// + /// JonmoBuilder::new().hold_signals([my_signal, my_signal_vec]); /// ``` pub fn hold_signals(self, holdables: impl IntoIterator> + SSs) -> Self { self.on_spawn(move |world, entity| { @@ -826,6 +738,106 @@ impl JonmoBuilder { } } +/// A type-erased signal that can be registered with a [`World`] at spawn time. +/// +/// This trait enables [`JonmoBuilder::hold_signals`] to accept signals that haven't +/// been registered yet, deferring their registration until the entity is spawned. +/// +/// Use the [`.hold()`](SignalHoldExt::hold) extension method to convert a [`Signal`], +/// [`SignalVec`], or [`SignalMap`] into a `Box`. +pub trait Holdable: Send + Sync + 'static { + /// Register this signal with the world and return its handle. + fn register_holdable(self: Box, world: &mut World) -> SignalHandle; +} + +struct HoldableSignal(S); + +impl Holdable for HoldableSignal { + fn register_holdable(self: Box, world: &mut World) -> SignalHandle { + self.0.register(world) + } +} + +/// Extension trait for converting a [`Signal`] into a [`Holdable`]. +pub trait SignalHoldExt: Signal + Sized + SSs { + /// Convert this signal into a type-erased [`Holdable`] for use with + /// [`JonmoBuilder::hold_signals`]. + fn hold(self) -> Box { + Box::new(HoldableSignal(self)) + } +} + +impl SignalHoldExt for S {} + +struct HoldableSignalVec(S); + +impl Holdable for HoldableSignalVec { + fn register_holdable(self: Box, world: &mut World) -> SignalHandle { + self.0.register(world) + } +} + +/// Extension trait for converting a [`SignalVec`] into a [`Holdable`]. +pub trait SignalVecHoldExt: SignalVec + Sized + SSs { + /// Convert this signal vec into a type-erased [`Holdable`] for use with + /// [`JonmoBuilder::hold_signals`]. + fn hold(self) -> Box { + Box::new(HoldableSignalVec(self)) + } +} + +impl SignalVecHoldExt for S {} + +struct HoldableSignalMap(S); + +impl Holdable for HoldableSignalMap { + fn register_holdable(self: Box, world: &mut World) -> SignalHandle { + self.0.register(world) + } +} + +/// Extension trait for converting a [`SignalMap`] into a [`Holdable`]. +pub trait SignalMapHoldExt: SignalMap + Sized + SSs { + /// Convert this signal map into a type-erased [`Holdable`] for use with + /// [`JonmoBuilder::hold_signals`]. + fn hold(self) -> Box { + Box::new(HoldableSignalMap(self)) + } +} + +impl SignalMapHoldExt for S {} + +fn on_despawn_hook(mut world: DeferredWorld, ctx: HookContext) { + let entity = ctx.entity; + let fs = world + .get_mut::(entity) + .unwrap() + .0 + .drain(..) + .collect::>(); + for f in fs { + f(&mut world, entity); + } +} + +#[allow(clippy::type_complexity)] +#[derive(Component)] +#[component(on_remove = on_despawn_hook)] +struct OnDespawnCallbacks(Vec>); + +fn add_handles(world: &mut World, entity: Entity, handles: I) +where + I: IntoIterator, +{ + if let Ok(mut entity) = world.get_entity_mut(entity) + && let Some(mut existing_handles) = entity.get_mut::() + { + for handle in handles { + existing_handles.add(handle); + } + } +} + fn offset(i: usize, child_block_populations: &[usize]) -> usize { child_block_populations[..i].iter().sum() } diff --git a/src/lib.rs b/src/lib.rs index 603ca90..827781e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -115,9 +115,7 @@ impl Plugin for JonmoPlugin { /// `use jonmo::prelude::*;` imports everything one needs to use start using [jonmo](crate). pub mod prelude { #[cfg(feature = "builder")] - pub use crate::builder::{ - Holdable, JonmoBuilder, SignalHoldExt, SignalMapHoldExt, SignalVecHoldExt, - }; + pub use crate::builder::{Holdable, JonmoBuilder, SignalHoldExt, SignalMapHoldExt, SignalVecHoldExt}; pub use crate::{ JonmoPlugin, graph::SignalHandles, diff --git a/src/signal.rs b/src/signal.rs index 672b5ed..2cd26ee 100644 --- a/src/signal.rs +++ b/src/signal.rs @@ -2779,20 +2779,19 @@ pub trait SignalExt: Signal { } } - // TODO: why won't doctest compile ? #[cfg(feature = "tracing")] #[track_caller] /// Adds debug logging to this [`Signal`]'s ouptut. /// /// # Example /// - /// ```ignore + /// ``` /// use bevy_ecs::prelude::*; /// use jonmo::prelude::*; /// /// SignalBuilder::from_system(|_: In<()>| 1) /// .debug() // logs `1` - /// .map_in(|x: i32| x * 2); + /// .map_in(|x: i32| x * 2) /// .debug(); // logs `2` /// ``` fn debug(self) -> Debug @@ -4059,9 +4058,7 @@ mod tests { assert_eq!(diffs.len(), 1, "Switching back to A should produce one diff."); assert_eq!( diffs[0], - VecDiff::Replace { - values: vec![10, 20], - }, + VecDiff::Replace { values: vec![10, 20] }, "Switching back should Replace with the current state of List A." ); diff --git a/src/signal_map.rs b/src/signal_map.rs index 2880fab..587f8a3 100644 --- a/src/signal_map.rs +++ b/src/signal_map.rs @@ -18,7 +18,12 @@ use bevy_platform::{ prelude::*, sync::{Arc, LazyLock, Mutex}, }; -use core::{fmt, marker::PhantomData, ops::Deref, sync::atomic::{AtomicBool, AtomicUsize, Ordering as AtomicOrdering}}; +use core::{ + fmt, + marker::PhantomData, + ops::Deref, + sync::atomic::{AtomicBool, AtomicUsize, Ordering as AtomicOrdering}, +}; use dyn_clone::{DynClone, clone_trait_object}; /// Describes the mutations made to the underlying [`MutableBTreeMap`] that are piped to downstream diff --git a/src/signal_vec.rs b/src/signal_vec.rs index 84b7440..096575c 100644 --- a/src/signal_vec.rs +++ b/src/signal_vec.rs @@ -18,7 +18,13 @@ use bevy_platform::{ prelude::*, sync::{Arc, LazyLock, Mutex}, }; -use core::{cmp::Ordering, fmt, marker::PhantomData, ops::Deref, sync::atomic::{AtomicBool, AtomicUsize, Ordering as AtomicOrdering}}; +use core::{ + cmp::Ordering, + fmt, + marker::PhantomData, + ops::Deref, + sync::atomic::{AtomicBool, AtomicUsize, Ordering as AtomicOrdering}, +}; use dyn_clone::{DynClone, clone_trait_object}; /// Describes the mutations made to the underlying [`MutableVec`] that are piped to downstream @@ -300,7 +306,6 @@ fn find_index<'a>(indices: impl Iterator, index: usize) -> usiz } /// Helper for `filter` - clones input to pass to system, returns original if predicate passes. -#[track_caller] fn filter_helper( world: &mut World, diffs: Vec>>, @@ -318,9 +323,7 @@ where *indices = Vec::with_capacity(values.len()); let mut output = Vec::with_capacity(values.len()); for input in values { - let include = world - .run_system_with(system, input.clone()) - .unwrap_or(false); + let include = world.run_system_with(system, input.clone()).unwrap_or(false); indices.push(include); if include { output.push(input); @@ -329,9 +332,7 @@ where Some(VecDiff::Replace { values: output }) } VecDiff::InsertAt { index, value } => { - let include = world - .run_system_with(system, value.clone()) - .unwrap_or(false); + let include = world.run_system_with(system, value.clone()).unwrap_or(false); if include { indices.insert(index, true); Some(VecDiff::InsertAt { @@ -344,9 +345,7 @@ where } } VecDiff::UpdateAt { index, value } => { - let include = world - .run_system_with(system, value.clone()) - .unwrap_or(false); + let include = world.run_system_with(system, value.clone()).unwrap_or(false); if include { if indices[index] { Some(VecDiff::UpdateAt { @@ -398,9 +397,7 @@ where } } VecDiff::Push { value } => { - let include = world - .run_system_with(system, value.clone()) - .unwrap_or(false); + let include = world.run_system_with(system, value.clone()).unwrap_or(false); if include { indices.push(true); Some(VecDiff::Push { value }) @@ -429,7 +426,6 @@ where } /// Helper for `filter_map` - does NOT clone input, system takes ownership and returns mapped value. -#[track_caller] fn filter_map_helper( world: &mut World, diffs: Vec>>, @@ -1450,7 +1446,6 @@ pub trait SignalVecExt: SignalVec { /// [`.for_each`](SignalVecExt::for_each), returns a [`Signal`], not a [`SignalVec`], since the /// output type need not be an [`Option>`]. If the [`System`] logic is infallible, /// wrapping the result in an option is unnecessary. - #[track_caller] fn for_each(self, system: F) -> ForEach where Self: Sized, @@ -1934,7 +1929,6 @@ pub trait SignalVecExt: SignalVec { /// .signal_vec() /// .filter_map(|In(s): In<&'static str>| s.parse::().ok()); // outputs `SignalVec -> [1, 5]` /// ``` - #[track_caller] fn filter_map(self, system: F) -> FilterMap where Self: Sized, @@ -2925,7 +2919,6 @@ pub trait SignalVecExt: SignalVec { } // TODO: the example is clearly a copout ... - // TODO: why won't doctest compile ? /// Place the [`Item`](SignalVec::Item) output by the [`System`] between adjacent items of this /// [`SignalVec`]; the [`System`] takes [`In`] a [`Signal>`] which outputs /// the index of the corresponding [`Item`](SignalVec::Item) or [`None`] if it has been removed. @@ -2934,13 +2927,13 @@ pub trait SignalVecExt: SignalVec { /// /// # Example /// - /// ```ignore + /// ``` /// use bevy_ecs::prelude::*; /// use jonmo::prelude::*; /// use jonmo::signal::{Dedupe, Source}; /// /// let mut world = World::new(); - /// MutableVec::from([1, 2, 3]).spawn(&mut world).signal_vec().intersperse_with(|_: In>| 0); // outputs `SignalVec -> [1, 0, 2, 0, 3]` + /// MutableVecBuilder::from([1, 2, 3]).spawn(&mut world).signal_vec().intersperse_with(|_: In>>| 0); // outputs `SignalVec -> [1, 0, 2, 0, 3]` /// ``` fn intersperse_with(self, separator_system: F) -> IntersperseWith where From 2ec12420c492b76d325fbf86eddb22bd7357afba Mon Sep 17 00:00:00 2001 From: databasedav <31483365+databasedav@users.noreply.github.com> Date: Thu, 25 Dec 2025 22:34:55 -0800 Subject: [PATCH 03/15] again --- CHANGELOG.md | 14 +++ src/builder.rs | 221 ++++++++++++++++++++++++++++++++++++++++++++++ src/signal_vec.rs | 164 ++++++++-------------------------- 3 files changed, 272 insertions(+), 127 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cfc4b9..fcc11d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,20 @@ the format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) ## unreleased +### changed + +- `JonmoBuilder::hold_signals` takes `Box`s instead of `SignalHandles` +- removed output `Clone` bound from `SignalExt::filter_map` + +### added +- `track_caller` derive for panicking `LazyEntity` methods + +### fixed + +- deadlock when despawning `MutableVec/Map`s during another `MutableVec/Map` despawn +- initially empty `MutableVec/Map`s work as expected when output to `.switch_signal_vec/map` +- `SignalVecExt::debug` and `SignalMapExt::debug` now log correct code location + # 0.5.0 (2025-12-19) ### changed diff --git a/src/builder.rs b/src/builder.rs index 9ac0480..984c0f2 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -3495,4 +3495,225 @@ mod tests { cleanup() } + + #[test] + fn test_on_despawn() { + // --- 1. Setup --- + let mut app = create_test_app(); + + // A resource to track whether the on_despawn callback was executed. + #[derive(Resource, Default, Clone)] + struct DespawnTracker(Arc>>); + + app.init_resource::(); + + // --- 2. Test Basic Callback Execution --- + let tracker = app.world().resource::().clone(); + let builder1 = JonmoBuilder::new().on_despawn({ + let tracker = tracker.clone(); + move |_world, entity| { + tracker.0.lock().unwrap().push((entity, "callback1".to_string())); + } + }); + + let entity1 = builder1.spawn(app.world_mut()); + app.update(); + + // Callback should not have been called yet. + assert!( + tracker.0.lock().unwrap().is_empty(), + "on_despawn callback should not run before despawn." + ); + + // Despawn the entity. + app.world_mut().entity_mut(entity1).despawn(); + app.update(); + + // Callback should have been called with the correct entity. + let tracker_guard = tracker.0.lock().unwrap(); + assert_eq!(tracker_guard.len(), 1, "Callback should have been called exactly once."); + assert_eq!(tracker_guard[0], (entity1, "callback1".to_string())); + drop(tracker_guard); + tracker.0.lock().unwrap().clear(); + + // --- 3. Test Multiple Callbacks on Same Entity --- + let builder2 = JonmoBuilder::new() + .on_despawn({ + let tracker = tracker.clone(); + move |_world, entity| { + tracker.0.lock().unwrap().push((entity, "first".to_string())); + } + }) + .on_despawn({ + let tracker = tracker.clone(); + move |_world, entity| { + tracker.0.lock().unwrap().push((entity, "second".to_string())); + } + }) + .on_despawn({ + let tracker = tracker.clone(); + move |_world, entity| { + tracker.0.lock().unwrap().push((entity, "third".to_string())); + } + }); + + let entity2 = builder2.spawn(app.world_mut()); + app.update(); + + // Despawn the entity. + app.world_mut().entity_mut(entity2).despawn(); + app.update(); + + // All three callbacks should have been called. + let tracker_guard = tracker.0.lock().unwrap(); + assert_eq!(tracker_guard.len(), 3, "All three callbacks should have been called."); + // Callbacks are stored in a Vec and called in order. + assert_eq!(tracker_guard[0], (entity2, "first".to_string())); + assert_eq!(tracker_guard[1], (entity2, "second".to_string())); + assert_eq!(tracker_guard[2], (entity2, "third".to_string())); + drop(tracker_guard); + tracker.0.lock().unwrap().clear(); + + // --- 4. Test Multi-Entity Independence --- + let builder3 = JonmoBuilder::new().on_despawn({ + let tracker = tracker.clone(); + move |_world, entity| { + tracker.0.lock().unwrap().push((entity, "entity3".to_string())); + } + }); + let entity3 = builder3.spawn(app.world_mut()); + + let builder4 = JonmoBuilder::new().on_despawn({ + let tracker = tracker.clone(); + move |_world, entity| { + tracker.0.lock().unwrap().push((entity, "entity4".to_string())); + } + }); + let entity4 = builder4.spawn(app.world_mut()); + app.update(); + + // Despawn only entity3. + app.world_mut().entity_mut(entity3).despawn(); + app.update(); + + // Only entity3's callback should have been called. + let tracker_guard = tracker.0.lock().unwrap(); + assert_eq!(tracker_guard.len(), 1, "Only one callback should have been called."); + assert_eq!(tracker_guard[0], (entity3, "entity3".to_string())); + drop(tracker_guard); + tracker.0.lock().unwrap().clear(); + + // Now despawn entity4. + app.world_mut().entity_mut(entity4).despawn(); + app.update(); + + // entity4's callback should have been called. + let tracker_guard = tracker.0.lock().unwrap(); + assert_eq!(tracker_guard.len(), 1, "entity4's callback should have been called."); + assert_eq!(tracker_guard[0], (entity4, "entity4".to_string())); + drop(tracker_guard); + tracker.0.lock().unwrap().clear(); + + // --- 5. Test DeferredWorld Access --- + // Verify that the callback can actually interact with the world. + #[derive(Resource, Default)] + struct DespawnCounter(u32); + + app.init_resource::(); + + let builder5 = JonmoBuilder::new().on_despawn(|world, _entity| { + world.resource_mut::().0 += 1; + }); + + let entity5 = builder5.spawn(app.world_mut()); + app.update(); + + assert_eq!( + app.world().resource::().0, + 0, + "Counter should be 0 before despawn." + ); + + app.world_mut().entity_mut(entity5).despawn(); + app.update(); + + assert_eq!( + app.world().resource::().0, + 1, + "Counter should be incremented by on_despawn callback." + ); + + // Spawn and despawn another entity to verify counter increments again. + let builder6 = JonmoBuilder::new().on_despawn(|world, _entity| { + world.resource_mut::().0 += 10; + }); + let entity6 = builder6.spawn(app.world_mut()); + app.update(); + app.world_mut().entity_mut(entity6).despawn(); + app.update(); + + assert_eq!( + app.world().resource::().0, + 11, + "Counter should be 1 + 10 = 11 after second despawn." + ); + + // --- 6. Test Combination with Other Builder Methods --- + // Ensure on_despawn works correctly with other builder methods like insert and child. + #[derive(Component)] + struct TestMarker; + + let child_despawn_tracker = Arc::new(Mutex::new(Vec::new())); + let parent_despawn_tracker = Arc::new(Mutex::new(Vec::new())); + + let child_tracker = child_despawn_tracker.clone(); + let parent_tracker = parent_despawn_tracker.clone(); + + let builder_parent = JonmoBuilder::new() + .insert(TestMarker) + .on_despawn(move |_world, entity| { + parent_tracker.lock().unwrap().push(entity); + }) + .child(JonmoBuilder::new().on_despawn(move |_world, entity| { + child_tracker.lock().unwrap().push(entity); + })); + + let parent_entity = builder_parent.spawn(app.world_mut()); + app.update(); + + // Get the child entity. + let children: Vec = app + .world() + .get::(parent_entity) + .map(|c| c.iter().collect()) + .unwrap_or_default(); + assert_eq!(children.len(), 1, "Parent should have one child."); + let child_entity = children[0]; + + // Despawn the parent (which should also despawn the child due to Bevy's hierarchy). + app.world_mut().entity_mut(parent_entity).despawn(); + app.update(); + + // Both callbacks should have been called. + assert_eq!( + parent_despawn_tracker.lock().unwrap().len(), + 1, + "Parent's on_despawn should have been called." + ); + assert_eq!( + parent_despawn_tracker.lock().unwrap()[0], + parent_entity, + "Parent callback should receive parent entity." + ); + assert_eq!( + child_despawn_tracker.lock().unwrap().len(), + 1, + "Child's on_despawn should have been called when parent is despawned." + ); + assert_eq!( + child_despawn_tracker.lock().unwrap()[0], + child_entity, + "Child callback should receive child entity." + ); + } } diff --git a/src/signal_vec.rs b/src/signal_vec.rs index 096575c..0f7a573 100644 --- a/src/signal_vec.rs +++ b/src/signal_vec.rs @@ -305,35 +305,30 @@ fn find_index<'a>(indices: impl Iterator, index: usize) -> usiz indices.take(index).filter(|x| **x).count() } -/// Helper for `filter` - clones input to pass to system, returns original if predicate passes. -fn filter_helper( - world: &mut World, - diffs: Vec>>, - system: SystemId, +/// Generic helper for filter/filter_map operations on VecDiff streams. +/// `process` takes a value and returns `Option` - `Some` means include, `None` means exclude. +fn filter_generic_helper( + diffs: Vec>, indices: &mut Vec, -) -> Option>>> -where - T: SystemInput + 'static, - T::Inner<'static>: Clone, -{ + mut process: impl FnMut(T) -> Option, +) -> Option>> { let mut output = vec![]; for diff in diffs { let diff_option = match diff { VecDiff::Replace { values } => { *indices = Vec::with_capacity(values.len()); - let mut output = Vec::with_capacity(values.len()); + let mut out_values = Vec::with_capacity(values.len()); for input in values { - let include = world.run_system_with(system, input.clone()).unwrap_or(false); - indices.push(include); - if include { - output.push(input); + let mapped = process(input); + indices.push(mapped.is_some()); + if let Some(value) = mapped { + out_values.push(value); } } - Some(VecDiff::Replace { values: output }) + Some(VecDiff::Replace { values: out_values }) } VecDiff::InsertAt { index, value } => { - let include = world.run_system_with(system, value.clone()).unwrap_or(false); - if include { + if let Some(value) = process(value) { indices.insert(index, true); Some(VecDiff::InsertAt { index: find_index(indices.iter(), index), @@ -345,8 +340,7 @@ where } } VecDiff::UpdateAt { index, value } => { - let include = world.run_system_with(system, value.clone()).unwrap_or(false); - if include { + if let Some(value) = process(value) { if indices[index] { Some(VecDiff::UpdateAt { index: find_index(indices.iter(), index), @@ -397,8 +391,7 @@ where } } VecDiff::Push { value } => { - let include = world.run_system_with(system, value.clone()).unwrap_or(false); - if include { + if let Some(value) = process(value) { indices.push(true); Some(VecDiff::Push { value }) } else { @@ -425,7 +418,25 @@ where if output.is_empty() { None } else { Some(output) } } -/// Helper for `filter_map` - does NOT clone input, system takes ownership and returns mapped value. +fn filter_helper( + world: &mut World, + diffs: Vec>>, + system: SystemId, + indices: &mut Vec, +) -> Option>>> +where + T: SystemInput + 'static, + T::Inner<'static>: Clone, +{ + filter_generic_helper(diffs, indices, |value| { + if world.run_system_with(system, value.clone()).unwrap_or(false) { + Some(value) + } else { + None + } + }) +} + fn filter_map_helper( world: &mut World, diffs: Vec>>, @@ -436,110 +447,9 @@ where T: SystemInput + 'static, O: 'static, { - let mut output = vec![]; - for diff in diffs { - let diff_option = match diff { - VecDiff::Replace { values } => { - *indices = Vec::with_capacity(values.len()); - let mut output = Vec::with_capacity(values.len()); - for input in values { - let mapped = world.run_system_with(system, input).ok().flatten(); - indices.push(mapped.is_some()); - if let Some(value) = mapped { - output.push(value); - } - } - Some(VecDiff::Replace { values: output }) - } - VecDiff::InsertAt { index, value } => { - if let Some(value) = world.run_system_with(system, value).ok().flatten() { - indices.insert(index, true); - Some(VecDiff::InsertAt { - index: find_index(indices.iter(), index), - value, - }) - } else { - indices.insert(index, false); - None - } - } - VecDiff::UpdateAt { index, value } => { - if let Some(value) = world.run_system_with(system, value).ok().flatten() { - if indices[index] { - Some(VecDiff::UpdateAt { - index: find_index(indices.iter(), index), - value, - }) - } else { - indices[index] = true; - Some(VecDiff::InsertAt { - index: find_index(indices.iter(), index), - value, - }) - } - } else if indices[index] { - indices[index] = false; - Some(VecDiff::RemoveAt { - index: find_index(indices.iter(), index), - }) - } else { - None - } - } - VecDiff::Move { old_index, new_index } => { - // Determine if the item being moved is part of the filtered set. - let was_included = indices[old_index]; - - // Calculate the filtered old and new indices BEFORE and AFTER mutating `indices`. - let filtered_old_index = find_index(indices.iter(), old_index); - let temp_val = indices.remove(old_index); - indices.insert(new_index, temp_val); - let filtered_new_index = find_index(indices.iter(), new_index); - - if was_included { - Some(VecDiff::Move { - old_index: filtered_old_index, - new_index: filtered_new_index, - }) - } else { - None - } - } - VecDiff::RemoveAt { index } => { - if indices.remove(index) { - Some(VecDiff::RemoveAt { - index: find_index(indices.iter(), index), - }) - } else { - None - } - } - VecDiff::Push { value } => { - if let Some(value) = world.run_system_with(system, value).ok().flatten() { - indices.push(true); - Some(VecDiff::Push { value }) - } else { - indices.push(false); - None - } - } - VecDiff::Pop => { - if indices.pop().expect("can't pop from empty vec") { - Some(VecDiff::Pop) - } else { - None - } - } - VecDiff::Clear => { - indices.clear(); - Some(VecDiff::Clear) - } - }; - if let Some(diff) = diff_option { - output.push(diff); - } - } - if output.is_empty() { None } else { Some(output) } + filter_generic_helper(diffs, indices, |value| { + world.run_system_with(system, value).ok().flatten() + }) } /// Signal graph node which selectively forwards upstream [`Item`](SignalVec::Item)s, see From 57112badba54b54256fcc1afb7d8bc842bca392f Mon Sep 17 00:00:00 2001 From: databasedav <31483365+databasedav@users.noreply.github.com> Date: Thu, 25 Dec 2025 22:43:25 -0800 Subject: [PATCH 04/15] more --- CHANGELOG.md | 1 + src/signal.rs | 4 ---- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fcc11d7..4654088 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ the format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) - removed output `Clone` bound from `SignalExt::filter_map` ### added +- `SignalExt::take` - `track_caller` derive for panicking `LazyEntity` methods ### fixed diff --git a/src/signal.rs b/src/signal.rs index 2cd26ee..08c35d7 100644 --- a/src/signal.rs +++ b/src/signal.rs @@ -1485,8 +1485,6 @@ pub trait SignalExt: Signal { /// Outputs up to the first `count` values from this [`Signal`], and then terminates for all /// subsequent frames. /// - /// After `count` values are emitted, this signal will stop propagating any further values. - /// /// If `count` is `0`, this will never propagate any values. /// /// # Example @@ -1522,8 +1520,6 @@ pub trait SignalExt: Signal { /// Outputs this [`Signal`]'s first value and then terminates for all subsequent frames. /// - /// After the first value is emitted, this signal will stop propagating any further values. - /// /// # Example /// /// ``` From d179e2728b2c25344d1464d99c3bd5884a8bb4bc Mon Sep 17 00:00:00 2001 From: databasedav <31483365+databasedav@users.noreply.github.com> Date: Fri, 26 Dec 2025 00:33:30 -0800 Subject: [PATCH 05/15] add cloning warning for jonmobuilder --- src/builder.rs | 55 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/src/builder.rs b/src/builder.rs index 984c0f2..3782899 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -13,6 +13,8 @@ use bevy_ecs::{ system::{IntoObserverSystem, RunSystemOnce}, world::DeferredWorld, }; +#[cfg(debug_assertions)] +use bevy_log::warn; use bevy_platform::{ prelude::*, sync::{Arc, Mutex}, @@ -31,13 +33,64 @@ use bevy_platform::{ /// [`DomBuilder`](https://docs.rs/dominator/latest/dominator/struct.DomBuilder.html), /// and [haalka](https://github.com/databasedav/haalka)'s /// [`NodeBuilder`](https://docs.rs/haalka/latest/haalka/node_builder/struct.NodeBuilder.html). -#[derive(Clone, Default)] +/// +/// # `Clone` semantics +/// +/// This type implements [`Clone`] **only** to satisfy trait bounds required by signal combinators. +/// **Cloning [`JonmoBuilder`]s at runtime is a bug.** See [`JonmoBuilder::clone`] for +/// details. +#[derive(Default)] pub struct JonmoBuilder { #[allow(clippy::type_complexity)] on_spawns: Arc>>>, child_block_populations: Arc>>, } +impl Clone for JonmoBuilder { + /// # Warning + /// + /// This clone implementation exists **only** to satisfy trait bounds required by signal + /// combinators (e.g., [`SignalExt::map`], [`SignalVecExt::filter_map`]). **Cloning + /// [`JonmoBuilder`]s at runtime is a bug and will lead to unexpected behavior.** + /// + /// Clones share internal on-spawn hooks via [`Arc`]. These hooks are one-shot ([`FnOnce`]) + /// and are consumed when the builder is spawned. Spawning one clone will affect all other + /// clones. + /// + /// Use factory functions instead if you need reusable UI templates: + /// + /// ``` + /// use bevy_ecs::prelude::*; + /// use jonmo::prelude::*; + /// + /// fn my_widget(label: &str) -> JonmoBuilder { + /// JonmoBuilder::new().insert(Name::new(label.to_string())) + /// // ... other configuration + /// } + /// + /// // Correct: each call creates a fresh builder + /// let mut world = World::new(); + /// let widget1 = my_widget("First").spawn(&mut world); + /// let widget2 = my_widget("Second").spawn(&mut world); + /// ``` + #[track_caller] + fn clone(&self) -> Self { + #[cfg(debug_assertions)] + warn!( + "Cloning `JonmoBuilder` at {} is a bug! `JonmoBuilder`'s `Clone` shares internal on-spawn \ + hook queues via `Arc`. These hooks are one-shot (`FnOnce`) and are consumed on spawn. \ + Spawning one clone will affect all other clones. Use factory functions instead if you \ + need reusable UI templates.", + core::panic::Location::caller() + ); + + Self { + on_spawns: self.on_spawns.clone(), + child_block_populations: self.child_block_populations.clone(), + } + } +} + impl From for JonmoBuilder { fn from(bundle: T) -> Self { Self::new().insert(bundle) From 7f49ade83cbbf95edc1dfbef96f1447bebf9a30c Mon Sep 17 00:00:00 2001 From: databasedav <31483365+databasedav@users.noreply.github.com> Date: Fri, 26 Dec 2025 00:38:17 -0800 Subject: [PATCH 06/15] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4654088..4f1d0f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ the format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) ### added - `SignalExt::take` - `track_caller` derive for panicking `LazyEntity` methods +- warning that cloning `JonmoBuilder`s at runtime is a bug ### fixed From 102619ea00034826461618f3d3e443cc57754610 Mon Sep 17 00:00:00 2001 From: databasedav <31483365+databasedav@users.noreply.github.com> Date: Sat, 27 Dec 2025 00:10:51 -0800 Subject: [PATCH 07/15] always warn --- src/builder.rs | 2 -- src/signal_vec.rs | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index 3782899..f039569 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -13,7 +13,6 @@ use bevy_ecs::{ system::{IntoObserverSystem, RunSystemOnce}, world::DeferredWorld, }; -#[cfg(debug_assertions)] use bevy_log::warn; use bevy_platform::{ prelude::*, @@ -75,7 +74,6 @@ impl Clone for JonmoBuilder { /// ``` #[track_caller] fn clone(&self) -> Self { - #[cfg(debug_assertions)] warn!( "Cloning `JonmoBuilder` at {} is a bug! `JonmoBuilder`'s `Clone` shares internal on-spawn \ hook queues via `Arc`. These hooks are one-shot (`FnOnce`) and are consumed on spawn. \ diff --git a/src/signal_vec.rs b/src/signal_vec.rs index 0f7a573..d494846 100644 --- a/src/signal_vec.rs +++ b/src/signal_vec.rs @@ -2483,7 +2483,7 @@ pub trait SignalVecExt: SignalVec { /// /// let mut world = World::new(); /// let mut vec = MutableVecBuilder::from([1]).spawn(&mut world); - /// let signal = vec.signal_vec().is_empty(); + /// let signal = vec.signal_vec().len(); /// // `signal` outputs `1` /// vec.write(&mut world).pop(); /// // `signal` outputs `0` @@ -2508,7 +2508,7 @@ pub trait SignalVecExt: SignalVec { /// /// let mut world = World::new(); /// let mut vec = MutableVecBuilder::from([1, 2, 3]).spawn(&mut world); - /// let signal = vec.signal_vec().is_empty(); + /// let signal = vec.signal_vec().sum(); /// // `signal` outputs `6` /// vec.write(&mut world).push(4); /// // `signal` outputs `10` From 2ce66e099792f9a30fd98e1f34b598269490f0ee Mon Sep 17 00:00:00 2001 From: databasedav <31483365+databasedav@users.noreply.github.com> Date: Thu, 15 Jan 2026 17:43:15 -0800 Subject: [PATCH 08/15] check before topo sort --- CHANGELOG.md | 6 +- src/builder.rs | 6 +- src/signal.rs | 241 +++++-- src/signal_map.rs | 13 +- src/signal_vec.rs | 1711 ++++++++++++++++++++++++++++++--------------- 5 files changed, 1336 insertions(+), 641 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f1d0f2..0ba784e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,8 +16,8 @@ the format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) ### fixed -- deadlock when despawning `MutableVec/Map`s during another `MutableVec/Map` despawn -- initially empty `MutableVec/Map`s work as expected when output to `.switch_signal_vec/map` +- deadlock when despawning `MutableVec/BTreeMap`s during another `MutableVec/BTreeMap` despawn +- initially empty `MutableVec/BTreeMap`s work as expected when output to `.switch_signal_vec/map` - `SignalVecExt::debug` and `SignalMapExt::debug` now log correct code location # 0.5.0 (2025-12-19) @@ -25,7 +25,7 @@ the format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) ### changed - `.entity_sync` renamed to `.lazy_entity` -- `SignalExt::combine` always `.clone`s its latest upstream outputs instead of `.take`-ing them +- `SignalExt::combine` emits its latest upstream outputs every frame, unlike previously, when it only emitted on frames where the latest output pair was yet to be emitted ### added diff --git a/src/builder.rs b/src/builder.rs index f039569..184d142 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -29,9 +29,7 @@ use bevy_platform::{ /// `&mut World` or [`Commands`]. /// /// Port of [Dominator](https://github.com/Pauan/rust-dominator)'s -/// [`DomBuilder`](https://docs.rs/dominator/latest/dominator/struct.DomBuilder.html), -/// and [haalka](https://github.com/databasedav/haalka)'s -/// [`NodeBuilder`](https://docs.rs/haalka/latest/haalka/node_builder/struct.NodeBuilder.html). +/// [`DomBuilder`](https://docs.rs/dominator/latest/dominator/struct.DomBuilder.html). /// /// # `Clone` semantics /// @@ -2009,7 +2007,7 @@ mod tests { let signal_factory = |entity_signal: crate::signal::Source| { // Combine the entity signal with the global source signal. entity_signal - .combine(SignalBuilder::from_resource::().dedupe()) + .with(SignalBuilder::from_resource::().dedupe()) // The `.map` combinator gives us access to other `SystemParam`s, like `Query`. .map( |In((entity, source)): In<(Entity, SignalSource)>, names: Query<&Name>| { diff --git a/src/signal.rs b/src/signal.rs index 08c35d7..a542e11 100644 --- a/src/signal.rs +++ b/src/signal.rs @@ -346,8 +346,8 @@ where } } -/// Signal graph node which combines two upstreams, see [`.combine`](SignalExt::combine). -pub struct Combine +/// Signal graph node which combines two upstreams, see [`.with`](SignalExt::with). +pub struct With where Left: Signal, Right: Signal, @@ -359,7 +359,7 @@ where signal: LazySignal, } -impl Clone for Combine +impl Clone for With where Left: Signal + Clone, Right: Signal + Clone, @@ -373,7 +373,7 @@ where } } -impl Signal for Combine +impl Signal for With where Left: Signal, Right: Signal, @@ -509,11 +509,6 @@ where } } -#[derive(Component)] -struct FlattenState { - value: Option, -} - /// Signal graph node which forwards the upstream output [`Signal`]'s output, see /// [`.flatten`](SignalExt::flatten). pub struct Flatten @@ -1662,9 +1657,9 @@ pub trait SignalExt: Signal { /// /// let signal_1 = SignalBuilder::from_system(|_: In<()>| 1); /// let signal_2 = SignalBuilder::from_system(|_: In<()>| 2); - /// signal_1.combine(signal_2); // outputs `(1, 2)` + /// signal_1.with(signal_2); // outputs `(1, 2)` /// ``` - fn combine(self, other: Other) -> Combine + fn with(self, other: Other) -> With where Self: Sized, Other: Signal, @@ -1691,7 +1686,7 @@ pub trait SignalExt: Signal { } }, ); - Combine { + With { left_wrapper, right_wrapper, signal, @@ -1732,14 +1727,18 @@ pub trait SignalExt: Signal { Self::Item: Signal + Clone + 'static, ::Item: Clone + Send + Sync, { + #[derive(Component)] + struct FlattenState { + value: Option, + } + let signal = LazySignal::new(move |world: &mut World| { - // 1. The state entity that holds the latest value. This is the communication channel between the - // dynamic inner signals and the static output signal. + // 1. State entity that holds the latest value, serving as the communication channel + // between dynamic inner signals and the static output signal. let reader_entity = LazyEntity::new(); - // 2. The final output signal (reader). Its ONLY job is to read the state component and propagate - // the value. It has no upstream dependencies in the graph; it is triggered manually by the - // forwarder. + // 2. The output signal (reader). Reads from state and propagates. Has no upstream + // dependencies; triggered manually by the forwarder. let reader_system = *SignalBuilder::from_system::<::Item, _, _, _>( clone!((reader_entity) move |_: In<()>, mut query: Query<&mut FlattenState<::Item>, Allow>| { if let Ok(mut state) = query.get_mut(reader_entity.get()) { @@ -1755,45 +1754,32 @@ pub trait SignalExt: Signal { .entity_mut(*reader_system) .insert(FlattenState::<::Item> { value: None }); - // 3. This is the "subscription manager" system. It reacts to the outer signal emitting new inner - // signals. + // 3. The subscription manager. Reacts to the outer signal emitting new inner signals. let manager_system = self .map( - // This closure contains the core logic for switching subscriptions. move |In(inner_signal): In, world: &mut World, mut active_forwarder: Local>, mut active_signal_id: Local>| { - // A. Get the canonical ID of the newly emitted inner signal. `register` is - // idempotent; it just increments the ref-count if the system exists. + // a. Get the canonical ID. `register` is idempotent for existing signals. let new_signal_id = inner_signal.clone().register(world); - // B. MEMOIZATION: Check if the signal has actually changed from the last frame. + // b. Memoization: if unchanged, balance the ref-count and return early. if Some(*new_signal_id) == *active_signal_id { - // The signal is the same. Do nothing. IMPORTANT: We must cleanup the handle from - // our `.register()` call above to balance the reference count, otherwise it will - // leak. new_signal_id.cleanup(world); return; } - // C. TEARDOWN: The signal is new, so clean up the old forwarder and its ID. + // c. Clean up the old forwarder. if let Some(old_handle) = active_forwarder.take() { old_handle.cleanup(world); } - // The old signal ID handle is implicitly dropped when overwritten. - // ================== The Core Setup Logic for the NEW Signal ================== - // D. Get the initial value of the new inner signal, synchronously. This is done - // by creating a temporary one-shot signal. - let temp_handle = inner_signal.clone().first().register(world); - let initial_value = poll_signal(world, *temp_handle) + // d. Poll the initial value synchronously. + let initial_value = poll_signal(world, *new_signal_id) .and_then(downcast_any_clone::<::Item>); - // The temporary handle must be cleaned up immediately. - temp_handle.cleanup(world); - - // E. Write this initial value directly into the state component. + // e. Write initial value to state. if let Some(value) = initial_value && let Some(mut state) = world.get_mut::::Item>>(*reader_system) @@ -1801,42 +1787,29 @@ pub trait SignalExt: Signal { state.value = Some(value); } - // F. Set up a persistent forwarder for all _subsequent_ updates from the new - // signal. + // f. Set up forwarder for subsequent values. let forwarder_handle = inner_signal - .map( - // This first map writes the value to the state component. - move |In(value), - mut query: Query< - &mut FlattenState<::Item>, - Allow, - >| { - if let Ok(mut state) = query.get_mut(*reader_system) { - state.value = Some(value); - } - }, - ) - .map( - // This second map triggers the reader to run immediately after the state is - // written. - move |_, world: &mut World| { - process_signals(world, [reader_system], Box::new(())); - }, - ) + .map(move |In(value), world: &mut World| { + if let Some(mut state) = + world.get_mut::::Item>>(*reader_system) + { + state.value = Some(value); + } + process_signals(world, [reader_system], Box::new(())); + }) .register(world); - // G. Store the new forwarder's handle and the new signal's ID for the next frame. + // g. Store handles for next frame's memoization check. *active_forwarder = Some(forwarder_handle); *active_signal_id = Some(*new_signal_id); - // H. Manually trigger the reader to run RIGHT NOW to consume the initial value. + // h. Trigger reader to consume the initial value. process_signals(world, [reader_system], Box::new(())); }, ) .register(world); - // 4. Set up entity hierarchy for automatic cleanup. When the `reader_system` (the final output) is - // cleaned up, it will also despawn its children. + // 4. Entity hierarchy for automatic cleanup. world .entity_mut(*reader_system) .insert(SignalHandles::from([manager_system])); @@ -2887,7 +2860,7 @@ impl SignalExt for T where T: Signal {} macro_rules! eq { // Entry point ($s1:expr, $s2:expr $(, $rest:expr)* $(,)?) => { - $crate::__signal_combine_and_map!($s1, $s2 $(, $rest)*; |val| { + $crate::__signal_with_and_map!($s1, $s2 $(, $rest)*; |val| { $crate::eq!(@check val, $s1, $s2 $(, $rest)*) }) }; @@ -2905,32 +2878,32 @@ macro_rules! eq { #[doc(hidden)] #[macro_export] -macro_rules! __signal_combine_and_map { +macro_rules! __signal_with_and_map { // Single argument case ($s1:expr $(,)?; |$val:ident| $body:block) => { $s1.map(|In($val)| $body) }; // Entry point ($s1:expr, $s2:expr $(, $rest:expr)* $(,)?; |$val:ident| $body:block) => { - $crate::__signal_combine_and_map!(@combine $s1, $s2 $(, $rest)*) - .map(|In($val @ $crate::__signal_combine_and_map!(@pattern $s1, $s2 $(, $rest)*))| $body) + $crate::__signal_with_and_map!(@combine $s1, $s2 $(, $rest)*) + .map(|In($val @ $crate::__signal_with_and_map!(@pattern $s1, $s2 $(, $rest)*))| $body) }; // Combine chain (@combine $first:expr, $second:expr) => { - $first.combine($second) + $first.with($second) }; (@combine $first:expr, $second:expr, $($rest:expr),+) => { - $crate::__signal_combine_and_map!(@combine $first.combine($second), $($rest),+) + $crate::__signal_with_and_map!(@combine $first.with($second), $($rest),+) }; // Pattern generator (nested tuple of `_` matching the combine nesting) (@pattern $s1:expr, $s2:expr $(, $rest:expr)*) => { - $crate::__signal_combine_and_map!(@pattern_helper (_, _) $(, $rest)*) + $crate::__signal_with_and_map!(@pattern_helper (_, _) $(, $rest)*) }; (@pattern_helper $acc:tt $(,)?) => { $acc }; (@pattern_helper $acc:tt, $head:expr $(, $tail:expr)*) => { - $crate::__signal_combine_and_map!(@pattern_helper ($acc, _) $(, $tail)*) + $crate::__signal_with_and_map!(@pattern_helper ($acc, _) $(, $tail)*) }; } @@ -3070,7 +3043,7 @@ macro_rules! __signal_reduce_binop { }; // Entry point ($op:tt; $s1:expr, $s2:expr $(, $rest:expr)* $(,)?) => { - $crate::__signal_combine_and_map!($s1, $s2 $(, $rest)*; |val| { + $crate::__signal_with_and_map!($s1, $s2 $(, $rest)*; |val| { $crate::__signal_reduce_binop!(@apply $op, val, $s1, $s2 $(, $rest)*) }) }; @@ -3166,7 +3139,7 @@ macro_rules! __signal_reduce_cmp { }; // Entry point ($cmp:tt; $s1:expr, $s2:expr $(, $rest:expr)* $(,)?) => { - $crate::__signal_combine_and_map!($s1, $s2 $(, $rest)*; |val| { + $crate::__signal_with_and_map!($s1, $s2 $(, $rest)*; |val| { $crate::__signal_reduce_cmp!(@select $cmp, val, $s1, $s2 $(, $rest)*) }) }; @@ -3185,6 +3158,68 @@ macro_rules! __signal_reduce_cmp { }; } +/// Zips multiple [`Signal`]s into a single [`Signal`] that outputs a flat tuple of all their +/// outputs. Unlike chaining [`.with()`](SignalExt::with) calls which produces nested tuples +/// like `(((A, B), C), D)`, this macro produces flat tuples like `(A, B, C, D)`. +/// +/// The resulting [`Signal`] will only output a value when all input [`Signal`]s have outputted a +/// value. +/// +/// Accepts 1 or more [`Signal`]s. +/// +/// # Example +/// +/// ``` +/// use bevy_ecs::prelude::*; +/// use jonmo::{prelude::*, signal}; +/// +/// let s1 = SignalBuilder::from_system(|_: In<()>| 1); +/// let s2 = SignalBuilder::from_system(|_: In<()>| "hello"); +/// let s3 = SignalBuilder::from_system(|_: In<()>| 3.14); +/// let s4 = SignalBuilder::from_system(|_: In<()>| true); +/// +/// // Outputs a flat tuple (i32, &str, f64, bool) +/// let zipped = signal::zip!(s1, s2, s3, s4); +/// +/// // Compare to chained .with() which would give (((i32, &str), f64), bool) +/// ``` +#[macro_export] +macro_rules! zip { + // Single signal case - just return it as-is wrapped in a 1-tuple for consistency + ($s1:expr $(,)?) => { + $s1.map(|In(__v)| (__v,)) + }; + // Two signals - use with directly (already flat) + ($s1:expr, $s2:expr $(,)?) => { + $s1.with($s2) + }; + // Three or more signals - build combine chain then flatten + ($s1:expr, $s2:expr $(, $rest:expr)+ $(,)?) => { + $crate::__signal_with_and_map!($s1, $s2 $(, $rest)+; |__v| { + $crate::__signal_zip_flatten!(__v; $s1, $s2 $(, $rest)+;) + }) + }; +} + +#[doc(hidden)] +#[macro_export] +macro_rules! __signal_zip_flatten { + // Entry point - start building from innermost (.0.0...) outward + ($val:expr; $($signals:expr),+;) => { + $crate::__signal_zip_flatten!(@collect $val; $($signals),+; ()) + }; + + // Base case: 2 signals left - prepend val.0, val.1 to accumulator + (@collect $val:expr; $s1:expr, $s2:expr; ($($acc:expr),*)) => { + ($val.0, $val.1 $(, $acc)*) + }; + + // Recursive case: 3+ signals - add val.1 to accumulator, recurse with val.0 + (@collect $val:expr; $head:expr, $($tail:expr),+; ($($acc:expr),*)) => { + $crate::__signal_zip_flatten!(@collect $val.0; $($tail),+; ($val.1 $(, $acc)*)) + }; +} + pub use all; pub use any; pub use distinct; @@ -3193,6 +3228,7 @@ pub use max; pub use min; pub use product; pub use sum; +pub use zip; #[cfg(test)] mod tests { @@ -3673,11 +3709,11 @@ mod tests { } #[test] - fn test_combine() { + fn test_with() { let mut app = create_test_app(); app.init_resource::>(); let signal = SignalBuilder::from_system(move |_: In<()>| 10) - .combine(SignalBuilder::from_system(move |_: In<()>| "hello")) + .with(SignalBuilder::from_system(move |_: In<()>| "hello")) .map(capture_output) .register(app.world_mut()); app.update(); @@ -3685,6 +3721,63 @@ mod tests { signal.cleanup(app.world_mut()); } + #[test] + fn test_zip_macro() { + use crate::signal::zip; + + let mut app = create_test_app(); + + // Test 1 signal - returns 1-tuple + app.init_resource::>(); + let s1 = SignalBuilder::from_system(|_: In<()>| 1); + let signal = zip!(s1).map(capture_output).register(app.world_mut()); + app.update(); + assert_eq!(get_output::<(i32,)>(app.world()), Some((1,))); + signal.cleanup(app.world_mut()); + + // Test 2 signals - same as .with() + app.init_resource::>(); + let s1 = SignalBuilder::from_system(|_: In<()>| 1); + let s2 = SignalBuilder::from_system(|_: In<()>| "two"); + let signal = zip!(s1, s2).map(capture_output).register(app.world_mut()); + app.update(); + assert_eq!(get_output::<(i32, &'static str)>(app.world()), Some((1, "two"))); + signal.cleanup(app.world_mut()); + + // Test 3 signals - flat tuple (not nested) + app.init_resource::>(); + let s1 = SignalBuilder::from_system(|_: In<()>| 1); + let s2 = SignalBuilder::from_system(|_: In<()>| "two"); + let s3 = SignalBuilder::from_system(|_: In<()>| 3.0); + let signal = zip!(s1, s2, s3).map(capture_output).register(app.world_mut()); + app.update(); + assert_eq!(get_output::<(i32, &'static str, f64)>(app.world()), Some((1, "two", 3.0))); + signal.cleanup(app.world_mut()); + + // Test 4 signals - verifies flattening works for more signals + app.init_resource::>(); + let s1 = SignalBuilder::from_system(|_: In<()>| 1); + let s2 = SignalBuilder::from_system(|_: In<()>| "two"); + let s3 = SignalBuilder::from_system(|_: In<()>| 3.0); + let s4 = SignalBuilder::from_system(|_: In<()>| true); + let signal = zip!(s1, s2, s3, s4).map(capture_output).register(app.world_mut()); + app.update(); + assert_eq!(get_output::<(i32, &'static str, f64, bool)>(app.world()), Some((1, "two", 3.0, true))); + signal.cleanup(app.world_mut()); + + // Test 5 signals - ensures unlimited recursion works + app.init_resource::>(); + let s1 = SignalBuilder::from_system(|_: In<()>| 1); + let s2 = SignalBuilder::from_system(|_: In<()>| 2); + let s3 = SignalBuilder::from_system(|_: In<()>| 3); + let s4 = SignalBuilder::from_system(|_: In<()>| 4); + let s5 = SignalBuilder::from_system(|_: In<()>| 5); + let signal = zip!(s1, s2, s3, s4, s5).map(capture_output).register(app.world_mut()); + app.update(); + assert_eq!(get_output::<(i32, i32, i32, i32, i32)>(app.world()), Some((1, 2, 3, 4, 5))); + signal.cleanup(app.world_mut()); + } + #[test] fn test_flatten() { let mut app = create_test_app(); diff --git a/src/signal_map.rs b/src/signal_map.rs index 587f8a3..784606e 100644 --- a/src/signal_map.rs +++ b/src/signal_map.rs @@ -22,7 +22,7 @@ use core::{ fmt, marker::PhantomData, ops::Deref, - sync::atomic::{AtomicBool, AtomicUsize, Ordering as AtomicOrdering}, + sync::atomic::{AtomicUsize, Ordering as AtomicOrdering}, }; use dyn_clone::{DynClone, clone_trait_object}; @@ -611,6 +611,9 @@ pub trait SignalMapExt: SignalMap { S::Item: Clone + SSs, F: IntoSystem, S, M> + SSs, { + #[derive(Component)] + struct QueuedMapDiffs(Vec>); + let signal = LazySignal::new(move |world: &mut World| { let factory_system_id = world.register_system(system); let output_signal_entity = LazyEntity::new(); @@ -1075,9 +1078,6 @@ pub struct MutableBTreeMapData { broadcaster: LazySignal, } -#[derive(Component)] -struct QueuedMapDiffs(Vec>); - /// Wrapper around a [`BTreeMap`] that emits mutations as [`MapDiff`]s, enabling diff-less /// constant-time reactive updates for downstream [`SignalMap`]s. pub struct MutableBTreeMap { @@ -1214,10 +1214,9 @@ impl MutableBTreeMap { let was_initially_empty = self_.read(&*world).is_empty(); let replay_entity = LazyEntity::new(); - let is_first_replay = Arc::new(AtomicBool::new(true)); - let replay_system = clone!((self_, replay_entity, is_first_replay) move |In(upstream_diffs): In>>, replay_onces: Query<&ReplayOnce, Allow>, mutable_btree_map_datas: Query<&MutableBTreeMapData>| { + let replay_system = clone!((self_, replay_entity) move |In(upstream_diffs): In>>, replay_onces: Query<&ReplayOnce, Allow>, mutable_btree_map_datas: Query<&MutableBTreeMapData>, mut has_replayed: Local| { if replay_onces.contains(*replay_entity) { - let first_replay = is_first_replay.swap(false, AtomicOrdering::SeqCst); + let first_replay = !core::mem::replace(&mut *has_replayed, true); if first_replay && was_initially_empty { // Initial replay with empty map - just forward upstream diffs if upstream_diffs.is_empty() { diff --git a/src/signal_vec.rs b/src/signal_vec.rs index d494846..f5c85f3 100644 --- a/src/signal_vec.rs +++ b/src/signal_vec.rs @@ -14,7 +14,7 @@ use bevy_ecs::{change_detection::Mut, entity_disabling::Internal, prelude::*, sy #[cfg(feature = "tracing")] use bevy_log::debug; use bevy_platform::{ - collections::HashMap, + collections::{HashMap, HashSet}, prelude::*, sync::{Arc, LazyLock, Mutex}, }; @@ -23,7 +23,7 @@ use core::{ fmt, marker::PhantomData, ops::Deref, - sync::atomic::{AtomicBool, AtomicUsize, Ordering as AtomicOrdering}, + sync::atomic::{AtomicUsize, Ordering as AtomicOrdering}, }; use dyn_clone::{DynClone, clone_trait_object}; @@ -268,9 +268,6 @@ where } } -#[derive(Component, Deref, DerefMut, Clone, Copy)] -struct ItemIndex(usize); - /// Signal graph node which applies a [`System`] to each [`Item`](SignalVec::Item) of its upstream, /// forwarding the output of each resulting [`Signal`], see /// [`.map_signal`](SignalVecExt::map_signal). @@ -513,109 +510,6 @@ where } } -struct FilterSignalItem { - signal: SignalHandle, - value: T, - filtered: bool, -} - -#[derive(Component)] -struct FilterSignalData { - items: Vec>, - diffs: Vec>, -} - -fn with_filter_signal_data( - world: &mut World, - entity: Entity, - f: impl FnOnce(Mut>) -> O, -) -> O { - let data = world.get_mut::>(entity).unwrap(); - f(data) -} - -fn find_filter_signal_index(filter_signal_items: &[FilterSignalItem], i: usize) -> usize { - find_index(filter_signal_items.iter().map(|signal| &signal.filtered), i) -} - -#[derive(Component, Deref, DerefMut)] -struct FilterSignalIndex(usize); - -fn spawn_filter_signal( - world: &mut World, - index: usize, - signal: impl Signal + 'static, - parent: Entity, -) -> (SignalHandle, bool) { - let entity = LazyEntity::new(); - let processor_system = clone!((entity) move |In(filter): In, world: &mut World| { - let self_entity = *entity; - - // The processor might run after its parent has been cleaned up. - let Ok(signal_index_comp) = world.query_filtered::<&FilterSignalIndex, Allow>().get(world, self_entity) else { - return; - }; - let item_index = signal_index_comp.0; - let Ok(mut filter_signal_data) = world.query_filtered::<&mut FilterSignalData, Allow>().get_mut(world, parent) else { - return; - }; - - // This item_index might be stale if other items were removed in the same frame. - // Ensure we don't panic. - if item_index >= filter_signal_data.items.len() { - return; - } - let old_filtered_state = filter_signal_data.items[item_index].filtered; - if old_filtered_state == filter { - // No change, do nothing. - return; - } - let diff_to_queue = if filter { - // Item is now INCLUDED. Find its insertion point based on the state _before_ it's - // marked as included. - let new_filtered_index = find_filter_signal_index(&filter_signal_data.items, item_index); - filter_signal_data.items[item_index].filtered = true; - VecDiff::InsertAt { - index: new_filtered_index, - value: filter_signal_data.items[item_index].value.clone(), - } - } else { - // Item is now EXCLUDED. Find its removal point based on the state _before_ it's - // marked as excluded. - let old_filtered_index = find_filter_signal_index(&filter_signal_data.items, item_index); - filter_signal_data.items[item_index].filtered = false; - VecDiff::RemoveAt { index: old_filtered_index } - }; - filter_signal_data.diffs.push(diff_to_queue); - - // Poke the main output signal to process its queue immediately. - process_signals(world, [parent.into()], Box::new(Vec::>::new())); - }); - - // Use .map() to attach the processor. The dedupe is still important. - let mapped_signal = signal.dedupe().map(processor_system); - let handle = mapped_signal.register(world); - entity.set(**handle); - world.entity_mut(**handle).insert(FilterSignalIndex(index)); - - // To get the initial value, we must poll the original signal before the - // map/dedupe. The upstream of the `map` node is the `dedupe` node. - let dedupe_handle = world.get::(**handle).unwrap().iter().next().cloned().unwrap(); - - // The upstream of the `dedupe` node is the original signal. - let original_signal_handle = world - .get::(*dedupe_handle) - .unwrap() - .iter() - .next() - .cloned() - .unwrap(); - let initial_value = poll_signal(world, original_signal_handle) - .and_then(downcast_any_clone::) - .expect("filter_signal's inner signal must emit an initial value upon registration"); - (handle, initial_value) -} - /// Signal graph node which selectively forwards upstream [`Item`](SignalVec::Item)s depending on a /// [`Signal`], see [`.filter_signal`](SignalVecExt::filter_signal). pub struct FilterSignal { @@ -643,13 +537,6 @@ where } } -#[derive(Component, Default)] -struct EnumerateState { - key_to_index: HashMap, - ordered_keys: Vec, - next_key: usize, -} - /// Signal graph node which prepends an index signal to each upstream [`Item`](SignalVec::Item), see /// [`.enumerate`](SignalVecExt::enumerate). pub struct Enumerate @@ -902,30 +789,6 @@ where } } -#[derive(Component)] -struct IntersperseState { - /// The original, non-interspersed values. We need this to know the length. - local_values: Vec, - /// Stable keys for the separators. The key at index `i` corresponds to the separator that comes - /// _after_ `local_values[i]`. - separator_keys: Vec, - /// Maps a separator's stable key to its current logical index (0, 1, 2...). - key_to_index: HashMap, - /// A counter to generate new unique, stable keys. - next_key: usize, -} - -impl Default for IntersperseState { - fn default() -> Self { - Self { - local_values: Vec::new(), - separator_keys: Vec::new(), - key_to_index: HashMap::new(), - next_key: 0, - } - } -} - /// Signal graph node that places a [`System`]-dependent separator between each upstream /// [`Item`](SignalVec::Item), see [`.intersperse_with`](SignalVecExt::intersperse_with). pub struct IntersperseWith { @@ -1007,163 +870,6 @@ where } } -/* -struct FlattenItem { - processor_handle: SignalHandle, - values: Vec, - _marker: PhantomData O>, -} - -#[derive(Component)] -struct FlattenData { - items: Vec>, - diffs: Vec>, -} - -fn with_flatten_data( - world: &mut World, - parent: Entity, - f: impl FnOnce(Mut>) -> R, -) -> R { - f(world.get_mut::>(parent).unwrap()) -} - -#[derive(Component, Deref, DerefMut)] -struct FlattenInnerIndex(usize); - -#[allow(clippy::type_complexity)] -fn create_flatten_processor( - entity: LazyEntity, - parent: Entity, -) -> impl Fn(In>>, Query<&FlattenInnerIndex>, Query<&mut FlattenData>) { - move |In(diffs): In>>, - query_self_index: Query<&FlattenInnerIndex>, - mut query_parent_data: Query<&mut FlattenData>| { - if let Ok(mut parent_data) = query_parent_data.get_mut(parent) { - if let Ok(&FlattenInnerIndex(self_index)) = query_self_index.get(entity.get()) { - if self_index >= parent_data.items.len() { - return; - } - - let offset: usize = parent_data.items[..self_index] - .iter() - .map(|item| item.values.len()) - .sum(); - - for diff in diffs { - let apply_and_queue = |pd: &mut Mut>, d: VecDiff| { - let translated_diff = match d { - VecDiff::Replace { values } => { - let old_len = pd.items[self_index].values.len(); - for _ in 0..old_len { - pd.diffs.push(VecDiff::RemoveAt { index: offset }); - } - for (i, v) in values.iter().enumerate() { - pd.diffs.push(VecDiff::InsertAt { - index: offset + i, - value: v.clone(), - }); - } - pd.items[self_index].values = values; - return; - } - VecDiff::InsertAt { index, value } => { - pd.items[self_index].values.insert(index, value.clone()); - VecDiff::InsertAt { - index: index + offset, - value, - } - } - VecDiff::UpdateAt { index, value } => { - pd.items[self_index].values[index] = value.clone(); - VecDiff::UpdateAt { - index: index + offset, - value, - } - } - VecDiff::RemoveAt { index } => { - pd.items[self_index].values.remove(index); - VecDiff::RemoveAt { - index: index + offset, - } - } - VecDiff::Move { - old_index, - new_index, - } => { - let val = pd.items[self_index].values.remove(old_index); - pd.items[self_index].values.insert(new_index, val); - VecDiff::Move { - old_index: old_index + offset, - new_index: new_index + offset, - } - } - VecDiff::Push { value } => { - let old_len = pd.items[self_index].values.len(); - pd.items[self_index].values.push(value.clone()); - VecDiff::InsertAt { - index: offset + old_len, - value, - } - } - VecDiff::Pop => { - pd.items[self_index].values.pop(); - VecDiff::RemoveAt { - index: offset + pd.items[self_index].values.len(), - } - } - VecDiff::Clear => { - let old_len = pd.items[self_index].values.len(); - pd.items[self_index].values.clear(); - for _ in 0..old_len { - pd.diffs.push(VecDiff::RemoveAt { index: offset }); - } - return; - } - }; - pd.diffs.push(translated_diff); - }; - apply_and_queue(&mut parent_data, diff); - } - } - } - } -} - -fn spawn_flatten_item( - world: &mut World, - parent: Entity, - index: usize, - inner_signal: impl SignalVec + 'static + Clone, -) -> (FlattenItem, Vec) { - // These are placeholder function calls based on the names - let temp_get_state_signal = inner_signal.clone(); // .to_signal().first(); - let handle = temp_get_state_signal.register(world); - let initial_values = poll_signal(world, &handle) - .and_then(downcast_any_clone::>) - .unwrap_or_default(); - handle.cleanup(world); - - let entity = LazyEntity::new(); - let processor_system = create_flatten_processor(entity.clone(), parent); - let processor_handle = inner_signal.for_each(processor_system).register(world); - - entity.set( - world - .entity_mut(*processor_handle) - .insert(FlattenInnerIndex(index)) - .id(), - ); - - let item = FlattenItem { - processor_handle, - values: initial_values.clone(), - _marker: PhantomData, - }; - - (item, initial_values) -} - /// A node that flattens a `SignalVec` of `SignalVec`s. #[derive(Clone)] pub struct Flatten { @@ -1182,7 +888,7 @@ where fn register_boxed_signal_vec(self: Box, world: &mut World) -> SignalHandle { self.signal.register(world).into() } -} */ +} cfg_if::cfg_if! { if #[cfg(feature = "tracing")] { @@ -1531,6 +1237,9 @@ pub trait SignalVecExt: SignalVec { S::Item: Clone + Send + Sync, F: IntoSystem, S, M> + SSs, { + #[derive(Component, Deref, DerefMut, Clone, Copy)] + struct ItemIndex(usize); + fn spawn_processor + Clone + 'static>( world: &mut World, output_signal: SignalSystem, @@ -1893,6 +1602,109 @@ pub trait SignalVecExt: SignalVec { S: Signal + 'static, F: IntoSystem, S, M> + SSs, { + struct FilterSignalItem { + signal: SignalHandle, + value: T, + filtered: bool, + } + + #[derive(Component)] + struct FilterSignalData { + items: Vec>, + diffs: Vec>, + } + + fn with_filter_signal_data( + world: &mut World, + entity: Entity, + f: impl FnOnce(Mut>) -> O, + ) -> O { + let data = world.get_mut::>(entity).unwrap(); + f(data) + } + + fn find_filter_signal_index(filter_signal_items: &[FilterSignalItem], i: usize) -> usize { + find_index(filter_signal_items.iter().map(|signal| &signal.filtered), i) + } + + #[derive(Component, Deref, DerefMut)] + struct FilterSignalIndex(usize); + + fn spawn_filter_signal( + world: &mut World, + index: usize, + signal: impl Signal + 'static, + parent: Entity, + ) -> (SignalHandle, bool) { + let entity = LazyEntity::new(); + let processor_system = clone!((entity) move |In(filter): In, world: &mut World| { + let self_entity = *entity; + + // The processor might run after its parent has been cleaned up. + let Ok(signal_index_comp) = world.query_filtered::<&FilterSignalIndex, Allow>().get(world, self_entity) else { + return; + }; + let item_index = signal_index_comp.0; + let Ok(mut filter_signal_data) = world.query_filtered::<&mut FilterSignalData, Allow>().get_mut(world, parent) else { + return; + }; + + // This item_index might be stale if other items were removed in the same frame. + // Ensure we don't panic. + if item_index >= filter_signal_data.items.len() { + return; + } + let old_filtered_state = filter_signal_data.items[item_index].filtered; + if old_filtered_state == filter { + // No change, do nothing. + return; + } + let diff_to_queue = if filter { + // Item is now INCLUDED. Find its insertion point based on the state _before_ it's + // marked as included. + let new_filtered_index = find_filter_signal_index(&filter_signal_data.items, item_index); + filter_signal_data.items[item_index].filtered = true; + VecDiff::InsertAt { + index: new_filtered_index, + value: filter_signal_data.items[item_index].value.clone(), + } + } else { + // Item is now EXCLUDED. Find its removal point based on the state _before_ it's + // marked as excluded. + let old_filtered_index = find_filter_signal_index(&filter_signal_data.items, item_index); + filter_signal_data.items[item_index].filtered = false; + VecDiff::RemoveAt { index: old_filtered_index } + }; + filter_signal_data.diffs.push(diff_to_queue); + + // Poke the main output signal to process its queue immediately. + process_signals(world, [parent.into()], Box::new(Vec::>::new())); + }); + + // Use .map() to attach the processor. The dedupe is still important. + let mapped_signal = signal.dedupe().map(processor_system); + let handle = mapped_signal.register(world); + entity.set(**handle); + world.entity_mut(**handle).insert(FilterSignalIndex(index)); + + // To get the initial value, we must poll the original signal before the + // map/dedupe. The upstream of the `map` node is the `dedupe` node. + let dedupe_handle = world.get::(**handle).unwrap().iter().next().cloned().unwrap(); + + // The upstream of the `dedupe` node is the original signal. + let original_signal_handle = world + .get::(*dedupe_handle) + .unwrap() + .iter() + .next() + .cloned() + .unwrap(); + let initial_value = poll_signal(world, original_signal_handle) + .and_then(downcast_any_clone::) + .expect("filter_signal's inner signal must emit an initial value upon registration"); + (handle, initial_value) + } + let signal = LazySignal::new(move |world: &mut World| { let parent_entity = LazyEntity::new(); let system_id = world.register_system(system); @@ -2239,6 +2051,13 @@ pub trait SignalVecExt: SignalVec { Self: Sized, Self::Item: Clone + 'static, { + #[derive(Component, Default)] + struct EnumerateState { + key_to_index: HashMap, + ordered_keys: Vec, + next_key: usize, + } + let signal = LazySignal::new(move |world: &mut World| { let processor_entity_handle = LazyEntity::new(); let processor_logic = clone!( @@ -2851,6 +2670,30 @@ pub trait SignalVecExt: SignalVec { Self::Item: Clone + SSs, F: IntoSystem>>, Self::Item, M> + SSs, { + #[derive(Component)] + struct IntersperseState { + /// The original, non-interspersed values. We need this to know the length. + local_values: Vec, + /// Stable keys for the separators. The key at index `i` corresponds to the separator that comes + /// _after_ `local_values[i]`. + separator_keys: Vec, + /// Maps a separator's stable key to its current logical index (0, 1, 2...). + key_to_index: HashMap, + /// A counter to generate new unique, stable keys. + next_key: usize, + } + + impl Default for IntersperseState { + fn default() -> Self { + Self { + local_values: Vec::new(), + separator_keys: Vec::new(), + key_to_index: HashMap::new(), + next_key: 0, + } + } + } + let signal = LazySignal::new(move |world: &mut World| { // 1. Register the user's factory system once. let factory_system_id = world.register_system(separator_system); @@ -3530,162 +3373,485 @@ pub trait SignalVecExt: SignalVec { } } - /* fn flatten(self) -> Flatten + /// Flattens a [`SignalVec`] of [`SignalVec`]s into a single [`SignalVec`]. + /// + /// When the outer [`SignalVec`] adds/removes/moves inner [`SignalVec`]s, or when any inner + /// [`SignalVec`] itself changes, the flattened output is updated accordingly. + /// + /// # Example + /// + /// ``` + /// use bevy_ecs::prelude::*; + /// use jonmo::prelude::*; + /// + /// let mut world = World::new(); + /// + /// let inner1 = MutableVecBuilder::from([1, 2]).spawn(&mut world); + /// let inner2 = MutableVecBuilder::from([3, 4, 5]).spawn(&mut world); + /// + /// let outer = MutableVecBuilder::from([ + /// inner1.signal_vec(), + /// inner2.signal_vec(), + /// ]).spawn(&mut world); + /// + /// let flattened = outer.signal_vec().flatten(); + /// // flattened outputs `[1, 2, 3, 4, 5]` + /// ``` + fn flatten(self) -> Flatten where - Self: Sized, // This should be a trait like SignalVec + Self: Sized, Self::Item: SignalVec + 'static + Clone, ::Item: Clone + SSs, { + type InnerItem = <::Item as SignalVec>::Item; + + #[derive(Component)] + struct FlattenState { + /// Each item tracks: (processor handle, current length of that inner signal vec) + items: Vec<(SignalHandle, usize)>, + } + + /// Component to track which index in the FlattenState.items this processor corresponds to + #[derive(Component)] + struct FlattenItemIndex(usize); + let signal = LazySignal::new(move |world: &mut World| { - let parent_entity = LazyEntity::new(); + let output_signal_entity = LazyEntity::new(); + + // The output signal just drains and emits any queued diffs + let output_signal = *SignalBuilder::from_system::>>, _, _, _>( + clone!((output_signal_entity) move |_: In<()>, world: &mut World| { + if let Some(mut diffs) = world.get_mut::>>(output_signal_entity.get()) { + if diffs.0.is_empty() { + None + } else { + Some(diffs.0.drain(..).collect()) + } + } else { + None + } + }), + ) + .register(world); + output_signal_entity.set(*output_signal); + + /// Finds all VecReplayTrigger ancestors of a signal + fn find_replay_triggers(world: &World, signal: SignalSystem) -> Vec { + let mut triggers = Vec::new(); + let mut stack = vec![signal]; + let mut visited = HashSet::new(); + while let Some(current) = stack.pop() { + if !visited.insert(current) { + continue; + } + if world.get::(*current).is_some() { + triggers.push(current); + } + if let Some(upstream) = world.get::(*current) { + stack.extend(upstream.0.iter().copied()); + } + } + triggers + } + + /// Triggers replay on the given signals + fn trigger_replays(world: &mut World, triggers: Vec) { + for signal in triggers { + trigger_replay::(world, *signal); + } + } + + /// Creates a processor for an inner SignalVec that translates its diffs to the flattened output. + /// Returns (processor handle, replay triggers to call later). + /// NOTE: The caller should add the entry to FlattenState.items and THEN call trigger_replays. + fn create_inner_processor( + world: &mut World, + output_signal: SignalSystem, + index: usize, + inner_signal_vec: impl SignalVec + 'static + Clone, + ) -> (SignalHandle, Vec) { + let processor_entity = LazyEntity::new(); + + // Processor that handles diffs from the inner SignalVec + let processor_logic = clone!((processor_entity) move |In(diffs): In>>, world: &mut World| { + let self_entity = processor_entity.get(); + + // Get our current index + let Some(item_index_comp) = world.get::(self_entity) else { + return; + }; + let item_index = item_index_comp.0; - let SignalHandle(output_signal) = self - .for_each::::Item>>, _, _, _>( - clone!((parent_entity) move |In(diffs): In>>, world: &mut World| { - let parent = parent_entity.get(); - let mut new_diffs = vec![]; + // Get the flatten state + let Some(mut state) = world.get_mut::(*output_signal) else { + return; + }; + + // Bounds check (should not happen if used correctly, but defensive) + if item_index >= state.items.len() { + return; + } + + // Calculate offset (sum of lengths of all inner vecs before this one) + let offset: usize = state.items[..item_index] + .iter() + .map(|(_, len)| *len) + .sum(); + + let mut out_diffs = Vec::new(); for diff in diffs { match diff { - VecDiff::Push { value: inner_signal } => { - let index = with_flatten_data::<::Item, _>(world, parent, |data| data.items.len()); - let (item, initial_values) = spawn_flatten_item(world, parent, index, inner_signal); - let offset: usize = with_flatten_data::<::Item, _>(world, parent, |data| data.items.iter().map(|i| i.values.len()).sum()); - with_flatten_data(world, parent, |mut data| data.items.push(item)); - for (i, value) in initial_values.into_iter().enumerate() { - new_diffs.push(VecDiff::InsertAt { index: offset + i, value }); - } - } - VecDiff::InsertAt { index, value: inner_signal } => { - let (item, initial_values) = spawn_flatten_item(world, parent, index, inner_signal); - let offset: usize = with_flatten_data::<::Item, _>(world, parent, |data| data.items[..index].iter().map(|i| i.values.len()).sum()); - with_flatten_data(world, parent, |mut data| data.items.insert(index, item)); - let signals_to_update = with_flatten_data::<::Item, _>(world, parent, |data| data.items.iter().map(|i| i.processor_handle.clone()).collect::>()); - for (i, handle) in signals_to_update.into_iter().enumerate() { - if let Some(mut idx) = world.get_mut::(*handle) { - idx.0 = i; - } + VecDiff::Replace { values } => { + // Remove all old values + let old_len = state.items[item_index].1; + for _ in 0..old_len { + out_diffs.push(VecDiff::RemoveAt { index: offset }); } - for (i, value) in initial_values.into_iter().enumerate() { - new_diffs.push(VecDiff::InsertAt { index: offset + i, value }); + // Insert all new values + for (i, v) in values.iter().enumerate() { + out_diffs.push(VecDiff::InsertAt { + index: offset + i, + value: v.clone(), + }); } + state.items[item_index].1 = values.len(); } - VecDiff::RemoveAt { index } => { - let (item, offset) = with_flatten_data(world, parent, |mut data: Mut::Item>>| { - let offset = data.items[..index].iter().map(|i| i.values.len()).sum(); - (data.items.remove(index), offset) + VecDiff::InsertAt { index, value } => { + state.items[item_index].1 += 1; + out_diffs.push(VecDiff::InsertAt { + index: offset + index, + value, }); - item.processor_handle.cleanup(world); - for _ in 0..item.values.len() { - new_diffs.push(VecDiff::RemoveAt { index: offset }); - } } - VecDiff::Pop => { - let (item, offset) = with_flatten_data(world, parent, |mut data: Mut::Item>>| { - let offset = data.items.iter().map(|i| i.values.len()).sum::().saturating_sub(data.items.last().map_or(0, |i| i.values.len())); - (data.items.pop().unwrap(), offset) + VecDiff::UpdateAt { index, value } => { + // Length doesn't change for UpdateAt + out_diffs.push(VecDiff::UpdateAt { + index: offset + index, + value, }); - item.processor_handle.cleanup(world); - for _ in 0..item.values.len() { - new_diffs.push(VecDiff::RemoveAt { index: offset }); - } - } - VecDiff::Clear => { - let old_items = with_flatten_data(world, parent, |mut data: Mut::Item>>| data.items.drain(..).collect::>()); - for item in old_items { - item.processor_handle.cleanup(world); - } - new_diffs.push(VecDiff::Clear); - } - VecDiff::Replace { values } => { - let old_items = with_flatten_data(world, parent, |mut data: Mut::Item>>| data.items.drain(..).collect::>()); - for item in old_items { - item.processor_handle.cleanup(world); - } - new_diffs.push(VecDiff::Clear); - for (i, inner_signal) in values.into_iter().enumerate() { - let (item, initial_values) = spawn_flatten_item(world, parent, i, inner_signal); - with_flatten_data(world, parent, |mut data| data.items.push(item)); - for value in initial_values { - new_diffs.push(VecDiff::Push { value }); - } - } } - VecDiff::UpdateAt { index, value: inner_signal } => { - let (old_item, offset) = with_flatten_data(world, parent, |mut data: Mut::Item>>| { - let offset = data.items[..index].iter().map(|i| i.values.len()).sum(); - (data.items.remove(index), offset) + VecDiff::RemoveAt { index } => { + state.items[item_index].1 -= 1; + out_diffs.push(VecDiff::RemoveAt { + index: offset + index, }); - old_item.processor_handle.cleanup(world); - for _ in 0..old_item.values.len() { - new_diffs.push(VecDiff::RemoveAt { index: offset }); - } - let (new_item, initial_values) = spawn_flatten_item(world, parent, index, inner_signal); - with_flatten_data(world, parent, |mut data| data.items.insert(index, new_item)); - for (i, value) in initial_values.into_iter().enumerate() { - new_diffs.push(VecDiff::InsertAt { index: offset + i, value }); - } } VecDiff::Move { old_index, new_index } => { - let (old_offset, moved_item, signals_to_update) = with_flatten_data(world, parent, |mut data: Mut::Item>>| { - let old_offset = data.items[..old_index].iter().map(|i| i.values.len()).sum(); - let moved_item = data.items.remove(old_index); - data.items.insert(new_index, moved_item); - let signals_to_update = data.items.iter().map(|i| i.processor_handle.clone()).collect::>(); - (old_offset, data.items[new_index].clone(), signals_to_update) + // Length doesn't change for Move + out_diffs.push(VecDiff::Move { + old_index: offset + old_index, + new_index: offset + new_index, }); - - for (i, handle) in signals_to_update.into_iter().enumerate() { - if let Some(mut fi_index) = world.get_mut::(*handle) { - fi_index.0 = i; - } - } - for _ in 0..moved_item.values.len() { - new_diffs.push(VecDiff::RemoveAt { index: old_offset }); + } + VecDiff::Push { value } => { + let insert_index = offset + state.items[item_index].1; + state.items[item_index].1 += 1; + out_diffs.push(VecDiff::InsertAt { + index: insert_index, + value, + }); + } + VecDiff::Pop => { + if state.items[item_index].1 > 0 { + state.items[item_index].1 -= 1; + out_diffs.push(VecDiff::RemoveAt { + index: offset + state.items[item_index].1, + }); } - let new_offset: usize = with_flatten_data::<::Item, _>(world, parent, |data| data.items[..new_index].iter().map(|i| i.values.len()).sum()); - for (i, value) in moved_item.values.into_iter().enumerate() { - new_diffs.push(VecDiff::InsertAt { index: new_offset + i, value }); + } + VecDiff::Clear => { + let old_len = state.items[item_index].1; + state.items[item_index].1 = 0; + for _ in 0..old_len { + out_diffs.push(VecDiff::RemoveAt { index: offset }); } } } } - let queued_diffs = with_flatten_data(world, parent, |mut data: Mut::Item>>| data.diffs.drain(..).collect::>()); - new_diffs.extend(queued_diffs); - - if new_diffs.is_empty() { - None - } else { - Some(new_diffs) + // Queue the translated diffs + if !out_diffs.is_empty() { + if let Some(mut queue) = world.get_mut::>(*output_signal) { + queue.0.extend(out_diffs); + } + process_signals(world, [output_signal], Box::new(())); } - }), - ) - .register(world); + }); - parent_entity.set(output_signal); + // Register the processor + let processor_handle = inner_signal_vec.for_each(processor_logic).register(world); + processor_entity.set(**processor_handle); + world.entity_mut(**processor_handle).insert(FlattenItemIndex(index)); - let SignalHandle(flusher) = SignalBuilder::from_entity(output_signal) - .map::::Item>>, _, _, _>( - move |In(_), data: Query<&FlattenData<::Item>>| { - if let Ok(data) = data.get(parent_entity.get()) { - if !data.diffs.is_empty() { - return Some(vec![]); - } - } - None - }, - ) - .register(world); + // Find replay triggers but don't trigger them yet + let replay_triggers = find_replay_triggers(world, (**processor_handle).into()); - pipe_signal(world, flusher, output_signal); + (processor_handle, replay_triggers) + } - world - .entity_mut(output_signal) - .insert(FlattenData::<::Item> { - items: vec![], - diffs: vec![], - }); + // Main manager that handles outer SignalVec diffs + let manager_logic = clone!((output_signal_entity) move |In(diffs): In>>, world: &mut World| { + let output_signal: SignalSystem = output_signal_entity.get().into(); + let mut out_diffs: Vec>> = Vec::new(); + + for diff in diffs { + match diff { + VecDiff::Replace { values } => { + // Clean up all old processors + let old_items = { + let mut state = world.get_mut::(*output_signal).unwrap(); + state.items.drain(..).collect::>() + }; + let had_old_items = !old_items.is_empty(); + for (handle, _) in old_items { + handle.cleanup(world); + } + // Only emit Clear if there were old items + // Note: The inner processors will emit InsertAt diffs for their initial values, + // so we need Clear to come BEFORE those. We'll queue it directly now. + if had_old_items { + if let Some(mut queue) = world.get_mut::>>(*output_signal) { + queue.0.push(VecDiff::Clear); + } + } + + // Create processors and collect their replay triggers + let mut all_triggers = Vec::new(); + for (i, inner_signal_vec) in values.into_iter().enumerate() { + let (handle, triggers) = create_inner_processor(world, output_signal, i, inner_signal_vec); + // Add entry with real handle + let mut state = world.get_mut::(*output_signal).unwrap(); + state.items.push((handle, 0)); + all_triggers.extend(triggers); + } + // Now trigger all replays (entries exist) + trigger_replays(world, all_triggers); + } + VecDiff::InsertAt { index, value: inner_signal_vec } => { + // Update indices for all items after the insertion point FIRST + { + let handles_to_update: Vec<_> = { + let state = world.get::(*output_signal).unwrap(); + state.items + .iter() + .enumerate() + .skip(index) + .map(|(i, (h, _))| (**h, i + 1)) + .collect() + }; + for (handle_entity, new_index) in handles_to_update { + if let Some(mut idx) = world.get_mut::(*handle_entity) { + idx.0 = new_index; + } + } + } + + // Create processor + let (handle, triggers) = create_inner_processor(world, output_signal, index, inner_signal_vec); + + // Insert entry with real handle + { + let mut state = world.get_mut::(*output_signal).unwrap(); + state.items.insert(index, (handle, 0)); + } + + // Now trigger replays (entry exists) + trigger_replays(world, triggers); + } + VecDiff::UpdateAt { index, value: inner_signal_vec } => { + // Remove old processor and its values + let (old_handle, old_len, offset) = { + let mut state = world.get_mut::(*output_signal).unwrap(); + let offset: usize = state.items[..index].iter().map(|(_, len)| *len).sum(); + let (old_handle, old_len) = state.items.remove(index); + (old_handle, old_len, offset) + }; + old_handle.cleanup(world); + + // Emit RemoveAt for old values IMMEDIATELY (before creating new processor) + // because trigger_replays will emit InsertAt diffs + if old_len > 0 { + let mut remove_diffs = Vec::new(); + for _ in 0..old_len { + remove_diffs.push(VecDiff::RemoveAt { index: offset }); + } + if let Some(mut queue) = world.get_mut::>>(*output_signal) { + queue.0.extend(remove_diffs); + } + } + + // Create new processor + let (handle, triggers) = create_inner_processor(world, output_signal, index, inner_signal_vec); + + // Insert entry with real handle + { + let mut state = world.get_mut::(*output_signal).unwrap(); + state.items.insert(index, (handle, 0)); + } + + // Now trigger replays (entry exists) + trigger_replays(world, triggers); + } + VecDiff::RemoveAt { index } => { + let (handle, old_len, offset) = { + let mut state = world.get_mut::(*output_signal).unwrap(); + let offset: usize = state.items[..index].iter().map(|(_, len)| *len).sum(); + let (handle, len) = state.items.remove(index); + (handle, len, offset) + }; + handle.cleanup(world); + + // Update indices for items after the removal + { + let handles_to_update: Vec<_> = { + let state = world.get::(*output_signal).unwrap(); + state.items + .iter() + .enumerate() + .skip(index) + .map(|(i, (h, _))| (**h, i)) + .collect() + }; + for (handle_entity, new_index) in handles_to_update { + if let Some(mut idx) = world.get_mut::(*handle_entity) { + idx.0 = new_index; + } + } + } + + for _ in 0..old_len { + out_diffs.push(VecDiff::RemoveAt { index: offset }); + } + } + VecDiff::Move { old_index, new_index } => { + let (old_offset, moved_len) = { + let mut state = world.get_mut::(*output_signal).unwrap(); + let old_offset: usize = state.items[..old_index].iter().map(|(_, len)| *len).sum(); + let item = state.items.remove(old_index); + let moved_len = item.1; + state.items.insert(new_index, item); + (old_offset, moved_len) + }; + + // Update indices for all affected items + { + let handles_to_update: Vec<_> = { + let state = world.get::(*output_signal).unwrap(); + let start = old_index.min(new_index); + let end = old_index.max(new_index) + 1; + state.items + .iter() + .enumerate() + .skip(start) + .take(end - start) + .map(|(i, (h, _))| (**h, i)) + .collect() + }; + for (handle_entity, new_idx) in handles_to_update { + if let Some(mut idx) = world.get_mut::(*handle_entity) { + idx.0 = new_idx; + } + } + } + + // Calculate the new offset after the move + let new_offset: usize = { + let state = world.get::(*output_signal).unwrap(); + state.items[..new_index].iter().map(|(_, len)| *len).sum() + }; + + // Emit Move diffs for each element in the block + // We need to move elements one by one from old position to new position + if old_offset < new_offset { + // Moving forward: move from end to start to avoid overwriting + for i in (0..moved_len).rev() { + out_diffs.push(VecDiff::Move { + old_index: old_offset + i, + new_index: new_offset + i, + }); + } + } else if old_offset > new_offset { + // Moving backward: move from start to end + for i in 0..moved_len { + out_diffs.push(VecDiff::Move { + old_index: old_offset + i, + new_index: new_offset + i, + }); + } + } + // If offsets are equal, no moves needed + } + VecDiff::Push { value: inner_signal_vec } => { + let index = { + let state = world.get::(*output_signal).unwrap(); + state.items.len() + }; + + // Create processor + let (handle, triggers) = create_inner_processor(world, output_signal, index, inner_signal_vec); + + // Push entry with real handle + { + let mut state = world.get_mut::(*output_signal).unwrap(); + state.items.push((handle, 0)); + } + + // Now trigger replays (entry exists) + trigger_replays(world, triggers); + } + VecDiff::Pop => { + let (handle, old_len, offset) = { + let mut state = world.get_mut::(*output_signal).unwrap(); + if let Some((handle, len)) = state.items.pop() { + let offset: usize = state.items.iter().map(|(_, len)| *len).sum(); + (Some(handle), len, offset) + } else { + (None, 0, 0) + } + }; + + if let Some(handle) = handle { + handle.cleanup(world); + } + for _ in 0..old_len { + out_diffs.push(VecDiff::RemoveAt { index: offset }); + } + } + VecDiff::Clear => { + let old_items = { + let mut state = world.get_mut::(*output_signal).unwrap(); + state.items.drain(..).collect::>() + }; + for (handle, _) in old_items { + handle.cleanup(world); + } + out_diffs.push(VecDiff::Clear); + } + } + } + + // Queue the diffs + if !out_diffs.is_empty() { + if let Some(mut queue) = world.get_mut::>>(*output_signal) { + queue.0.extend(out_diffs); + } + process_signals(world, [output_signal], Box::new(())); + } + }); + + let manager_handle = self.for_each(manager_logic).register(world); + + // Pipe manager to output_signal so output_signal processes AFTER manager fills the queue + pipe_signal(world, *manager_handle, output_signal); + + world + .entity_mut(*output_signal) + .insert(( + FlattenState { items: vec![] }, + QueuedVecDiffs::>(vec![]), + )) + .insert(SignalHandles::from([manager_handle])); output_signal }); @@ -3694,7 +3860,7 @@ pub trait SignalVecExt: SignalVec { signal, _marker: PhantomData, } - } */ + } #[cfg(feature = "tracing")] #[track_caller] @@ -4005,10 +4171,9 @@ impl MutableVec { let was_initially_empty = self_.read(&*world).is_empty(); let replay_entity = LazyEntity::new(); - let is_first_replay = Arc::new(AtomicBool::new(true)); - let replay_system = clone!((self_, replay_entity, is_first_replay) move |In(upstream_diffs): In>>, replay_onces: Query<&ReplayOnce, Allow>, mutable_vec_datas: Query<&MutableVecData>| { + let replay_system = clone!((self_, replay_entity) move |In(upstream_diffs): In>>, replay_onces: Query<&ReplayOnce, Allow>, mutable_vec_datas: Query<&MutableVecData>, mut has_replayed: Local| { if replay_onces.contains(*replay_entity) { - let first_replay = is_first_replay.swap(false, AtomicOrdering::SeqCst); + let first_replay = !core::mem::replace(&mut *has_replayed, true); if first_replay && was_initially_empty { // Initial replay with empty vec - just forward upstream diffs if upstream_diffs.is_empty() { @@ -7589,138 +7754,578 @@ pub(crate) mod tests { cleanup(true) } - /* #[test] + #[test] fn test_flatten() { - let mut app = create_test_app(); - app.init_resource::>(); - - // --- Setup --- - // Create the inner vectors that will be the items of our outer vector. - let vec_a = MutableVec::from([10u32, 11]); - let vec_b = MutableVec::from([20u32]); - let vec_c = MutableVec::from([]); // Start with an empty inner vec - - // Create the outer vector, which holds SignalVecs. - let source_of_vecs = MutableVec::from([vec_a.signal_vec(), vec_b.signal_vec(), vec_c.signal_vec()]); - - // The signal chain under test. - let flattened_signal = source_of_vecs.signal_vec().flatten(); - - let handle = flattened_signal - .for_each(capture_signal_vec_output) - .register(app.world_mut()); - - // --- 1. Initial State --- - // The first update should replay the initial state of all inner vectors. - app.update(); - let diffs = get_and_clear_output::(app.world_mut()); - assert_eq!(diffs.len(), 1, "Initial update should produce one Replace diff."); - assert_eq!( - diffs[0], - VecDiff::Replace { - values: vec![10, 11, 20] - }, - "Initial state should be a Replace with combined contents." - ); + { + // A comprehensive test for `SignalVecExt::flatten`. This tests flattening a + // SignalVec of SignalVecs into a single SignalVec, covering: + // - Initial state + // - Inner vec changes (push, pop, update, etc.) + // - Outer vec changes (push, pop, remove, move, clear, replace, update) + // - Cleanup verification (removed inner vecs should be ignored) - let mut current_state = vec![10, 11, 20]; + let mut app = create_test_app(); + app.init_resource::>(); - // --- 2. Inner Vec Change: Push to `vec_a` --- - // A push to the _first_ inner vector should insert at the correct global index. - vec_a.write().push(12); - app.update(); + // --- Setup --- + // Create the inner vectors that will be the items of our outer vector. + let vec_a = MutableVecBuilder::from([10u32, 11]).spawn(app.world_mut()); + let vec_b = MutableVecBuilder::from([20u32]).spawn(app.world_mut()); + let vec_c = MutableVecBuilder::::from([]).spawn(app.world_mut()); // Start with an empty inner vec + + // Create the outer vector, which holds SignalVecs. + // No need for boxed_clone() since all inner vecs are the same concrete type (Source) + let source_of_vecs = MutableVecBuilder::from([ + vec_a.signal_vec(), + vec_b.signal_vec(), + vec_c.signal_vec(), + ]) + .spawn(app.world_mut()); - let diffs = get_and_clear_output::(app.world_mut()); - assert_eq!(diffs.len(), 1, "Push to inner vec_a should produce one diff."); - assert_eq!( - diffs[0], - VecDiff::InsertAt { index: 2, value: 12 }, - "Should insert at index 2 (end of vec_a's block)." - ); - apply_diffs(&mut current_state, diffs); - assert_eq!(current_state, vec![10, 11, 12, 20]); - - // --- 3. Inner Vec Change: Push to `vec_b` --- - // A push to the _second_ inner vector should insert at an offset index. - vec_b.write().push(21); - app.update(); - - let diffs = get_and_clear_output::(app.world_mut()); - assert_eq!(diffs.len(), 1, "Push to inner vec_b should produce one diff."); - // Global index = len(vec_a) + new_index_in_b = 3 + 1 = 4 - assert_eq!( - diffs[0], - VecDiff::InsertAt { index: 4, value: 21 }, - "Should insert at index 4 (end of vec_b's block)." - ); - apply_diffs(&mut current_state, diffs); - assert_eq!(current_state, vec![10, 11, 12, 20, 21]); - - // --- 4. Outer Vec Change: Remove `vec_b` --- - // Removing an inner vector should remove its entire block of items. - source_of_vecs.write().remove(1); // Removes vec_b - app.update(); - - let diffs = get_and_clear_output::(app.world_mut()); - // The implementation should produce two RemoveAt diffs for vec_b's two items. - assert_eq!(diffs.len(), 2, "Removing vec_b should produce 2 diffs."); - apply_diffs(&mut current_state, diffs); - // The final state should no longer contain 20 or 21. - assert_eq!( - current_state, - vec![10, 11, 12], - "State after removing vec_b is incorrect." - ); - // Note: vec_c is now at index 1 in the outer vec. - - // --- 5. Cleanup Verification --- - // Updates to the removed `vec_b` should now be ignored. - vec_b.write().push(99); - app.update(); - let diffs = get_and_clear_output::(app.world_mut()); - assert!(diffs.is_empty(), "Should ignore diffs from the removed vec_b."); - - // --- 6. Outer Vec Change: Push a new vector --- - let vec_d = MutableVec::from([40u32, 41]); - source_of_vecs.write().push(vec_d.signal_vec()); - app.update(); - - let diffs = get_and_clear_output::(app.world_mut()); - // Pushing a new inner vector should add its initial items to the end. - assert_eq!(diffs.len(), 2, "Pushing new vec_d should produce 2 diffs."); - apply_diffs(&mut current_state, diffs); - assert_eq!( - current_state, - vec![10, 11, 12, 40, 41], - "State after pushing vec_d is incorrect." - ); + // The signal chain under test. + let flattened_signal = source_of_vecs.signal_vec().flatten(); - // --- 7. Outer Vec Change: Move --- - // Current outer vec: [vec_a, vec_c, vec_d]. - // Current state: [10, 11, 12, (empty), 40, 41]. - // Move vec_d (index 2) to the front (index 0). - source_of_vecs.write().move_item(2, 0); - app.update(); - - let diffs = get_and_clear_output::(app.world_mut()); - apply_diffs(&mut current_state, diffs); - assert_eq!( - current_state, - vec![40, 41, 10, 11, 12], - "State after moving vec_d to front is incorrect." - ); + let handle = flattened_signal + .for_each(capture_signal_vec_output) + .register(app.world_mut()); - // --- 8. Outer Vec Change: Clear --- - // Clearing the outer vector should remove all items. - source_of_vecs.write().clear(); - app.update(); + // --- 1. Initial State --- + // The first update should replay the initial state of all inner vectors. + app.update(); + let diffs = get_and_clear_output::(app.world_mut()); + // Initial state should be Clear + Push for each item + let mut current_state: Vec = vec![]; + apply_diffs(&mut current_state, diffs); + assert_eq!( + current_state, + vec![10, 11, 20], + "Initial state should contain combined contents of all inner vecs." + ); + + // --- 2. Inner Vec Change: Push to `vec_a` --- + // A push to the _first_ inner vector should insert at the correct global index. + vec_a.write(app.world_mut()).push(12); + app.update(); - let diffs = get_and_clear_output::(app.world_mut()); - assert_eq!(diffs.len(), 1, "Clear on outer vec should produce one diff."); - assert_eq!(diffs[0], VecDiff::Clear, "Expected a Clear diff."); - apply_diffs(&mut current_state, diffs); - assert!(current_state.is_empty(), "Final state should be empty after clear."); + let diffs = get_and_clear_output::(app.world_mut()); + assert_eq!(diffs.len(), 1, "Push to inner vec_a should produce one diff."); + assert_eq!( + diffs[0], + VecDiff::InsertAt { index: 2, value: 12 }, + "Should insert at index 2 (end of vec_a's block)." + ); + apply_diffs(&mut current_state, diffs); + assert_eq!(current_state, vec![10, 11, 12, 20]); - handle.cleanup(app.world_mut()); - } */ -} + // --- 3. Inner Vec Change: Push to `vec_b` --- + // A push to the _second_ inner vector should insert at an offset index. + vec_b.write(app.world_mut()).push(21); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + assert_eq!(diffs.len(), 1, "Push to inner vec_b should produce one diff."); + // Global index = len(vec_a) + new_index_in_b = 3 + 1 = 4 + assert_eq!( + diffs[0], + VecDiff::InsertAt { index: 4, value: 21 }, + "Should insert at index 4 (end of vec_b's block)." + ); + apply_diffs(&mut current_state, diffs); + assert_eq!(current_state, vec![10, 11, 12, 20, 21]); + + // --- 4. Inner Vec Change: UpdateAt on vec_a --- + vec_a.write(app.world_mut()).set(1, 111); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + assert_eq!(diffs.len(), 1, "UpdateAt on vec_a should produce one diff."); + assert_eq!( + diffs[0], + VecDiff::UpdateAt { index: 1, value: 111 }, + "Should update at correct global index." + ); + apply_diffs(&mut current_state, diffs); + assert_eq!(current_state, vec![10, 111, 12, 20, 21]); + + // --- 5. Inner Vec Change: RemoveAt on vec_a --- + vec_a.write(app.world_mut()).remove(0); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + assert_eq!(diffs.len(), 1, "RemoveAt on vec_a should produce one diff."); + assert_eq!(diffs[0], VecDiff::RemoveAt { index: 0 }); + apply_diffs(&mut current_state, diffs); + assert_eq!(current_state, vec![111, 12, 20, 21]); + + // --- 6. Inner Vec Change: Pop on vec_b --- + vec_b.write(app.world_mut()).pop(); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + assert_eq!(diffs.len(), 1, "Pop on vec_b should produce one diff."); + assert_eq!(diffs[0], VecDiff::RemoveAt { index: 3 }); + apply_diffs(&mut current_state, diffs); + assert_eq!(current_state, vec![111, 12, 20]); + + // --- 7. Outer Vec Change: Remove `vec_b` --- + // Removing an inner vector should remove its entire block of items. + source_of_vecs.write(app.world_mut()).remove(1); // Removes vec_b + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + // The implementation should produce one RemoveAt diff for vec_b's remaining item. + assert_eq!(diffs.len(), 1, "Removing vec_b should produce 1 diff."); + apply_diffs(&mut current_state, diffs); + // The final state should no longer contain 20. + assert_eq!( + current_state, + vec![111, 12], + "State after removing vec_b is incorrect." + ); + // Note: vec_c is now at index 1 in the outer vec. + + // --- 8. Cleanup Verification --- + // Updates to the removed `vec_b` should now be ignored. + vec_b.write(app.world_mut()).push(99); + app.update(); + let diffs = get_and_clear_output::(app.world_mut()); + assert!(diffs.is_empty(), "Should ignore diffs from the removed vec_b."); + + // --- 9. Outer Vec Change: Push a new vector --- + let vec_d = MutableVecBuilder::from([40u32, 41]).spawn(app.world_mut()); + source_of_vecs.write(app.world_mut()).push(vec_d.signal_vec()); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + // Pushing a new inner vector should add its initial items to the end. + assert_eq!(diffs.len(), 2, "Pushing new vec_d should produce 2 diffs."); + apply_diffs(&mut current_state, diffs); + assert_eq!( + current_state, + vec![111, 12, 40, 41], + "State after pushing vec_d is incorrect." + ); + + // --- 10. Inner Vec Change: Push to empty vec_c --- + // vec_c is currently at outer index 1 (between vec_a and vec_d) + vec_c.write(app.world_mut()).push(30); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + assert_eq!(diffs.len(), 1, "Push to empty vec_c should produce one diff."); + // Global index = len(vec_a) = 2 + assert_eq!( + diffs[0], + VecDiff::InsertAt { index: 2, value: 30 }, + "Should insert at correct position between vec_a and vec_d." + ); + apply_diffs(&mut current_state, diffs); + assert_eq!(current_state, vec![111, 12, 30, 40, 41]); + + // --- 11. Outer Vec Change: Move --- + // Current outer vec: [vec_a, vec_c, vec_d]. + // Current state: [111, 12, 30, 40, 41]. + // Move vec_d (index 2) to the front (index 0). + source_of_vecs.write(app.world_mut()).move_item(2, 0); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + apply_diffs(&mut current_state, diffs); + assert_eq!( + current_state, + vec![40, 41, 111, 12, 30], + "State after moving vec_d to front is incorrect." + ); + + // --- 12. Outer Vec Change: UpdateAt (replace vec_c with a new vec) --- + let vec_e = MutableVecBuilder::from([50u32, 51, 52]).spawn(app.world_mut()); + source_of_vecs.write(app.world_mut()).set(2, vec_e.signal_vec()); // Replace vec_c + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + apply_diffs(&mut current_state, diffs); + // vec_c had [30], vec_e has [50, 51, 52] + assert_eq!( + current_state, + vec![40, 41, 111, 12, 50, 51, 52], + "State after UpdateAt on outer vec is incorrect." + ); + + // --- 13. Outer Vec Change: Pop --- + source_of_vecs.write(app.world_mut()).pop(); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + apply_diffs(&mut current_state, diffs); + assert_eq!( + current_state, + vec![40, 41, 111, 12], + "State after Pop on outer vec is incorrect." + ); + + // --- 14. Outer Vec Change: Clear --- + // Clearing the outer vector should remove all items. + source_of_vecs.write(app.world_mut()).clear(); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + assert_eq!(diffs.len(), 1, "Clear on outer vec should produce one diff."); + assert_eq!(diffs[0], VecDiff::Clear, "Expected a Clear diff."); + apply_diffs(&mut current_state, diffs); + assert!(current_state.is_empty(), "Final state should be empty after clear."); + + // --- 15. Outer Vec Change: Replace (re-populate after clear) --- + let vec_f = MutableVecBuilder::from([60u32]).spawn(app.world_mut()); + let vec_g = MutableVecBuilder::from([70u32, 71]).spawn(app.world_mut()); + source_of_vecs.write(app.world_mut()).replace(vec![ + vec_f.signal_vec(), + vec_g.signal_vec(), + ]); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + apply_diffs(&mut current_state, diffs); + assert_eq!( + current_state, + vec![60, 70, 71], + "State after Replace on outer vec is incorrect." + ); + + // --- 16. Inner Vec Change: Move within vec_g --- + vec_g.write(app.world_mut()).move_item(0, 1); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + assert_eq!(diffs.len(), 1, "Move within vec_g should produce one diff."); + assert_eq!( + diffs[0], + VecDiff::Move { + old_index: 1, + new_index: 2 + }, + "Move should be translated to correct global indices." + ); + apply_diffs(&mut current_state, diffs); + assert_eq!(current_state, vec![60, 71, 70]); + + // --- 17. Inner Vec Change: Clear on vec_g --- + vec_g.write(app.world_mut()).clear(); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + // Clear should produce RemoveAt diffs for each item + assert_eq!(diffs.len(), 2, "Clear on vec_g should produce 2 diffs."); + apply_diffs(&mut current_state, diffs); + assert_eq!(current_state, vec![60]); + + // --- 18. Inner Vec Change: Replace on vec_f --- + vec_f.write(app.world_mut()).replace(vec![61, 62, 63]); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + apply_diffs(&mut current_state, diffs); + assert_eq!( + current_state, + vec![61, 62, 63], + "State after Replace on vec_f is incorrect." + ); + + // --- Cleanup --- + handle.cleanup(app.world_mut()); + } + + // Additional edge case tests + { + // --- 19. InsertAt on outer vec (insert in middle) --- + let mut app = create_test_app(); + app.init_resource::>(); + + let vec_a = MutableVecBuilder::from([10u32, 11]).spawn(app.world_mut()); + let vec_b = MutableVecBuilder::from([30u32, 31]).spawn(app.world_mut()); + + let source = MutableVecBuilder::from([ + vec_a.signal_vec(), + vec_b.signal_vec(), + ]) + .spawn(app.world_mut()); + + let handle = source + .signal_vec() + .flatten() + .for_each(capture_signal_vec_output) + .register(app.world_mut()); + + app.update(); + let diffs = get_and_clear_output::(app.world_mut()); + let mut current_state: Vec = vec![]; + apply_diffs(&mut current_state, diffs); + assert_eq!(current_state, vec![10, 11, 30, 31], "Initial state"); + + // Insert a new inner vec in the middle + let vec_mid = MutableVecBuilder::from([20u32, 21, 22]).spawn(app.world_mut()); + source.write(app.world_mut()).insert(1, vec_mid.signal_vec()); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + apply_diffs(&mut current_state, diffs); + assert_eq!( + current_state, + vec![10, 11, 20, 21, 22, 30, 31], + "InsertAt on outer vec should insert items at correct position." + ); + + // Verify that updates to the inserted vec work correctly + vec_mid.write(app.world_mut()).push(23); + app.update(); + let diffs = get_and_clear_output::(app.world_mut()); + apply_diffs(&mut current_state, diffs); + assert_eq!( + current_state, + vec![10, 11, 20, 21, 22, 23, 30, 31], + "Push to inserted vec should work correctly." + ); + + // Verify that the vec after the insertion still has correct indices + vec_b.write(app.world_mut()).push(32); + app.update(); + let diffs = get_and_clear_output::(app.world_mut()); + apply_diffs(&mut current_state, diffs); + assert_eq!( + current_state, + vec![10, 11, 20, 21, 22, 23, 30, 31, 32], + "Push to vec after insertion should work correctly." + ); + + handle.cleanup(app.world_mut()); + } + + { + // --- 20. Multiple simultaneous inner vec changes --- + let mut app = create_test_app(); + app.init_resource::>(); + + let vec_a = MutableVecBuilder::from([10u32]).spawn(app.world_mut()); + let vec_b = MutableVecBuilder::from([20u32]).spawn(app.world_mut()); + + let source = MutableVecBuilder::from([ + vec_a.signal_vec(), + vec_b.signal_vec(), + ]) + .spawn(app.world_mut()); + + let handle = source + .signal_vec() + .flatten() + .for_each(capture_signal_vec_output) + .register(app.world_mut()); + + app.update(); + let diffs = get_and_clear_output::(app.world_mut()); + let mut current_state: Vec = vec![]; + apply_diffs(&mut current_state, diffs); + assert_eq!(current_state, vec![10, 20], "Initial state"); + + // Make changes to both inner vecs before update + vec_a.write(app.world_mut()).push(11); + vec_b.write(app.world_mut()).push(21); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + apply_diffs(&mut current_state, diffs); + assert_eq!( + current_state, + vec![10, 11, 20, 21], + "Multiple simultaneous changes should all be processed." + ); + + handle.cleanup(app.world_mut()); + } + + { + // --- 21. Empty outer vec initially --- + // Note: When starting with an empty outer vec, we need to specify the type explicitly. + // Using Source directly avoids boxing. + let mut app = create_test_app(); + app.init_resource::>(); + + let source = MutableVecBuilder::>::from([]).spawn(app.world_mut()); + + let handle = source + .signal_vec() + .flatten() + .for_each(capture_signal_vec_output) + .register(app.world_mut()); + + app.update(); + let diffs = get_and_clear_output::(app.world_mut()); + let mut current_state: Vec = vec![]; + apply_diffs(&mut current_state, diffs); + assert!(current_state.is_empty(), "Initial state should be empty"); + + // Push first inner vec + let vec_a = MutableVecBuilder::from([10u32, 11]).spawn(app.world_mut()); + source.write(app.world_mut()).push(vec_a.signal_vec()); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + apply_diffs(&mut current_state, diffs); + assert_eq!(current_state, vec![10, 11], "Push to empty outer vec"); + + // Push second inner vec + let vec_b = MutableVecBuilder::from([20u32]).spawn(app.world_mut()); + source.write(app.world_mut()).push(vec_b.signal_vec()); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + apply_diffs(&mut current_state, diffs); + assert_eq!(current_state, vec![10, 11, 20], "Second push to outer vec"); + + handle.cleanup(app.world_mut()); + } + + { + // --- 22. Move forward on outer vec (index 0 -> 2) --- + let mut app = create_test_app(); + app.init_resource::>(); + + let vec_a = MutableVecBuilder::from([10u32, 11]).spawn(app.world_mut()); + let vec_b = MutableVecBuilder::from([20u32]).spawn(app.world_mut()); + let vec_c = MutableVecBuilder::from([30u32, 31, 32]).spawn(app.world_mut()); + + let source = MutableVecBuilder::from([ + vec_a.signal_vec(), + vec_b.signal_vec(), + vec_c.signal_vec(), + ]) + .spawn(app.world_mut()); + + let handle = source + .signal_vec() + .flatten() + .for_each(capture_signal_vec_output) + .register(app.world_mut()); + + app.update(); + let diffs = get_and_clear_output::(app.world_mut()); + let mut current_state: Vec = vec![]; + apply_diffs(&mut current_state, diffs); + assert_eq!(current_state, vec![10, 11, 20, 30, 31, 32], "Initial state"); + + // Move vec_a (index 0) to the end (index 2) + source.write(app.world_mut()).move_item(0, 2); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + apply_diffs(&mut current_state, diffs); + assert_eq!( + current_state, + vec![20, 30, 31, 32, 10, 11], + "Move forward on outer vec should work correctly." + ); + + // Verify updates still work after move + vec_a.write(app.world_mut()).push(12); + app.update(); + let diffs = get_and_clear_output::(app.world_mut()); + apply_diffs(&mut current_state, diffs); + assert_eq!( + current_state, + vec![20, 30, 31, 32, 10, 11, 12], + "Push to moved vec should work correctly." + ); + + handle.cleanup(app.world_mut()); + } + + { + // --- 23. Moving an empty inner vec --- + let mut app = create_test_app(); + app.init_resource::>(); + + let vec_a = MutableVecBuilder::from([10u32]).spawn(app.world_mut()); + let vec_empty = MutableVecBuilder::::from([]).spawn(app.world_mut()); + let vec_c = MutableVecBuilder::from([30u32]).spawn(app.world_mut()); + + let source = MutableVecBuilder::from([ + vec_a.signal_vec(), + vec_empty.signal_vec(), + vec_c.signal_vec(), + ]) + .spawn(app.world_mut()); + + let handle = source + .signal_vec() + .flatten() + .for_each(capture_signal_vec_output) + .register(app.world_mut()); + + app.update(); + let diffs = get_and_clear_output::(app.world_mut()); + let mut current_state: Vec = vec![]; + apply_diffs(&mut current_state, diffs); + assert_eq!(current_state, vec![10, 30], "Initial state with empty inner vec"); + + // Move the empty vec to the end + source.write(app.world_mut()).move_item(1, 2); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + // Moving empty vec should produce no move diffs + assert!(diffs.is_empty(), "Moving empty inner vec should produce no diffs"); + assert_eq!(current_state, vec![10, 30], "State unchanged after moving empty vec"); + + // Verify the empty vec now works at its new position + vec_empty.write(app.world_mut()).push(20); + app.update(); + let diffs = get_and_clear_output::(app.world_mut()); + apply_diffs(&mut current_state, diffs); + assert_eq!( + current_state, + vec![10, 30, 20], + "Push to moved empty vec should insert at end." + ); + + handle.cleanup(app.world_mut()); + } + + { + // --- 24. Moving a non-empty inner vec vs empty (verify Move diffs emitted) --- + let mut app = create_test_app(); + app.init_resource::>(); + + let vec_a = MutableVecBuilder::from([10u32, 11]).spawn(app.world_mut()); + let vec_b = MutableVecBuilder::from([20u32]).spawn(app.world_mut()); + + let source = MutableVecBuilder::from([ + vec_a.signal_vec(), + vec_b.signal_vec(), + ]) + .spawn(app.world_mut()); + + let handle = source + .signal_vec() + .flatten() + .for_each(capture_signal_vec_output) + .register(app.world_mut()); + + app.update(); + let diffs = get_and_clear_output::(app.world_mut()); + let mut current_state: Vec = vec![]; + apply_diffs(&mut current_state, diffs); + assert_eq!(current_state, vec![10, 11, 20], "Initial state"); + + // Move vec_a (2 items) to after vec_b + source.write(app.world_mut()).move_item(0, 1); + app.update(); + + let diffs = get_and_clear_output::(app.world_mut()); + // Should emit 2 Move diffs (one for each item in vec_a) + assert_eq!(diffs.len(), 2, "Moving vec with 2 items should produce 2 Move diffs"); + apply_diffs(&mut current_state, diffs); + assert_eq!(current_state, vec![20, 10, 11], "State after move"); + + handle.cleanup(app.world_mut()); + } + + cleanup(true); + }} \ No newline at end of file From aca97cfe54c6ec06adca0cfe166d9385b0d32bac Mon Sep 17 00:00:00 2001 From: databasedav <31483365+databasedav@users.noreply.github.com> Date: Fri, 16 Jan 2026 03:29:33 -0800 Subject: [PATCH 09/15] topological eval --- src/graph.rs | 757 ++++++++++++++++++++++++++++++++++++++++++++------ src/lib.rs | 1 + src/signal.rs | 196 ++++++++++--- 3 files changed, 839 insertions(+), 115 deletions(-) diff --git a/src/graph.rs b/src/graph.rs index 64522db..fc4b733 100644 --- a/src/graph.rs +++ b/src/graph.rs @@ -1,5 +1,6 @@ //! Signal graph management and runtime. use super::utils::SSs; +use alloc::collections::VecDeque; use bevy_derive::Deref; use bevy_ecs::{ entity_disabling::Internal, @@ -134,6 +135,7 @@ pub(crate) fn pipe_signal(world: &mut World, source: SignalSystem, target: Signa downstream.insert(Upstream(HashSet::from([source]))); } } + world.resource_mut::().edge_change_seeds.insert(target); } #[derive(Component, Clone)] @@ -198,47 +200,458 @@ clone_trait_object!(AnyClone); impl AnyClone for T {} + +/// Tracks signal graph topology for level-based processing. +#[derive(Resource, Default)] +pub(crate) struct SignalGraphState { + /// Cached level for each signal (max distance from any root). + levels: HashMap, + /// Signals grouped by level for efficient iteration. + by_level: Vec>, + /// Signals that seed level recomputation after edge changes. + edge_change_seeds: HashSet, + /// Signals queued for removal while the graph is being processed. + deferred_removals: HashSet, + /// Whether the signal graph is currently being processed. + is_processing: bool, +} + +// Fan out a signal output to downstream input queues. When there is exactly one downstream, move the value without cloning. +fn enqueue_inputs( + world: &World, + inputs: &mut HashMap>>, + signal: SignalSystem, + value: Box, +) { + let downstreams = get_downstreams(world, signal); + if downstreams.is_empty() { + return; + } + + if downstreams.len() == 1 { + // avoid cloning if there's only a single downstream + inputs + .entry(downstreams[0]) + .or_default() + .push_back(value); + return; + } + + if let Some((last, rest)) = downstreams.split_last() { + for downstream in rest { + inputs + .entry(*downstream) + .or_default() + .push_back(value.clone()); + } + inputs.entry(*last).or_default().push_back(value); + } +} + +fn get_upstreams(world: &World, signal: SignalSystem) -> Vec { + world + .get::(*signal) + .map(|u| u.0.iter().copied().collect()) + .unwrap_or_default() +} + +fn get_downstreams(world: &World, signal: SignalSystem) -> Vec { + world + .get::(*signal) + .map(|d| d.0.iter().copied().collect()) + .unwrap_or_default() +} + +fn insert_sorted_by_index(bucket: &mut Vec, signal: SignalSystem) { + if bucket.iter().any(|s| *s == signal) { + return; + } + let key = signal.index(); + let index = bucket + .binary_search_by_key(&key, |s| s.index()) + .unwrap_or_else(|i| i); + bucket.insert(index, signal); +} + +// Computes a per-call, local topological ordering of signals reachable downstream +// from `seeds`. This intentionally does NOT use `SignalGraphState` because it only +// needs a lightweight traversal for the provided subset and should not mutate or +// depend on the global cached topology. +fn downstream_levels_from_seeds( + world: &World, + seeds: &[SignalSystem], +) -> Vec> { + let mut levels: HashMap = HashMap::new(); + let mut by_level: Vec> = Vec::new(); + let mut queue: VecDeque = VecDeque::new(); + + for signal in seeds { + levels.insert(*signal, 0); + queue.push_back(*signal); + } + + while let Some(signal) = queue.pop_front() { + let level = *levels.get(&signal).unwrap_or(&0); + for downstream in get_downstreams(world, signal) { + let next_level = level.saturating_add(1); + let current = levels.get(&downstream).copied().unwrap_or(0); + if next_level > current { + levels.insert(downstream, next_level); + queue.push_back(downstream); + } + } + } + + for (signal, level) in levels.iter() { + while by_level.len() <= *level as usize { + by_level.push(HashSet::new()); + } + by_level[*level as usize].insert(*signal); + } + + by_level + .into_iter() + .map(|level| { + let mut signals_at_level: Vec = level.into_iter().collect(); + // Sort by Entity index to get a deterministic, stable order independent of hash iteration. + signals_at_level.sort_by_key(|signal| signal.index()); + signals_at_level + }) + .collect() +} + +// Rebuilds per-signal levels using a Kahn-style topological traversal. +// +// - Roots (in-degree 0) start at level 0. +// - Each node's level is 1 + max(level of its upstreams). +// - Nodes are bucketed by level for deterministic per-level iteration. +// - If a cycle or inconsistent edges are detected (not all nodes processed), +// this panics because the graph invariants were violated. +fn rebuild_levels(world: &mut World, state: &mut SignalGraphState) { + state.levels.clear(); + state.by_level.clear(); + + let mut all_signals = + SystemState::, Allow)>>::new(world); + let signals = all_signals + .get(world) + .iter() + .map(SignalSystem) + .collect::>(); + + let mut in_degree: HashMap = HashMap::new(); + let mut upstreams_map: HashMap> = HashMap::new(); + let mut downstreams_map: HashMap> = HashMap::new(); + + for signal in signals { + let upstreams = get_upstreams(world, signal); + in_degree.insert(signal, upstreams.len()); + upstreams_map.insert(signal, upstreams.clone()); + for upstream in upstreams { + downstreams_map.entry(upstream).or_default().push(signal); + } + } + + let mut queue: VecDeque = in_degree + .iter() + .filter_map(|(signal, degree)| if *degree == 0 { Some(*signal) } else { None }) + .collect(); + + let mut processed = 0usize; + while let Some(signal) = queue.pop_front() { + processed += 1; + let upstreams = upstreams_map.get(&signal).cloned().unwrap_or_default(); + let level = if upstreams.is_empty() { + 0 + } else { + upstreams + .iter() + .filter_map(|u| state.levels.get(u)) + .max() + .map(|m| m + 1) + .unwrap_or(0) + }; + + while state.by_level.len() <= level as usize { + state.by_level.push(Vec::new()); + } + state.by_level[level as usize].push(signal); + state.levels.insert(signal, level); + + if let Some(downstreams) = downstreams_map.get(&signal) { + for downstream in downstreams { + if let Some(count) = in_degree.get_mut(downstream) { + *count = count.saturating_sub(1); + if *count == 0 { + queue.push_back(*downstream); + } + } + } + } + } + + if processed < in_degree.len() { + panic!("signal graph contains a cycle or inconsistent edges during level rebuild"); + } + + for bucket in state.by_level.iter_mut() { + bucket.sort_by_key(|signal| signal.index()); + } +} + +fn update_levels_incremental(world: &mut World, state: &mut SignalGraphState) -> bool { + let mut affected: HashSet = HashSet::new(); + let mut queue: VecDeque = state.edge_change_seeds.iter().copied().collect(); + + while let Some(signal) = queue.pop_front() { + if affected.insert(signal) { + for downstream in get_downstreams(world, signal) { + queue.push_back(downstream); + } + } + } + if affected.is_empty() { + return true; + } + + let mut in_degree: HashMap = HashMap::new(); + let mut upstreams_map: HashMap> = HashMap::new(); + + for signal in affected.iter().copied() { + let upstreams = get_upstreams(world, signal); + let local_in_degree = upstreams + .iter() + .filter(|u| affected.contains(&(**u))) + .count(); + in_degree.insert(signal, local_in_degree); + upstreams_map.insert(signal, upstreams); + } + + let mut queue: VecDeque = in_degree + .iter() + .filter_map(|(signal, degree)| if *degree == 0 { Some(*signal) } else { None }) + .collect(); + + let mut new_levels: HashMap = HashMap::new(); + let mut processed = 0usize; + + while let Some(signal) = queue.pop_front() { + processed += 1; + let upstreams = upstreams_map.get(&signal).cloned().unwrap_or_default(); + + let mut level = 0u32; + for upstream in upstreams { + let upstream_level = new_levels + .get(&upstream) + .copied() + .or_else(|| state.levels.get(&upstream).copied()) + .unwrap_or(0); + level = level.max(upstream_level.saturating_add(1)); + } + + new_levels.insert(signal, level); + + for downstream in get_downstreams(world, signal) { + if !affected.contains(&downstream) { + continue; + } + if let Some(count) = in_degree.get_mut(&downstream) { + *count = count.saturating_sub(1); + if *count == 0 { + queue.push_back(downstream); + } + } + } + } + + if processed < affected.len() { + return false; + } + + for signal in affected { + if let Some(old_level) = state.levels.get(&signal).copied() { + if let Some(bucket) = state.by_level.get_mut(old_level as usize) { + if let Some(pos) = bucket.iter().position(|s| *s == signal) { + bucket.remove(pos); + } + } + } + + if let Some(new_level) = new_levels.get(&signal).copied() { + while state.by_level.len() <= new_level as usize { + state.by_level.push(Vec::new()); + } + insert_sorted_by_index(&mut state.by_level[new_level as usize], signal); + state.levels.insert(signal, new_level); + } else { + state.levels.remove(&signal); + } + } + + true +} + +fn update_edge_change_levels(world: &mut World, state: &mut SignalGraphState) { + if state.levels.is_empty() { + rebuild_levels(world, state); + state.edge_change_seeds.clear(); + return; + } + + if state.edge_change_seeds.is_empty() { + return; + } + + if !update_levels_incremental(world, state) { + panic!("signal graph contains a cycle or inconsistent edges during incremental update"); + } + state.edge_change_seeds.clear(); +} + +fn remove_signal_from_graph_state_internal(state: &mut SignalGraphState, signal: SignalSystem) { + if let Some(level) = state.levels.remove(&signal) { + if let Some(bucket) = state.by_level.get_mut(level as usize) { + if let Some(pos) = bucket.iter().position(|s| *s == signal) { + bucket.remove(pos); + } + } + } + state.edge_change_seeds.remove(&signal); +} + +fn remove_signal_from_graph_state(world: &mut World, signal: SignalSystem) { + if let Some(mut state) = world.get_resource_mut::() { + if state.is_processing { + state.deferred_removals.insert(signal); + return; + } + remove_signal_from_graph_state_internal(&mut state, signal); + } +} + +fn apply_deferred_removals(state: &mut SignalGraphState) { + if state.deferred_removals.is_empty() { + return; + } + let removals = state.deferred_removals.drain().collect::>(); + for signal in removals { + remove_signal_from_graph_state_internal(state, signal); + } +} + +fn process_signal( + world: &mut World, + signal: SignalSystem, + inputs: &mut HashMap>>, +) { + let runner = match world.get::(*signal).cloned() { + Some(runner) => runner, + None => { + if world.get_entity(*signal).is_err() { + // Re-entrant combinators can despawn signals during the same frame; skip these stale IDs. + return; + } + let upstreams = get_upstreams(world, signal); + let downstreams = get_downstreams(world, signal); + panic!( + "missing SystemRunner for signal {:?} during processing (entity exists). upstreams={:?}, downstreams={:?}", + signal, + upstreams, + downstreams + ); + } + }; + + let upstreams = get_upstreams(world, signal); + + if upstreams.is_empty() { + if let Some(output) = runner.run(world, Box::new(())) { + enqueue_inputs(world, inputs, signal, output); + } + return; + } + + let mut queue = inputs.remove(&signal).unwrap_or_default(); + while let Some(input) = queue.pop_front() { + if let Some(output) = runner.run(world, input) { + enqueue_inputs(world, inputs, signal, output); + } + } +} + pub(crate) fn process_signals( world: &mut World, - signals: impl IntoIterator, + signals: impl AsRef<[SignalSystem]>, input: Box, ) { - let mut iter = signals.into_iter().peekable(); - if let Some(first_signal) = iter.next() { - // avoid cloning the input if there's only a single downstream, according to - // gemini, the compiler cannot do this automatically - if iter.peek().is_none() { - if let Some(runner) = world - .get_entity(*first_signal) - .ok() - .and_then(|entity| entity.get::().cloned()) - && let Some(output) = runner.run(world, input) - && let Some(downstream) = world.get::(*first_signal).cloned() - { - process_signals(world, downstream.0, output); + let signals = signals.as_ref(); + if signals.is_empty() { + return; + } + + let mut inputs: HashMap>> = HashMap::new(); + let mut iter = signals.iter().copied().peekable(); + if let Some(first) = iter.next() { + let mut run_with_input = |signal: SignalSystem, input: Box| { + let runner = world + .get::(*signal) + .cloned() + .unwrap_or_else(|| { + panic!( + "missing SystemRunner for signal {:?} during processing", + signal + ) + }); + if let Some(output) = runner.run(world, input) { + enqueue_inputs(world, &mut inputs, signal, output); } + }; + + if iter.peek().is_none() { + // avoid cloning if there's only a single downstream + run_with_input(first, input); } else { - for signal in [first_signal].into_iter().chain(iter) { - if let Some(runner) = world - .get_entity(*signal) - .ok() - .and_then(|entity| entity.get::().cloned()) - && let Some(output) = runner.run(world, input.clone()) - && let Some(downstream) = world.get::(*signal).cloned() - { - process_signals(world, downstream.0, output); + let rest: Vec = iter.collect(); + if let Some((last, rest)) = rest.split_last() { + for signal in core::iter::once(first).chain(rest.iter().copied()) { + run_with_input(signal, input.clone()); } + run_with_input(*last, input); } } } + + let by_level = downstream_levels_from_seeds(world, signals); + let skip: HashSet = signals.iter().copied().collect(); + for level in by_level.into_iter().skip(1) { + for signal in level { + if skip.contains(&signal) { + continue; + } + process_signal(world, signal, &mut inputs); + } + } } pub(crate) fn process_signal_graph(world: &mut World) { - let mut orphan_parents = - SystemState::, Without, Allow)>>::new(world); - let orphan_parents = orphan_parents.get(world); - let orphan_parents = orphan_parents.iter().map(SignalSystem).collect::>(); - process_signals(world, orphan_parents, Box::new(())); + let mut levels_snapshot: Vec> = Vec::new(); + world.resource_scope(|world, mut state: Mut| { + update_edge_change_levels(world, &mut state); + state.is_processing = true; + levels_snapshot = core::mem::take(&mut state.by_level); + }); + + let mut inputs: HashMap>> = HashMap::new(); + for level in &levels_snapshot { + for &signal in level { + process_signal(world, signal, &mut inputs); + } + } + + let mut state = world.resource_mut::(); + state.is_processing = false; + state.by_level = levels_snapshot; + apply_deferred_removals(&mut state); } /// Handle to a particular node of the signal graph, returned by @@ -316,21 +729,21 @@ where IOO: Into> + 'static, F: IntoSystem, IOO, M> + 'static, { - let sys_id = world.register_system(system); - let entity = sys_id.entity(); - - // Wrap the typed node behind NodeErased + let signal_system = world.register_system(system); let runner: Arc> = Arc::new(Box::new(SystemHolder:: { - system: sys_id, + system: signal_system, _marker: PhantomData, })); - world.entity_mut(entity).insert(( + world.entity_mut(signal_system.entity()).insert(( SignalRegistrationCount::new(), SystemRunner { runner: Arc::new(Box::new(move |w, inp| runner.run(w, inp))), }, )); - entity.into() + if let Some(mut state) = world.get_resource_mut::() { + state.edge_change_seeds.insert(signal_system.entity().into()); + } + signal_system.into() } pub(crate) struct LazySignalState { @@ -414,14 +827,37 @@ pub(crate) struct LazySignalHolder(LazySignal); pub(crate) static STALE_SIGNALS: LazyLock>> = LazyLock::new(|| Mutex::new(Vec::new())); +fn unlink_downstreams_and_mark(world: &mut World, signal: SignalSystem) { + if let Some(downstreams) = world.get::(*signal).cloned() { + for &downstream in downstreams.iter() { + if let Ok(mut downstream_entity) = world.get_entity_mut(*downstream) + && let Some(mut upstream) = downstream_entity.get_mut::() + { + upstream.0.remove(&signal); + if upstream.0.is_empty() { + downstream_entity.remove::(); + } + } + if let Some(mut state) = world.get_resource_mut::() { + state.edge_change_seeds.insert(downstream); + } + } + } +} + pub(crate) fn despawn_stale_signals(world: &mut World) { let signals = STALE_SIGNALS.lock().unwrap().drain(..).collect::>(); for signal in signals { - if let Ok(entity) = world.get_entity_mut(*signal) - && let Some(registration_count) = entity.get::() - && **registration_count == 0 - { - entity.despawn(); + let should_despawn = world + .get::(*signal) + .map(|registration_count| **registration_count == 0) + .unwrap_or(false); + if should_despawn { + unlink_downstreams_and_mark(world, signal); + remove_signal_from_graph_state(world, signal); + if let Ok(entity) = world.get_entity_mut(*signal) { + entity.despawn(); + } } } } @@ -511,57 +947,66 @@ where } } -fn cleanup_recursive(world: &mut World, signal: SignalSystem) { - // Stage 1: Decrement the count and check if this node needs full cleanup. We do - // this in a short-lived block to release the borrow. - let mut needs_full_cleanup = false; +fn decrement_registration_and_needs_cleanup(world: &mut World, signal: SignalSystem) -> bool { if let Ok(mut entity) = world.get_entity_mut(*signal) && let Some(mut count) = entity.get_mut::() { count.decrement(); - if **count == 0 { - needs_full_cleanup = true; + return **count == 0; + } + false +} + +fn should_despawn_signal(world: &World, signal: SignalSystem) -> bool { + world + .get_entity(*signal) + .ok() + .map(|entity| { + if let Some(lazy_holder) = entity.get::() { + lazy_holder.0.inner.references.load(Ordering::SeqCst) == 1 + } else { + // This is a dynamically created signal without a holder, it can be despawned once + // its registrations are gone. + true + } + }) + .unwrap_or(false) +} + +fn unlink_from_upstream(world: &mut World, upstream_system: SignalSystem, signal: SignalSystem) { + if let Ok(mut upstream_entity) = world.get_entity_mut(*upstream_system) + && let Some(mut downstream) = upstream_entity.get_mut::() + { + downstream.0.remove(&signal); + if downstream.0.is_empty() { + upstream_entity.remove::(); } } +} - // If the count is not zero, we're done with this branch of the graph. - if !needs_full_cleanup { +fn cleanup_recursive(world: &mut World, signal: SignalSystem) { + // Stage 1: Decrement registration and bail if the node is still in use. + if !decrement_registration_and_needs_cleanup(world, signal) { return; } - // Stage 2: The count is zero. Perform the full cleanup. First, get the list of - // parents. + // Stage 2: The count is zero. Perform the full cleanup. First, get the list of parents. let upstreams = world.get::(*signal).cloned(); - // Now, we can despawn the current node if it's no longer needed. The check for - // LazySignalHolder is crucial. - if let Ok(entity) = world.get_entity_mut(*signal) { - if let Some(lazy_holder) = entity.get::() { - if lazy_holder.0.inner.references.load(Ordering::SeqCst) == 1 { - entity.despawn(); - } - } else { - // This is a dynamically created signal without a holder, it can be despawned once - // its registrations are gone. + // Unlink downstream edges and mark affected nodes for level recomputation. + unlink_downstreams_and_mark(world, signal); + + if should_despawn_signal(world, signal) { + remove_signal_from_graph_state(world, signal); + if let Ok(entity) = world.get_entity_mut(*signal) { entity.despawn(); } } - // Stage 3: Notify parents and recurse. This happens _after_ the current node has - // been processed and potentially despawned. + // Stage 3: Notify parents and recurse after processing this node. if let Some(upstreams) = upstreams { for &upstream_system in upstreams.iter() { - // Notify the parent to remove this signal from its downstream list. - if let Ok(mut upstream_entity) = world.get_entity_mut(*upstream_system) - && let Some(mut downstream) = upstream_entity.get_mut::() - { - downstream.0.remove(&signal); - if downstream.0.is_empty() { - upstream_entity.remove::(); - } - } - - // Now, recurse on the parent. + unlink_from_upstream(world, upstream_system, signal); cleanup_recursive(world, upstream_system); } } @@ -579,13 +1024,15 @@ fn poll_signal_one_shot(In(signal): In, world: &mut World) -> Opti } // 2. pull runner + upstream list - let runner = match world.get::(*node) { - Some(r) => r.clone(), - None => { - cache.insert(node, None); - return None; - } - }; + let runner = world + .get::(*node) + .cloned() + .unwrap_or_else(|| { + panic!( + "missing SystemRunner for signal {:?} during processing", + node + ) + }); let upstreams: Vec = world .get::(*node) .map(|u| { @@ -640,3 +1087,153 @@ pub fn poll_signal(world: &mut World, signal: SignalSystem) -> Option(any_clone: Box) -> Option { (any_clone as Box).downcast::().map(|o| *o).ok() } + +#[cfg(test)] +mod tests { + use super::*; + use bevy_ecs::prelude::{In, Mut, World}; + + #[derive(Resource, Default)] + struct Order(Vec<&'static str>); + + #[test] + #[should_panic(expected = "signal graph contains a cycle or inconsistent edges during incremental update")] + fn incremental_update_panics_on_cycle() { + let mut world = World::new(); + world.insert_resource(SignalGraphState::default()); + + let signal_a = spawn_signal::<(), i32, Option, _, _>(&mut world, |_: In<()>| Some(1)); + let signal_b = spawn_signal::<(), i32, Option, _, _>(&mut world, |_: In<()>| Some(2)); + + world.resource_scope(|world, mut state: Mut| { + rebuild_levels(world, &mut state); + }); + + world + .entity_mut(*signal_a) + .insert(Downstream(HashSet::from([signal_b]))) + .insert(Upstream(HashSet::from([signal_b]))); + world + .entity_mut(*signal_b) + .insert(Downstream(HashSet::from([signal_a]))) + .insert(Upstream(HashSet::from([signal_a]))); + + world + .resource_mut::() + .edge_change_seeds + .insert(signal_a); + + world.resource_scope(|world, mut state: Mut| { + update_edge_change_levels(world, &mut state); + }); + } + + #[test] + fn ordering_is_deterministic_with_multiple_roots() { + let mut world = World::new(); + world.insert_resource(SignalGraphState::default()); + world.insert_resource(Order::default()); + + let signal_a = spawn_signal::<(), (), Option<()>, _, _>(&mut world, |_: In<()>, mut order: ResMut| { + order.0.push("a"); + Some(()) + }); + let signal_b = spawn_signal::<(), (), Option<()>, _, _>(&mut world, |_: In<()>, mut order: ResMut| { + order.0.push("b"); + Some(()) + }); + + process_signal_graph(&mut world); + + let order = world.resource::().0.clone(); + if signal_a.0.index() < signal_b.0.index() { + assert_eq!(order, vec!["a", "b"]); + } else { + assert_eq!(order, vec!["b", "a"]); + } + } + + #[test] + fn piping_updates_levels_for_same_frame_execution() { + let mut world = World::new(); + world.insert_resource(SignalGraphState::default()); + world.insert_resource(Order::default()); + + let signal_a = spawn_signal::<(), (), Option<()>, _, _>(&mut world, |_: In<()>, mut order: ResMut| { + order.0.push("a"); + Some(()) + }); + let signal_b = spawn_signal::<(), (), Option<()>, _, _>(&mut world, |_: In<()>, mut order: ResMut| { + order.0.push("b"); + Some(()) + }); + + process_signal_graph(&mut world); + world.resource_mut::().0.clear(); + + pipe_signal(&mut world, signal_a, signal_b); + process_signal_graph(&mut world); + + let order = world.resource::().0.clone(); + assert_eq!(order, vec!["a", "b"]); + } + + #[test] + fn incremental_matches_full_rebuild_after_edge_change() { + let mut world = World::new(); + world.insert_resource(SignalGraphState::default()); + + let a = spawn_signal::<(), (), Option<()>, _, _>(&mut world, |_: In<()>| Some(())); + let b = spawn_signal::<(), (), Option<()>, _, _>(&mut world, |_: In<()>| Some(())); + let c = spawn_signal::<(), (), Option<()>, _, _>(&mut world, |_: In<()>| Some(())); + let d = spawn_signal::<(), (), Option<()>, _, _>(&mut world, |_: In<()>| Some(())); + + pipe_signal(&mut world, a, b); + pipe_signal(&mut world, b, c); + pipe_signal(&mut world, a, d); + + world.resource_scope(|world, mut state: Mut| { + rebuild_levels(world, &mut state); + }); + + pipe_signal(&mut world, c, d); + + let incremental_levels = world.resource_scope(|world, mut state: Mut| { + update_edge_change_levels(world, &mut state); + state.levels.clone() + }); + + let mut expected = SignalGraphState::default(); + rebuild_levels(&mut world, &mut expected); + + assert_eq!(incremental_levels, expected.levels); + } + + #[test] + fn cleanup_updates_downstream_levels() { + let mut world = World::new(); + world.insert_resource(SignalGraphState::default()); + + let a = spawn_signal::<(), (), Option<()>, _, _>(&mut world, |_: In<()>| Some(())); + let b = spawn_signal::<(), (), Option<()>, _, _>(&mut world, |_: In<()>| Some(())); + let c = spawn_signal::<(), (), Option<()>, _, _>(&mut world, |_: In<()>| Some(())); + + pipe_signal(&mut world, a, b); + pipe_signal(&mut world, b, c); + + world.resource_scope(|world, mut state: Mut| { + rebuild_levels(world, &mut state); + }); + + cleanup_recursive(&mut world, a); + + let levels_after = world.resource_scope(|world, mut state: Mut| { + update_edge_change_levels(world, &mut state); + state.levels.clone() + }); + + assert_eq!(levels_after.get(&b), Some(&0)); + assert_eq!(levels_after.get(&c), Some(&1)); + assert!(!levels_after.contains_key(&a)); + } +} diff --git a/src/lib.rs b/src/lib.rs index 827781e..d7496de 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -92,6 +92,7 @@ pub struct SignalProcessing; impl Plugin for JonmoPlugin { fn build(&self, app: &mut App) { + app.init_resource::(); app.add_systems( self.schedule, ( diff --git a/src/signal.rs b/src/signal.rs index a542e11..4578b60 100644 --- a/src/signal.rs +++ b/src/signal.rs @@ -352,11 +352,8 @@ where Left: Signal, Right: Signal, { - #[allow(clippy::type_complexity)] - left_wrapper: Map, Option)>, - #[allow(clippy::type_complexity)] - right_wrapper: Map, Option)>, signal: LazySignal, + _marker: PhantomData (Left, Right)>, } impl Clone for With @@ -366,9 +363,27 @@ where { fn clone(&self) -> Self { Self { - left_wrapper: self.left_wrapper.clone(), - right_wrapper: self.right_wrapper.clone(), signal: self.signal.clone(), + _marker: PhantomData, + } + } +} + +/// Internal cache for [`.with`](SignalExt::with) combinator. +/// +/// Each upstream updates its side. The gate signal runs once per frame and emits the latest +/// combined output once both sides have produced at least one value. +#[derive(Component)] +struct WithCache { + left: Option, + right: Option, +} + +impl Default for WithCache { + fn default() -> Self { + Self { + left: None, + right: None, } } } @@ -381,12 +396,7 @@ where type Item = (Left::Item, Right::Item); fn register_boxed_signal(self: Box, world: &mut World) -> SignalHandle { - let SignalHandle(left_upstream) = self.left_wrapper.register(world); - let SignalHandle(right_upstream) = self.right_wrapper.register(world); - let signal = self.signal.register(world); - pipe_signal(world, left_upstream, signal); - pipe_signal(world, right_upstream, signal); - signal.into() + self.signal.register(world).into() } } @@ -1645,7 +1655,7 @@ pub trait SignalExt: Signal { } } - /// Combines this [`Signal`] with another [`Signal`], outputting a tuple with both of their + /// Combines this [`Signal`] with another [`Signal`], outputting a tuple with both of their latest /// outputs. The resulting [`Signal`] will only output a value when both input [`Signal`]s have /// outputted a value. /// @@ -1663,33 +1673,52 @@ pub trait SignalExt: Signal { where Self: Sized, Other: Signal, - Self::Item: Clone + Send + 'static, - Other::Item: Clone + Send + 'static, + Self::Item: Clone + SSs, + Other::Item: Clone + SSs, { - let left_wrapper = self.map(|In(left): In| (Some(left), None::)); - let right_wrapper = other.map(|In(right): In| (None::, Some(right))); - let signal = lazy_signal_from_system::<_, (Self::Item, Other::Item), _, _, _>( - #[allow(clippy::type_complexity)] - move |In((left_option, right_option)): In<(Option, Option)>, - mut left_cache: Local>, - mut right_cache: Local>| { - if left_option.is_some() { - *left_cache = left_option; - } - if right_option.is_some() { - *right_cache = right_option; + let signal = LazySignal::new(move |world: &mut World| { + let with_entity = LazyEntity::new(); + + let left_wrapper = self.map(clone!((with_entity) move |In(left): In, world: &mut World| { + if let Some(mut cache) = world.get_mut::>(with_entity.get()) { + cache.left = Some(left); } - if left_cache.is_some() && right_cache.is_some() { - left_cache.clone().zip(right_cache.clone()) - } else { - None + })); + let right_wrapper = other.map(clone!((with_entity) move |In(right): In, world: &mut World| { + if let Some(mut cache) = world.get_mut::>(with_entity.get()) { + cache.right = Some(right); } - }, - ); + })); + + let with_signal = lazy_signal_from_system::<_, (Self::Item, Other::Item), _, _, _>( + clone!((with_entity) move |_: In<()>, world: &mut World| { + let Some(cache) = world.get_mut::>(with_entity.get()) else { + return None; + }; + match (&cache.left, &cache.right) { + (Some(left), Some(right)) => Some((left.clone(), right.clone())), + _ => None, + } + }), + ); + + let signal = with_signal.register(world); + if let Ok(mut entity) = world.get_entity_mut(*signal) { + entity.insert(WithCache::::default()); + } + with_entity.set(*signal); + + let SignalHandle(left_upstream) = left_wrapper.register(world); + let SignalHandle(right_upstream) = right_wrapper.register(world); + pipe_signal(world, left_upstream, signal); + pipe_signal(world, right_upstream, signal); + + signal + }); + With { - left_wrapper, - right_wrapper, signal, + _marker: PhantomData, } } @@ -3258,6 +3287,9 @@ mod tests { #[derive(Resource, Default, Debug)] struct SignalOutput(Option); + #[derive(Resource, Default, Debug)] + struct SignalOutputVec(Vec); + fn create_test_app() -> App { let mut app = App::new(); app.add_plugins((MinimalPlugins, JonmoPlugin::default())); @@ -3278,6 +3310,17 @@ mod tests { world.resource::>().0.clone() } + fn capture_output_vec(In(value): In, mut output: ResMut>) { + output.0.push(value); + } + + fn get_and_clear_output_vec(world: &mut World) -> Vec { + world + .get_resource_mut::>() + .map(|mut res| core::mem::take(&mut res.0)) + .unwrap_or_default() + } + #[test] fn test_map() { let mut app = create_test_app(); @@ -3721,6 +3764,89 @@ mod tests { signal.cleanup(app.world_mut()); } + #[test] + fn test_with_emits_after_both_emit() { + let mut app = create_test_app(); + app.init_resource::>(); + + let left = SignalBuilder::from_system(|_: In<()>, mut state: Local| { + *state += 1; + *state + }); + let right = SignalBuilder::from_system(|_: In<()>| 10).first(); + + let signal = left + .with(right) + .map(capture_output_vec) + .register(app.world_mut()); + + app.update(); + assert_eq!( + get_and_clear_output_vec::<(i32, i32)>(app.world_mut()), + vec![(1, 10), (1, 10)] + ); + + app.update(); + assert_eq!( + get_and_clear_output_vec::<(i32, i32)>(app.world_mut()), + vec![(2, 10)] + ); + + app.update(); + assert_eq!( + get_and_clear_output_vec::<(i32, i32)>(app.world_mut()), + vec![(3, 10)] + ); + + signal.cleanup(app.world_mut()); + } + + #[test] + fn test_chain_multi_level_same_frame() { + let mut app = create_test_app(); + app.init_resource::>(); + + let signal = SignalBuilder::from_system(|_: In<()>, mut state: Local| { + *state += 1; + *state + }) + .map_in(|x: i32| x + 1) + .map_in(|x: i32| x * 2) + .map(capture_output) + .register(app.world_mut()); + + app.update(); + assert_eq!(get_output::(app.world()), Some(4)); + signal.cleanup(app.world_mut()); + } + + #[test] + fn test_fan_out_fan_in_with() { + let mut app = create_test_app(); + app.init_resource::>(); + + let source = SignalBuilder::from_system(|_: In<()>, mut state: Local| { + *state += 1; + *state + }); + + let left = source.clone().map_in(|x: i32| x + 1); + let right = source.map_in(|x: i32| x + 10); + + let signal = left + .with(right) + .map(capture_output) + .register(app.world_mut()); + + app.update(); + assert_eq!(get_output::<(i32, i32)>(app.world()), Some((2, 11))); + + app.update(); + assert_eq!(get_output::<(i32, i32)>(app.world()), Some((3, 12))); + + signal.cleanup(app.world_mut()); + } + #[test] fn test_zip_macro() { use crate::signal::zip; From 2f0ed351d5579aeb5f04773d117777d7612feb13 Mon Sep 17 00:00:00 2001 From: databasedav <31483365+databasedav@users.noreply.github.com> Date: Sun, 18 Jan 2026 20:33:30 -0800 Subject: [PATCH 10/15] check --- CHANGELOG.md | 2 +- src/builder.rs | 96 +++++++++++++++++++++++++------------------------- src/graph.rs | 5 +-- src/lib.rs | 2 +- src/signal.rs | 48 ++++++++++++------------- 5 files changed, 77 insertions(+), 76 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ba784e..eddd507 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ the format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) ### changed -- `JonmoBuilder::hold_signals` takes `Box`s instead of `SignalHandles` +- `JonmoBuilder::hold_tasks` renamed to `JonmoBuilder::hold_signals` and takes `Box`s instead of `SignalHandles` - removed output `Clone` bound from `SignalExt::filter_map` ### added diff --git a/src/builder.rs b/src/builder.rs index 184d142..7d75009 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -119,10 +119,10 @@ impl JonmoBuilder { }) } - /// Attach [`Holdable`] signals to this entity for automatic cleanup on despawn. + /// Attach [`SignalTask`]s to this builder for automatic cleanup on despawn. /// - /// Use [`.hold()`](SignalHoldExt::hold) to convert a [`Signal`], [`SignalVec`], or - /// [`SignalMap`] into a [`Holdable`]. + /// Use [`.task()`](SignalTaskExt::task) to convert a [`Signal`], [`SignalVec`], or + /// [`SignalMap`] into a [`SignalTask`]. /// /// # Example /// @@ -135,22 +135,22 @@ impl JonmoBuilder { /// value: i32, /// } /// - /// let my_signal = SignalBuilder::from_resource::() + /// let my_signal_task = SignalBuilder::from_resource::() /// .map_in(|r: MyResource| println!("{}", r.value)) - /// .hold(); + /// .task(); /// /// let mut world = World::new(); /// let my_mutable_vec = MutableVecBuilder::from([1, 2, 3]).spawn(&mut world); - /// let my_signal_vec = my_mutable_vec + /// let my_signal_vec_task = my_mutable_vec /// .signal_vec() /// .map_in(|item: i32| println!("{}", item)) - /// .hold(); + /// .task(); /// - /// JonmoBuilder::new().hold_signals([my_signal, my_signal_vec]); + /// JonmoBuilder::new().hold_tasks([my_signal_task, my_signal_vec_task]); /// ``` - pub fn hold_signals(self, holdables: impl IntoIterator> + SSs) -> Self { + pub fn hold_tasks(self, tasks: impl IntoIterator> + SSs) -> Self { self.on_spawn(move |world, entity| { - let handles: Vec<_> = holdables.into_iter().map(|h| h.register_holdable(world)).collect(); + let handles: Vec<_> = tasks.into_iter().map(|task| task.register_task(world)).collect(); add_handles(world, entity, handles); }) } @@ -194,7 +194,7 @@ impl JonmoBuilder { } /// When this builder's [`Entity`] is despawned, run a function with mutable access to the - /// [`DeferredWorld`] and this entity's [`Entity`]. + /// [`DeferredWorld`] and this builder's [`Entity`]. pub fn on_despawn(self, on_despawn: impl FnOnce(&mut DeferredWorld, Entity) + Send + Sync + 'static) -> Self { self.on_spawn(|world, entity| { if let Ok(mut entity_mut) = world.get_entity_mut(entity) { @@ -787,74 +787,74 @@ impl JonmoBuilder { } } -/// A type-erased signal that can be registered with a [`World`] at spawn time. +/// A type-erased signal that can be managed as a free-floating "task". /// -/// This trait enables [`JonmoBuilder::hold_signals`] to accept signals that haven't +/// This trait enables [`JonmoBuilder::hold_tasks`] to accept signals that haven't /// been registered yet, deferring their registration until the entity is spawned. /// -/// Use the [`.hold()`](SignalHoldExt::hold) extension method to convert a [`Signal`], -/// [`SignalVec`], or [`SignalMap`] into a `Box`. -pub trait Holdable: Send + Sync + 'static { - /// Register this signal with the world and return its handle. - fn register_holdable(self: Box, world: &mut World) -> SignalHandle; +/// Use the [`.task()`](SignalTaskExt::task) method to convert a [`Signal`], +/// [`SignalVec`], or [`SignalMap`] into a [`SignalTask`]. +pub trait SignalTask: Send + Sync + 'static { + /// Register this signal task with the world and return its handle. + fn register_task(self: Box, world: &mut World) -> SignalHandle; } -struct HoldableSignal(S); +struct SignalTaskSignal(S); -impl Holdable for HoldableSignal { - fn register_holdable(self: Box, world: &mut World) -> SignalHandle { +impl SignalTask for SignalTaskSignal { + fn register_task(self: Box, world: &mut World) -> SignalHandle { self.0.register(world) } } -/// Extension trait for converting a [`Signal`] into a [`Holdable`]. -pub trait SignalHoldExt: Signal + Sized + SSs { - /// Convert this signal into a type-erased [`Holdable`] for use with - /// [`JonmoBuilder::hold_signals`]. - fn hold(self) -> Box { - Box::new(HoldableSignal(self)) +/// Extension trait for converting a [`Signal`] into a [`SignalTask`]. +pub trait SignalTaskExt: Signal + Sized + SSs { + /// Convert this signal into a type-erased [`SignalTask`] for use with + /// [`JonmoBuilder::hold_tasks`]. + fn task(self) -> Box { + Box::new(SignalTaskSignal(self)) } } -impl SignalHoldExt for S {} +impl SignalTaskExt for S {} -struct HoldableSignalVec(S); +struct SignalTaskSignalVec(S); -impl Holdable for HoldableSignalVec { - fn register_holdable(self: Box, world: &mut World) -> SignalHandle { +impl SignalTask for SignalTaskSignalVec { + fn register_task(self: Box, world: &mut World) -> SignalHandle { self.0.register(world) } } -/// Extension trait for converting a [`SignalVec`] into a [`Holdable`]. -pub trait SignalVecHoldExt: SignalVec + Sized + SSs { - /// Convert this signal vec into a type-erased [`Holdable`] for use with - /// [`JonmoBuilder::hold_signals`]. - fn hold(self) -> Box { - Box::new(HoldableSignalVec(self)) +/// Extension trait for converting a [`SignalVec`] into a [`SignalTask`]. +pub trait SignalVecTaskExt: SignalVec + Sized + SSs { + /// Convert this signal vec into a type-erased [`SignalTask`] for use with + /// [`JonmoBuilder::hold_tasks`]. + fn task(self) -> Box { + Box::new(SignalTaskSignalVec(self)) } } -impl SignalVecHoldExt for S {} +impl SignalVecTaskExt for S {} -struct HoldableSignalMap(S); +struct SignalTaskSignalMap(S); -impl Holdable for HoldableSignalMap { - fn register_holdable(self: Box, world: &mut World) -> SignalHandle { +impl SignalTask for SignalTaskSignalMap { + fn register_task(self: Box, world: &mut World) -> SignalHandle { self.0.register(world) } } -/// Extension trait for converting a [`SignalMap`] into a [`Holdable`]. -pub trait SignalMapHoldExt: SignalMap + Sized + SSs { - /// Convert this signal map into a type-erased [`Holdable`] for use with - /// [`JonmoBuilder::hold_signals`]. - fn hold(self) -> Box { - Box::new(HoldableSignalMap(self)) +/// Extension trait for converting a [`SignalMap`] into a [`SignalTask`]. +pub trait SignalMapTaskExt: SignalMap + Sized + SSs { + /// Convert this signal map into a type-erased [`SignalTask`] for use with + /// [`JonmoBuilder::hold_tasks`]. + fn task(self) -> Box { + Box::new(SignalTaskSignalMap(self)) } } -impl SignalMapHoldExt for S {} +impl SignalMapTaskExt for S {} fn on_despawn_hook(mut world: DeferredWorld, ctx: HookContext) { let entity = ctx.entity; diff --git a/src/graph.rs b/src/graph.rs index fc4b733..7bced66 100644 --- a/src/graph.rs +++ b/src/graph.rs @@ -523,9 +523,9 @@ fn remove_signal_from_graph_state(world: &mut World, signal: SignalSystem) { if let Some(mut state) = world.get_resource_mut::() { if state.is_processing { state.deferred_removals.insert(signal); - return; + } else { + remove_signal_from_graph_state_internal(&mut state, signal); } - remove_signal_from_graph_state_internal(&mut state, signal); } } @@ -651,6 +651,7 @@ pub(crate) fn process_signal_graph(world: &mut World) { let mut state = world.resource_mut::(); state.is_processing = false; state.by_level = levels_snapshot; + // this indirection allows us to avoid cloning the topological ordering every frame apply_deferred_removals(&mut state); } diff --git a/src/lib.rs b/src/lib.rs index d7496de..72621ab 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -116,7 +116,7 @@ impl Plugin for JonmoPlugin { /// `use jonmo::prelude::*;` imports everything one needs to use start using [jonmo](crate). pub mod prelude { #[cfg(feature = "builder")] - pub use crate::builder::{Holdable, JonmoBuilder, SignalHoldExt, SignalMapHoldExt, SignalVecHoldExt}; + pub use crate::builder::{JonmoBuilder, SignalMapTaskExt, SignalTask, SignalTaskExt, SignalVecTaskExt}; pub use crate::{ JonmoPlugin, graph::SignalHandles, diff --git a/src/signal.rs b/src/signal.rs index 4578b60..5c639d7 100644 --- a/src/signal.rs +++ b/src/signal.rs @@ -2296,8 +2296,8 @@ pub trait SignalExt: Signal { } } - /// If this [`Signal`] outputs [`true`], output the result of some [`System`], otherwise - /// terminate for the frame. + /// If this [`Signal`] outputs [`true`], output the [`System`] result wrapped in [`Some`], otherwise + /// output [`None`]. /// /// # Example /// @@ -2311,7 +2311,7 @@ pub trait SignalExt: Signal { /// *state /// } /// }) - /// .map_true(|_: In<()>| 1); // outputs `1`, terminates, outputs `1`, terminates, ... + /// .map_true(|_: In<()>| 1); // outputs `Some(1)`, `None`, `Some(1)`, `None`, ... /// ``` fn map_true(self, system: F) -> MapTrue where @@ -2342,8 +2342,8 @@ pub trait SignalExt: Signal { } } - /// If this [`Signal`] outputs [`true`], output the result of some [`FnMut`], otherwise - /// terminate for the frame. + /// If this [`Signal`] outputs [`true`], output the [`FnMut`] result wrapped in [`Some`], otherwise + /// output [`None`]. /// /// # Example /// @@ -2357,7 +2357,7 @@ pub trait SignalExt: Signal { /// *state /// } /// }) - /// .map_true_in(|| 1); // outputs `1`, terminates, outputs `1`, terminates, ... + /// .map_true_in(|| 1); // outputs `Some(1)`, `None`, `Some(1)`, `None`, ... /// ``` fn map_true_in(self, mut function: F) -> MapTrue where @@ -2382,8 +2382,8 @@ pub trait SignalExt: Signal { } } - /// If this [`Signal`] outputs [`false`], output the result of some [`System`], otherwise - /// terminate for the frame. + /// If this [`Signal`] outputs [`false`], output the [`System`] result wrapped in [`Some`], otherwise + /// output [`None`]. /// /// # Example /// @@ -2397,7 +2397,7 @@ pub trait SignalExt: Signal { /// *state /// } /// }) - /// .map_false(|_: In<()>| 1); // terminates, outputs `1`, terminates, outputs `1`, ... + /// .map_false(|_: In<()>| 1); // outputs `None`, `Some(1)`, `None`, `Some(1)`, ... /// ``` fn map_false(self, system: F) -> MapFalse where @@ -2428,8 +2428,8 @@ pub trait SignalExt: Signal { } } - /// If this [`Signal`] outputs [`false`], output the result of some [`FnMut`], otherwise - /// terminate for the frame. + /// If this [`Signal`] outputs [`false`], output the [`FnMut`] result wrapped in [`Some`], otherwise + /// output [`None`]. /// /// # Example /// @@ -2443,7 +2443,7 @@ pub trait SignalExt: Signal { /// *state /// } /// }) - /// .map_false_in(|| 1); // terminates, outputs `1`, terminates, outputs `1`, ... + /// .map_false_in(|| 1); // outputs `None`, `Some(1)`, `None`, `Some(1)`, ... /// ``` fn map_false_in(self, mut function: F) -> MapFalse where @@ -2561,8 +2561,8 @@ pub trait SignalExt: Signal { } } - /// If this [`Signal`] outputs [`Some`], output the result of some [`System`] which takes [`In`] - /// the [`Some`] value, otherwise terminate for the frame. + /// If this [`Signal`] outputs [`Some`], output the [`System`] (which takes [`In`] the [`Some`] + /// value) result wrapped in [`Some`], otherwise output [`None`]. /// /// # Example /// @@ -2578,7 +2578,7 @@ pub trait SignalExt: Signal { /// }) /// .map_some( /// |In(state): In| state - /// ); // outputs `true`, terminates, outputs `true`, terminates, ... + /// ); // outputs `Some(true)`, `None`, `Some(true)`, `None`, ... /// ``` fn map_some(self, system: F) -> MapSome where @@ -2607,8 +2607,8 @@ pub trait SignalExt: Signal { } } - /// If this [`Signal`] outputs [`Some`], output the result of some [`FnMut`] which takes [`In`] - /// the [`Some`] value, otherwise terminate for the frame. + /// If this [`Signal`] outputs [`Some`], output the [`FnMut`] (which takes [`In`] the [`Some`] + /// value) result wrapped in [`Some`], otherwise output [`None`]. /// /// # Example /// @@ -2622,7 +2622,7 @@ pub trait SignalExt: Signal { /// *state /// } /// }) - /// .map_some_in(|state: bool| state); // outputs `true`, terminates, outputs `true`, terminates, ... + /// .map_some_in(|state: bool| state); // outputs `Some(true)`, `None`, `Some(true)`, `None`, ... /// ``` fn map_some_in(self, mut function: F) -> MapSome where @@ -2647,8 +2647,8 @@ pub trait SignalExt: Signal { } } - /// If this [`Signal`] outputs [`None`], output the result of some [`System`], otherwise - /// terminate for the frame. + /// If this [`Signal`] outputs [`None`], output the [`System`] result wrapped in [`Some`], otherwise + /// output [`None`]. /// /// # Example /// @@ -2662,7 +2662,7 @@ pub trait SignalExt: Signal { /// *state /// } /// }) - /// .map_none(|_: In<()>| false); // terminates, outputs `false`, terminates, outputs `false`, ... + /// .map_none(|_: In<()>| false); // outputs `None`, `Some(false)`, `None`, `Some(false)`, ... /// ``` fn map_none(self, none_system: F) -> MapNone where @@ -2691,8 +2691,8 @@ pub trait SignalExt: Signal { } } - /// If this [`Signal`] outputs [`None`], output the result of some [`FnMut`], otherwise - /// terminate for the frame. + /// If this [`Signal`] outputs [`None`], output the [`FnMut`] result wrapped in [`Some`], otherwise + /// output [`None`]. /// /// # Example /// @@ -2706,7 +2706,7 @@ pub trait SignalExt: Signal { /// *state /// } /// }) - /// .map_none_in(|| false); // terminates, outputs `false`, terminates, outputs `false`, ... + /// .map_none_in(|| false); // outputs `None`, `Some(false)`, `None`, `Some(false)`, ... /// ``` fn map_none_in(self, mut function: F) -> MapNone where From fb62d25ffa86ccca8e1eee0d28a061fd4b17e77a Mon Sep 17 00:00:00 2001 From: databasedav <31483365+databasedav@users.noreply.github.com> Date: Sat, 24 Jan 2026 14:46:16 -0800 Subject: [PATCH 11/15] check --- CHANGELOG.md | 12 +- Cargo.toml | 2 +- README.md | 25 +- examples/basic.rs | 3 +- examples/basic_builder.rs | 20 +- examples/basic_builder_inject.rs | 17 +- examples/counter.rs | 59 +- examples/filters.rs | 140 +- examples/letters.rs | 57 +- examples/lifetimes.rs | 40 +- examples/test.rs | 28 +- src/builder.rs | 2444 +++++------------------------- src/graph.rs | 167 +- src/lib.rs | 21 +- src/signal.rs | 1113 ++++++++------ src/signal_map.rs | 230 ++- src/signal_vec.rs | 619 ++++---- 17 files changed, 1829 insertions(+), 3168 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eddd507..9197bd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,13 +6,19 @@ the format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) ### changed -- `JonmoBuilder::hold_tasks` renamed to `JonmoBuilder::hold_signals` and takes `Box`s instead of `SignalHandles` -- removed output `Clone` bound from `SignalExt::filter_map` +- signal graph processed in deterministic topological order instead of the previous nondeterministic depth first order +- renamed `SignalExt::combine` to `SignalExt::with` +- `JonmoBuilder` renamed to `jonmo::Builder` +- `jonmo::Builder::hold_signals` renamed to `jonmo::Builder::hold_tasks` and takes `Box`s instead of `SignalHandles` +- removed self item `Clone` bound from `SignalVecExt::map_signal` and `SignalVecExt::filter_map` +- `SignalBuilder::*` methods moved to free functions in `signal` module (e.g. `SignalBuilder::always` -> `signal::always`) ### added - `SignalExt::take` +- `signal::zip!`, a variadic flattened version of `SignalExt::with` +- `SignalVecExt::flatten` - `track_caller` derive for panicking `LazyEntity` methods -- warning that cloning `JonmoBuilder`s at runtime is a bug +- warning that cloning `jonmo::Builder`s at runtime is a bug ### fixed diff --git a/Cargo.toml b/Cargo.toml index 691cb44..8cd0c6f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,7 +26,7 @@ document-features = { version = "0.2", optional = true } [features] default = ["builder", "tracing", "time", "std"] -## Enables access to jonmo's high-level entity builder, [`JonmoBuilder`](https://docs.rs/jonmo/latest/jonmo/builder/struct.JonmoBuilder.html). +## Enables access to jonmo's high-level entity builder, [`jonmo::Builder`](https://docs.rs/jonmo/latest/jonmo/builder/struct.Builder.html). builder = [] ## Enables access to signal ext `.debug` methods, which conveniently logs signal outputs at any step. diff --git a/README.md b/README.md index 13291e8..0ed0806 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ in bengali, jonmo means "birth" ``` -[jonmo](https://github.com/databasedav/jonmo) provides an ergonomic, functional, and declarative API for specifying Bevy [system](https://docs.rs/bevy/latest/bevy/ecs/system/index.html) dependency graphs, where "output" handles to nodes of the graph are canonically referred to as "signals". Building upon these signals, jonmo offers a high level [entity builder](https://docs.rs/jonmo/latest/jonmo/builder/struct.JonmoBuilder.html) which enables one to declare reactive entities, components, and children using a familiar fluent syntax with semantics and API ported from the incredible [FRP](https://en.wikipedia.org/wiki/Functional_reactive_programming) signals of [futures-signals](https://github.com/Pauan/rust-signals) and its web UI dependents [MoonZoon](https://github.com/MoonZoon/MoonZoon) and [Dominator](https://github.com/Pauan/rust-dominator). +[jonmo](https://github.com/databasedav/jonmo) provides an ergonomic, functional, and declarative API for specifying Bevy [system](https://docs.rs/bevy/latest/bevy/ecs/system/index.html) dependency graphs, where "output" handles to nodes of the graph are canonically referred to as "signals". Building upon these signals, jonmo offers a high level [entity builder](https://docs.rs/jonmo/latest/jonmo/builder/struct.Builder.html) which enables one to declare reactive entities, components, and children using a familiar fluent syntax with semantics and API ported from the incredible [FRP](https://en.wikipedia.org/wiki/Functional_reactive_programming) signals of [futures-signals](https://github.com/Pauan/rust-signals) and its web UI dependents [MoonZoon](https://github.com/MoonZoon/MoonZoon) and [Dominator](https://github.com/Pauan/rust-dominator). The runtime of jonmo is quite simple; every frame, the outputs of systems are forwarded to their dependants, recursively. The complexity and power of jonmo really emerges from its monadic signal combinators, defined within the [`SignalExt`](https://docs.rs/jonmo/latest/jonmo/signal/trait.SignalExt.html), [`SignalVecExt`](https://docs.rs/jonmo/latest/jonmo/signal_vec/trait.SignalVecExt.html), and [`SignalMapExt`](https://docs.rs/jonmo/latest/jonmo/signal_map/trait.SignalMapExt.html) traits (ported from futures-signals' traits of the same name), which internally manage special Bevy systems that allow for the declarative composition of complex data flows with minimalistic, high-level, signals-oriented methods. @@ -51,10 +51,10 @@ fn main() { #[derive(Component, Clone, Deref, DerefMut)] struct Counter(i32); -fn ui_root() -> JonmoBuilder { +fn ui_root() -> jonmo::Builder { let counter_holder = LazyEntity::new(); - JonmoBuilder::from(Node { + jonmo::Builder::from(Node { width: Val::Percent(100.0), height: Val::Percent(100.0), justify_content: JustifyContent::Center, @@ -62,7 +62,7 @@ fn ui_root() -> JonmoBuilder { ..default() }) .child( - JonmoBuilder::from(Node { + jonmo::Builder::from(Node { flex_direction: FlexDirection::Row, column_gap: Val::Px(15.0), align_items: AlignItems::Center, @@ -73,11 +73,10 @@ fn ui_root() -> JonmoBuilder { .lazy_entity(counter_holder.clone()) .child(counter_button(counter_holder.clone(), PINK, "-", -1)) .child( - JonmoBuilder::from((Node::default(), TextFont::from_font_size(25.))) + jonmo::Builder::from((Node::default(), TextFont::from_font_size(25.))) .component_signal( - SignalBuilder::from_component_lazy(counter_holder.clone()) - .map_in(|counter: Counter| *counter) - .dedupe() + signal::from_component_changed_lazy::(counter_holder.clone()) + .map_in(deref_copied) .map_in_ref(ToString::to_string) .map_in(Text) .map_in(Some), @@ -87,8 +86,8 @@ fn ui_root() -> JonmoBuilder { ) } -fn counter_button(counter_holder: LazyEntity, color: Color, label: &'static str, step: i32) -> JonmoBuilder { - JonmoBuilder::from(( +fn counter_button(counter_holder: LazyEntity, color: Color, label: &'static str, step: i32) -> jonmo::Builder { + jonmo::Builder::from(( Node { width: Val::Px(45.0), justify_content: JustifyContent::Center, @@ -99,11 +98,9 @@ fn counter_button(counter_holder: LazyEntity, color: Color, label: &'static str, BackgroundColor(color), )) .observe(move |_: On>, mut counters: Query<&mut Counter>| { - if let Ok(mut counter) = counters.get_mut(*counter_holder) { - **counter += step; - } + **counters.get_mut(*counter_holder).unwrap() += step; }) - .child(JonmoBuilder::from((Text::from(label), TextFont::from_font_size(25.)))) + .child(jonmo::Builder::from((Text::from(label), TextFont::from_font_size(25.)))) } fn camera(mut commands: Commands) { diff --git a/examples/basic.rs b/examples/basic.rs index 8fc732a..ede6dee 100644 --- a/examples/basic.rs +++ b/examples/basic.rs @@ -26,8 +26,7 @@ fn ui(world: &mut World) { let text = world .spawn((Node::default(), TextFont::from_font_size(100.), Value(0))) .id(); - let signal = SignalBuilder::from_component(text) - .dedupe() + let signal = signal::from_component_changed::(text) .map(move |In(value): In, mut commands: Commands| { commands.entity(text).insert(Text(value.0.to_string())); }) diff --git a/examples/basic_builder.rs b/examples/basic_builder.rs index 798b8cd..f14e903 100644 --- a/examples/basic_builder.rs +++ b/examples/basic_builder.rs @@ -29,23 +29,25 @@ struct ValueTicker(Timer); #[derive(Component, Clone, Default, PartialEq)] struct Value(i32); -fn ui() -> JonmoBuilder { - JonmoBuilder::from(Node { +fn ui() -> jonmo::Builder { + let lazy_entity = LazyEntity::new(); + jonmo::Builder::from(Node { justify_content: JustifyContent::Center, align_items: AlignItems::Center, height: Val::Percent(100.), width: Val::Percent(100.), ..default() }) + .lazy_entity(lazy_entity.clone()) .child( - JonmoBuilder::from((Node::default(), TextFont::from_font_size(100.))) + jonmo::Builder::from((Node::default(), TextFont::from_font_size(100.))) .insert(Value(0)) - .component_signal_from_component(|signal| { - signal - .dedupe() - .map(|In(value): In| Text(value.0.to_string())) - .map_in(Some) - }), + .component_signal( + signal::from_component_changed_lazy::(lazy_entity) + .map_in(|value| value.0.to_string()) + .map_in(Text) + .map_in(Some), + ), ) } diff --git a/examples/basic_builder_inject.rs b/examples/basic_builder_inject.rs index 7a03f2c..e7abf02 100644 --- a/examples/basic_builder_inject.rs +++ b/examples/basic_builder_inject.rs @@ -34,14 +34,15 @@ fn ui(world: &mut World) { }); ui_root.add_child(text); - JonmoBuilder::new() - .component_signal_from_component(|signal| { - signal - .dedupe() - .map(|In(value): In| Text(value.0.to_string())) - .map_in(Some) - }) - .spawn_on_entity(world, text); + jonmo::Builder::new() + .component_signal( + signal::from_component_changed::(text) + .map_in(|value| value.0.to_string()) + .map_in(Text) + .map_in(Some), + ) + .spawn_on_entity(world, text) + .unwrap(); } fn incr_value(mut ticker: ResMut, time: Res