Skip to content

Changing the notification protocol for core_widgets. #20086

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

Merged
merged 2 commits into from
Jul 13, 2025

Conversation

viridia
Copy link
Contributor

@viridia viridia commented Jul 11, 2025

Notifications now include the source entity. This is useful for callbacks that are responsible for more than one widget.

Part of #19236

This is an incremental change only: I have not altered the fundamental nature of callbacks, as this is still in discussion. The only change here is to include the source entity id with the notification.

The existing examples don't leverage this new field, but that will change when I work on the color sliders PR.

I have been careful not to use the word "events" in describing the notification message structs because they are not capital-E Events at this time. That may change depending on the outcome of discussions.

@alice-i-cecile

viridia added 2 commits July 11, 2025 08:52
Notifications now include the source entity. This is useful for
callbacks that are responsible for more than one widget.
@alice-i-cecile alice-i-cecile requested review from ickshonpe and cart July 12, 2025 04:24
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.

Good, I suspect this will be useful regardless of the final direction we end up choosing.

@alice-i-cecile alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Contentious There are nontrivial implications that should be thought through D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 12, 2025
Copy link
Contributor

@ickshonpe ickshonpe left a comment

Choose a reason for hiding this comment

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

It always bothers me how fragile objects composed from multiple entities seem in Bevy.
Agree the source entities are necessary, and the input value wrapper types provide some extra context to the callbacks which might be helpful to users I guess.

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 13, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jul 13, 2025
Merged via the queue into bevyengine:main with commit ace0114 Jul 13, 2025
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants