Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reactive ECS-integrated signals #16309

Closed
wants to merge 7 commits into from
Closed

Reactive ECS-integrated signals #16309

wants to merge 7 commits into from

Conversation

JMS55
Copy link
Contributor

@JMS55 JMS55 commented Nov 9, 2024

Objective

  • Provide a way for an entity to have some component C that automatically updates based on the value of some other piece of data of type S
  • This "piece of data" is called a Signal https://docs.solidjs.com/concepts/signals

Solution

  • Spawn an entity to hold the signal data S
  • Provide a SubscribedComponentSetup component that given a mapping function from &S -> C computes an initial value c based on the current value of the signal, inserts c on the subscribed entity (the one with the setup component), and sets up an observer for monitoring changes to the signal data, and then calling the mapping function again to produce a new c and updating the subscribed entity's C.

Problems

  • Automatic signal cleanup requires storing Arc<()> in Signal, which means Signal is no longer Copy, which is quite painful to pass into observer callbacks :/. We can remove this, but then unused signals never get cleaned up...

TODO

  • Depends on first merging Mutate Observer #16183
  • Derived signals
  • Allow signals to add/remove components, not just set values
  • Allow signals to control entity hierarchies, and not just components
  • Add commands.spawn_signal() and app.spawn_signal()?
  • Write tests
  • Flesh out docs
  • Add an example file

@JMS55 JMS55 added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events labels Nov 9, 2024
/// System to automatically clean up unused [`Signal`]s.
pub fn signal_cleanup(signals: Query<(Entity, &SignalRefcount)>, mut commands: Commands) {
for (signal, refcount) in &signals {
if Arc::strong_count(&refcount.0) == 1 {
Copy link
Contributor

@ItsDoot ItsDoot Nov 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend doing what Asset handles do and have an Arc<T> where T queues an event to despawn the entity when the Arc data is dropped. SignalRefcount would probably want to hold a Weak<T> then. This means you don't have to iterate through all signals, just ones that actually need to be cleaned up.

Edit: Unless you actually need to check the reference count for other reasons, you probably don't need SignalRefcount at all with the above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless you actually need to check the reference count for other reasons, you probably don't need SignalRefcount at all with the above.

Not sure I get how, but I would love to get rid of it. Currently it's used for two things:

  • If an entity (X) has a component subscribed to a signal, that's handled by spawning a new observer entity. Said observer entity gets a SignalRefcount component to ensure that the signal entity is not despawned while the observer entity is alive, i.e. while entity X has a subscription to the signal.
  • If you have a Signal type (which is not a bevy component or resource), it internally holds a SignalRefcount so that the signal entity is not despawned while Signal is alive. You can pass this Signal into observer callbacks for button clicks or event handling systems or whatever in order to call signal.set(v) to update the signal's value.

I would love to get rid of the refcount so that Signal can be Copy again, but then I don't know how to automatically decide when to cleanup the signal entity.

@JMS55 JMS55 closed this Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants