Skip to content

Commit 8aec5ef

Browse files
committed
check
1 parent ab3ec47 commit 8aec5ef

File tree

9 files changed

+89
-41
lines changed

9 files changed

+89
-41
lines changed

Cargo.lock

Lines changed: 22 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ deployed_wasm_example = []
6565
[dev-dependencies]
6666
bevy = "0.16"
6767
rand = "0.9"
68+
test-log = { version = "0.2", default-features = false, features = ["trace"] }
6869

6970
[target.'cfg(target_arch = "wasm32")'.dev-dependencies.getrandom]
7071
version = "0.3"

README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ fn ui_root() -> JonmoBuilder {
8888
flex_direction: FlexDirection::Row,
8989
column_gap: Val::Px(15.0),
9090
align_items: AlignItems::Center,
91+
padding: UiRect::all(Val::Px(25.)),
9192
..default()
9293
})
9394
.insert(Counter(0))
@@ -117,7 +118,7 @@ fn ui_root() -> JonmoBuilder {
117118
// `.dedupe()` ensures the rest of the chain only runs when the counter's value *actually
118119
// changes*, preventing redundant updates every frame.
119120
.dedupe()
120-
// `Text` expects a `String`
121+
// `Text` expects a `String` and `.to_string` expects a reference
121122
.map_in_ref(ToString::to_string)
122123
.map_in(Text)
123124
// `component_signal` expects an `Option<Component>`. If the signal produces `None`, the

examples/counter.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ fn ui_root() -> JonmoBuilder {
9191
// `.dedupe()` ensures the rest of the chain only runs when the counter's value *actually
9292
// changes*, preventing redundant updates every frame.
9393
.dedupe()
94-
// `Text` expects a `String`
94+
// `Text` expects a `String` and `.to_string` expects a reference
9595
.map_in_ref(ToString::to_string)
9696
.map_in(Text)
9797
// `component_signal` expects an `Option<Component>`. If the signal produces `None`, the

src/graph.rs

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ impl Clone for LazySignal {
399399
impl Drop for LazySignal {
400400
fn drop(&mut self) {
401401
// <= 2 because we also wna queue if only the holder remains
402-
if self.inner.references.fetch_sub(1, Ordering::SeqCst) == 1
402+
if self.inner.references.fetch_sub(1, Ordering::SeqCst) <= 2
403403
&& let LazySystem::Registered(signal) = *self.inner.system.read().unwrap()
404404
{
405405
CLEANUP_SIGNALS.lock().unwrap().push(signal);
@@ -510,52 +510,54 @@ where
510510
}
511511

512512
fn cleanup_recursive(world: &mut World, signal: SignalSystem) {
513-
// Stage 1: Decrement the count and check if this node needs full cleanup.
513+
// Stage 1: Decrement the count and check if this node needs full cleanup. We do
514+
// this in a short-lived block to release the borrow.
514515
let mut needs_full_cleanup = false;
515516
if let Ok(mut entity) = world.get_entity_mut(*signal)
516-
&& let Some(mut count) = entity.get_mut::<SignalRegistrationCount>() {
517-
count.decrement();
518-
if **count == 0 {
519-
needs_full_cleanup = true;
520-
}
517+
&& let Some(mut count) = entity.get_mut::<SignalRegistrationCount>()
518+
{
519+
count.decrement();
520+
if **count == 0 {
521+
needs_full_cleanup = true;
521522
}
523+
}
522524

525+
// If the count is not zero, we're done with this branch of the graph.
523526
if !needs_full_cleanup {
524527
return;
525528
}
526529

527-
// Stage 2: The count is zero. Get upstreams before we potentially despawn the entity.
530+
// Stage 2: The count is zero. Perform the full cleanup. First, get the list of
531+
// parents.
528532
let upstreams = world.get::<Upstream>(*signal).cloned();
529533

530-
// Stage 3: Despawn the current node if it's no longer needed.
534+
// Now, we can despawn the current node if it's no longer needed. The check for
535+
// LazySignalHolder is crucial.
531536
if let Ok(entity) = world.get_entity_mut(*signal) {
532-
let should_despawn = if let Some(lazy_holder) = entity.get::<LazySignalHolder>() {
533-
// This signal is held by a LazySignal. It can only be despawned if the
534-
// only remaining reference is the one inside this holder component.
535-
lazy_holder.0.inner.references.load(Ordering::SeqCst) == 1
537+
if let Some(lazy_holder) = entity.get::<LazySignalHolder>() {
538+
if lazy_holder.0.inner.references.load(Ordering::SeqCst) == 1 {
539+
entity.despawn();
540+
}
536541
} else {
537-
// This is a dynamically created signal without an external holder.
538-
// It can be despawned now that its graph registrations are gone.
539-
true
540-
};
541-
542-
if should_despawn {
542+
// This is a dynamically created signal without a holder, it can be despawned once
543+
// its registrations are gone.
543544
entity.despawn();
544545
}
545546
}
546547

547-
// Stage 4: Recurse on parents. This happens _after_ the current node has
548+
// Stage 3: Notify parents and recurse. This happens _after_ the current node has
548549
// been processed and potentially despawned.
549550
if let Some(upstreams) = upstreams {
550551
for &upstream_system in upstreams.iter() {
551552
// Notify the parent to remove this signal from its downstream list.
552553
if let Ok(mut upstream_entity) = world.get_entity_mut(*upstream_system)
553-
&& let Some(mut downstream) = upstream_entity.get_mut::<Downstream>() {
554-
downstream.0.remove(&signal);
555-
if downstream.0.is_empty() {
556-
upstream_entity.remove::<Downstream>();
557-
}
554+
&& let Some(mut downstream) = upstream_entity.get_mut::<Downstream>()
555+
{
556+
downstream.0.remove(&signal);
557+
if downstream.0.is_empty() {
558+
upstream_entity.remove::<Downstream>();
558559
}
560+
}
559561

560562
// Now, recurse on the parent.
561563
cleanup_recursive(world, upstream_system);

src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
feature = "document-features",
55
doc = document_features::document_features!()
66
)]
7-
#![no_std]
7+
#![cfg_attr(not(test), no_std)]
88

99
extern crate alloc;
1010

src/signal.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,13 @@ impl<U: 'static> Signal for Box<dyn Signal<Item = U> + Send + Sync> {
5858
///
5959
/// Relevant in contexts where some function may require a [`Clone`] [`Signal`], but the concrete
6060
/// type can't be known at compile-time.
61-
pub trait SignalClone: Signal + DynClone {}
61+
pub trait SignalDynClone: Signal + DynClone {}
6262

63-
clone_trait_object!(< T > SignalClone < Item = T >);
63+
clone_trait_object!(<T> SignalDynClone <Item = T>);
6464

65-
impl<T: Signal + Clone + 'static> SignalClone for T {}
65+
impl<T: Signal + Clone + 'static> SignalDynClone for T {}
6666

67-
impl<O: 'static> Signal for Box<dyn SignalClone<Item = O> + Send + Sync> {
67+
impl<O: 'static> Signal for Box<dyn SignalDynClone<Item = O> + Send + Sync> {
6868
type Item = O;
6969

7070
fn register_boxed_signal(self: Box<Self>, world: &mut World) -> SignalHandle {
@@ -1947,7 +1947,7 @@ pub trait SignalExt: Signal {
19471947
/// SignalBuilder::from_system(|_: In<()>| 1).dedupe().boxed_clone() // this is a `Dedupe<Source<i32>>`
19481948
/// }; // without the `.boxed_clone()`, the compiler would not allow this
19491949
/// ```
1950-
fn boxed_clone(self) -> Box<dyn SignalClone<Item = Self::Item>>
1950+
fn boxed_clone(self) -> Box<dyn SignalDynClone<Item = Self::Item>>
19511951
where
19521952
Self: Sized + Clone,
19531953
{

src/signal_map.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -948,7 +948,7 @@ impl<K, V> MutableBTreeMap<K, V> {
948948
clone!(
949949
(self_entity) move | In(upstream_diffs): In<Vec<MapDiff<K, V>>>,
950950
world: & mut World,
951-
mut has_run: Local < bool >| {
951+
mut has_run: Local <bool>| {
952952
if !*has_run {
953953
// First run: This is triggered manually by the `MapReplayTrigger`.
954954
// It processes the initial state queued on its own entity and ignores upstream.

src/signal_vec.rs

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -157,13 +157,13 @@ impl<O: 'static> SignalVec for Box<dyn SignalVec<Item = O> + Send + Sync> {
157157
/// Relevant in contexts where some function may require a [`Clone`] [`SignalVec`], but the concrete
158158
/// type can't be known at compile-time, e.g. in a
159159
/// [`.switch_signal_vec`](SignalExt::switch_signal_vec).
160-
pub trait SignalVecClone: SignalVec + DynClone {}
160+
pub trait SignalVecDynClone: SignalVec + DynClone {}
161161

162-
clone_trait_object!(< T > SignalVecClone < Item = T >);
162+
clone_trait_object!(<T> SignalVecDynClone <Item = T>);
163163

164-
impl<T: SignalVec + Clone + 'static> SignalVecClone for T {}
164+
impl<T: SignalVec + Clone + 'static> SignalVecDynClone for T {}
165165

166-
impl<O: 'static> SignalVec for Box<dyn SignalVecClone<Item = O> + Send + Sync> {
166+
impl<O: 'static> SignalVec for Box<dyn SignalVecDynClone<Item = O> + Send + Sync> {
167167
type Item = O;
168168

169169
fn register_boxed_signal_vec(self: Box<Self>, world: &mut World) -> SignalHandle {
@@ -3480,7 +3480,7 @@ pub trait SignalVecExt: SignalVec {
34803480
/// } // without the `.boxed_clone()`, the compiler would not allow this
34813481
/// });
34823482
/// ```
3483-
fn boxed_clone(self) -> Box<dyn SignalVecClone<Item = Self::Item> + Send + Sync>
3483+
fn boxed_clone(self) -> Box<dyn SignalVecDynClone<Item = Self::Item> + Send + Sync>
34843484
where
34853485
Self: Sized + Clone,
34863486
{
@@ -3659,12 +3659,22 @@ impl<T> Clone for MutableVec<T> {
36593659

36603660
impl<T> Drop for MutableVec<T> {
36613661
fn drop(&mut self) {
3662+
bevy_log::info!("cur: {}", self.references.load(core::sync::atomic::Ordering::SeqCst));
36623663
if self.references.fetch_sub(1, core::sync::atomic::Ordering::SeqCst) == 1 {
36633664
STALE_MUTABLE_VECS.lock().unwrap().push(self.entity);
36643665
}
36653666
}
36663667
}
36673668

3669+
#[derive(Component)]
3670+
struct MutableVecHandle<T>(MutableVec<T>);
3671+
3672+
impl<T> Clone for MutableVecHandle<T> {
3673+
fn clone(&self) -> Self {
3674+
Self(self.0.clone())
3675+
}
3676+
}
3677+
36683678
impl<T> MutableVec<T> {
36693679
/// Provides read-only access to the underlying [`Vec`] via either a `&World` or a
36703680
/// `&Query<MutableVecData<T>>`.
@@ -3721,7 +3731,17 @@ impl<T> MutableVec<T> {
37213731
} else {
37223732
vec![]
37233733
};
3724-
world.entity_mut(*replay_signal).insert((QueuedVecDiffs(initial_diffs), VecReplayTrigger(trigger)));
3734+
3735+
let mut replay_entity = world.entity_mut(*replay_signal);
3736+
replay_entity.insert((QueuedVecDiffs(initial_diffs), VecReplayTrigger(trigger)));
3737+
3738+
// --- FIX STARTS HERE ---
3739+
// By inserting a cloned handle onto the signal's entity, we tie the lifetime
3740+
// of the MutableVecData to the lifetime of the signal itself. When this
3741+
// signal is cleaned up and its entity is despawned, this component will be
3742+
// dropped, decrementing the reference count and allowing for proper cleanup.
3743+
replay_entity.insert(MutableVecHandle(self_.clone()));
3744+
// --- FIX ENDS HERE ---
37253745

37263746
pipe_signal(world, broadcaster_system, replay_signal);
37273747
replay_signal
@@ -3927,6 +3947,7 @@ mod tests {
39273947
use crate::JonmoPlugin;
39283948
use bevy::prelude::*;
39293949
use bevy_platform::sync::RwLock;
3950+
use test_log;
39303951

39313952
#[derive(Resource, Default, Debug)]
39323953
struct SignalVecOutput<T: Clone + fmt::Debug>(Vec<VecDiff<T>>);
@@ -4978,7 +4999,7 @@ mod tests {
49784999
handle.cleanup(app.world_mut());
49795000
}
49805001

4981-
#[test]
5002+
#[test_log::test]
49825003
fn test_filter_map() {
49835004
// A comprehensive test for `SignalVecExt::filter_map`, covering all `VecDiff`
49845005
// types and the three status-change scenarios for `UpdateAt`.
@@ -5019,6 +5040,7 @@ mod tests {
50195040
// --- 3. Test `Push` ---
50205041
// Push a value that passes the filter_map ("30").
50215042
source_vec.write(app.world_mut()).push("30");
5043+
drop(source_vec)
50225044
// app.update();
50235045
// let diffs = get_and_clear_output::<u32>(app.world_mut());
50245046
// assert_eq!(diffs.len(), 1);

0 commit comments

Comments
 (0)