Skip to content

Split observer events#20151

Closed
tim-blackbird wants to merge 18 commits intobevyengine:mainfrom
tim-blackbird:split-observer-events
Closed

Split observer events#20151
tim-blackbird wants to merge 18 commits intobevyengine:mainfrom
tim-blackbird:split-observer-events

Conversation

@tim-blackbird
Copy link
Contributor

@tim-blackbird tim-blackbird commented Jul 15, 2025

Objective

  • Split the two types of observer usages - trigger and trigger_targets - by trait

Solution

Check out the diff of the release notes.

Notes

  • It's not possible to derive BroadcastEvent and EntityEvent together, one must be implemented manually if you want both. This is probably okay as having both implemented is somewhat undesirable.
  • Planned follow-up to this PR is to rename Event to ObserverEvent.

@tim-blackbird tim-blackbird added A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jul 15, 2025
@alice-i-cecile
Copy link
Member

What to do with the SceneInstanceReady event which is the only event in Bevy to use both trigger and trigger_targets.

We can replace the untargeted form with a buffered event IMO.

@alice-i-cecile alice-i-cecile added this to the 0.17 milestone Jul 15, 2025
@tim-blackbird tim-blackbird marked this pull request as ready for review July 17, 2025 13:45
@@ -1,4 +1,20 @@
//! Event handling types.
//! Events are things that "happen" and can be processed by app logic.
Copy link
Member

Choose a reason for hiding this comment

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

Really excellent docs!

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Very nice. I'm pleased with how simple the diff is. I'm increasingly fond of this as the final form: this should give us 99% of the type-safety benefits without any complexity around mutually exclusive traits or a On variant for broadcast events.

Well, at least after Event -> ObserverEvent is done :)

@alice-i-cecile alice-i-cecile added X-Needs-SME This type of work requires an SME to approve it. S-Waiting-on-SME This is currently waiting for an SME to resolve something controversial and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jul 17, 2025
@alice-i-cecile
Copy link
Member

I spent some time today writing up a focused little design doc for the ultimate split here: https://hackmd.io/@alice-i-cecile/SyuIWJPLxg

I don't particularly care whether we do it in a single PR or across multiple, as long as we converge to our ideal form before we ship 0.17.

Copy link
Contributor

@SteveAlexander SteveAlexander left a comment

Choose a reason for hiding this comment

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

Great to see improved types and improved docs. No comments.

Copy link
Contributor

Choose a reason for hiding this comment

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

Much clearer docs :-)

@alice-i-cecile
Copy link
Member

Consolidating into #20731.

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-Code-Quality A section of code that is hard to understand or change D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Waiting-on-SME This is currently waiting for an SME to resolve something controversial X-Needs-SME This type of work requires an SME to approve it.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments