Skip to content

Granular event privacy #5356

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

Closed
wants to merge 3 commits into from
Closed

Conversation

joseph-gio
Copy link
Member

@joseph-gio joseph-gio commented Jul 17, 2022

Objective

  • Allow users to separately restrict access to either EventReader<T> or EventWriter<T>, for a given T.
  • Support construction of robust API's by making it easy to restrict access.
    • Read-only events - good for sending messages to downstream plugins without letting them interfere with one another.
    • Write-only events - good for sending messages to upstream plugins without letting the downstream plugins observe each other.
  • Enforce invariants - by allowing users to enforce invariants, it could allow type-specific optimizations in the future.
    • Example - if you know that there is only one EventReader of a given type, you can massively simplify the implementation of the event queue.
    • Example - if you know that there is only one EventWriter of a given type, you don't need to bother with synchronizing writes.
    • This would require some way of changing the "event handler" from Events<T> to a custom implementation.

Solution

  • Add marker traits Read<> and Write<>, which restrict access to EventReader and EventWriter based on the visibility of a marker type parameter.
  • Default to unrestricted visibility - no breaking changes.
mod inner {
    struct Private;

    #[derive(Default)]
    pub struct MyEvent;

    // Can only send events if you can name the type `Private`.
    // Here, that means only within the module `inner`.
    impl bevy_ecs::event::Write<Private> for MyEvent {}

    // No restrictions on where you can send events.
    impl bevy_ecs::event::Read for MyEvent {}

    // the type `Private` is like a key that unlocks the `EventReader`.
    fn inner_system(mut writer: EventWriter<MyEvent, Private>) {
        writer.send_default();
    }
}

// This doesn't compile, can't name `Private` outside of its module.
fn outer_system(mut writer: EventWriter<inner::MyEvent, inner::Private>) {}

// ...and this doesn't compile either.
fn outer_system2(mut writer: EventWriter<inner::MyEvent>) {}

// This *does* compile - reads are unrestricted so we don't even need to worry about `Private`.
fn read_system(mut reader: EventReader<inner::MyEvent>) {}

// For the common case where you want unrestricted writes and reads, you can just
#[derive(Event)]
pub struct YourEvent

// We are allowed to read or write anywhere we want.
fn your_system(mut writer: EventWriter<YourEvent>) {}
fn your_system2(mut reader: EventReader<YourEvent>) {}

Notes

#[derive(Event)] macro is necessary to make this change ergonomic, but it seems like we're already going that direction since other PRs require configuring events at the type level. For example, some of the discussion in #4832.


Changelog

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events X-Controversial There is active debate or serious implications around merging this PR labels Jul 17, 2022
@BoxyUwU
Copy link
Member

BoxyUwU commented Jul 21, 2022

thoughts:

  • If this can be bypassed then a lot of the "pros" of doing this get dulled a bit
  • When coming up with the idea of MutPerm to restrict mutable access to components we still allowed getting access to a Mut<T> out of a Query and you had to provide a value of the MutPerm assoc type to be allowed to get an &mut T. Might be worth considering a design like that for event privacy instead of purely basing it on "can the user name the type"

@joseph-gio
Copy link
Member Author

joseph-gio commented Jul 21, 2022

  • If this can be bypassed then a lot of the "pros" of doing this get dulled a bit

Agreed, that's the main reason this is still a draft. As mentioned, I'd like to turn Events<_> into an implementation detail. Anyone who wants to encapsulate their events would have to swap it out for some PrivateEvents<_, Marker> type (strawman), or roll their custom implementation.

  • When coming up with the idea of MutPerm to restrict mutable access to components we still allowed getting access to a Mut<T> out of a Query and you had to provide a value of the MutPerm assoc type to be allowed to get an &mut T. Might be worth considering a design like that for event privacy instead of purely basing it on "can the user name the type"

That's interesting, do you have some example code?

@joseph-gio
Copy link
Member Author

Actually, I figured out how to encapsulate Events<_> in a way that should be seamless. I'm not currently aware of any safe ways to bypass this, now.

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 X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants