-
-
Notifications
You must be signed in to change notification settings - Fork 4k
Split observer events #20151
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
base: main
Are you sure you want to change the base?
Split observer events #20151
Conversation
We can replace the untargeted form with a buffered event IMO. |
@@ -1,4 +1,20 @@ | |||
//! Event handling types. | |||
//! Events are things that "happen" and can be processed by app logic. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really excellent docs!
There was a problem hiding this 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 :)
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. |
Objective
trigger
andtrigger_targets
- by traitSolution
Check out the diff of the release notes.
Notes
BroadcastEvent
andEntityEvent
together, one must be implemented manually if you want both. This is probably okay as having both implemented is somewhat undesirable.Event
toObserverEvent
.