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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - Add helpers to send Events from World #5355

Closed
wants to merge 9 commits into from
Closed

[Merged by Bors] - Add helpers to send Events from World #5355

wants to merge 9 commits into from

Conversation

aevyrie
Copy link
Member

@aevyrie aevyrie commented Jul 17, 2022 •
edited
Loading

Objective

  • With access to World, it's not obvious how to send an event.
  • This is especially useful if you are writing a Command that needs to send an Event.
  • Events are a first-class construct in bevy, even though they are just Resources under the hood. Their methods should be discoverable.

Solution

  • Provide a simple helpers to send events through Res<Events<T>>.

Changelog

send_event, send_default_event, and send_event_batch methods added to World.

@aevyrie aevyrie added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jul 17, 2022
@aevyrie aevyrie changed the title add event sending helpers to World Add helpers to send Events from World Jul 17, 2022
@aevyrie aevyrie marked this pull request as ready for review July 17, 2022 16:14
@alice-i-cecile
Copy link
Member

send_event and send_default_event for legibility please :)

@alice-i-cecile
Copy link
Member

Previously attempted in #3839.

Nilirad
Nilirad approved these changes Jul 17, 2022
View reviewed changes
Copy link
Contributor

@Nilirad Nilirad left a comment

Choose a reason for hiding this comment

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

I have mixed feelings about using the noun-verb grammar, but I wouldn't block that.

I would have suggested you to link to EventWriter examples, but there are none. 😐

crates/bevy_ecs/src/world/mod.rs Outdated Show resolved Hide resolved
crates/bevy_ecs/src/world/mod.rs Outdated Show resolved Hide resolved
crates/bevy_ecs/src/world/mod.rs Outdated Show resolved Hide resolved
aevyrie and others added 2 commits July 17, 2022 11:25
@aevyrie aevyrie force-pushed the world-event-helpers branch from 2a57b6a to e4b489c Compare July 17, 2022 19:16
@aevyrie aevyrie added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jul 17, 2022
mockersf
mockersf requested changes Jul 18, 2022
View reviewed changes
Copy link
Member

@mockersf mockersf left a comment

Choose a reason for hiding this comment

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

Could you also add the send_batch method to have the same features that the EventWriter?

Nilirad
Nilirad suggested changes Jul 18, 2022
View reviewed changes
Copy link
Contributor

@Nilirad Nilirad left a comment

Choose a reason for hiding this comment

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

I prefer to write “Event” in monospace since we're linking to the trait itself and we're passing a type that implements it as an argument.

I'm OK for now to link to the trait instead of the module in the docs, since there is practically no documentation at the module level. This can be changed later with an event docs overhaul.

I prefer that the suggestion made in this comment should be applied before merging.

crates/bevy_ecs/src/world/mod.rs Outdated Show resolved Hide resolved
crates/bevy_ecs/src/world/mod.rs Outdated Show resolved Hide resolved
@Nilirad Nilirad removed the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jul 18, 2022
@aevyrie
Copy link
Member Author

aevyrie commented Jul 19, 2022

Added send_batch. I refactored the other two methods as special cases of send_batch to keep the error checks nice and DRY. 😛

@aevyrie aevyrie added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jul 19, 2022
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Jul 19, 2022
@aevyrie
Add helpers to send Events from World (#5355)
282f8ed 
# Objective

- With access to `World`, it's not obvious how to send an event.
- This is especially useful if you are writing a `Command` that needs to send an `Event`.
- `Events` are a first-class construct in bevy, even though they are just `Resources` under the hood. Their methods should be discoverable.

## Solution

- Provide a simple helpers to send events through `Res<Events<T>>`.
---

## Changelog

> `send_event`, `send_default_event`, and `send_event_batch` methods added to `World`.
@bors
Copy link
Contributor

bors bot commented Jul 19, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Add helpers to send Events from World [Merged by Bors] - Add helpers to send Events from World Jul 19, 2022
@bors bors bot closed this Jul 19, 2022
@mockersf mockersf mentioned this pull request Jul 19, 2022
Closed
@mockersf
Copy link
Member

missed to propose a rename before this was merged: #5383

bors bot pushed a commit that referenced this pull request Jul 19, 2022
@mockersf
rename send_default_event to send_event_default on world (#5383)
0fa499a 
after #5355, three methods were added on world:
* `send_event`
* `send_event_batch`
* `send_default_event`

rename `send_default_event` to `send_event_default` for better discoverability
inodentry pushed a commit to IyesGames/bevy that referenced this pull request Aug 8, 2022
@aevyrie @inodentry
Add helpers to send Events from World (bevyengine#5355)
ad18714 
# Objective

- With access to `World`, it's not obvious how to send an event.
- This is especially useful if you are writing a `Command` that needs to send an `Event`.
- `Events` are a first-class construct in bevy, even though they are just `Resources` under the hood. Their methods should be discoverable.

## Solution

- Provide a simple helpers to send events through `Res<Events<T>>`.
---

## Changelog

> `send_event`, `send_default_event`, and `send_event_batch` methods added to `World`.
inodentry pushed a commit to IyesGames/bevy that referenced this pull request Aug 8, 2022
@mockersf @inodentry
rename send_default_event to send_event_default on world (bevyengine#…
e7e7e7a 
…5383)

after bevyengine#5355, three methods were added on world:
* `send_event`
* `send_event_batch`
* `send_default_event`

rename `send_default_event` to `send_event_default` for better discoverability
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
@aevyrie @james7132
Add helpers to send Events from World (bevyengine#5355)
54d9004 
# Objective

- With access to `World`, it's not obvious how to send an event.
- This is especially useful if you are writing a `Command` that needs to send an `Event`.
- `Events` are a first-class construct in bevy, even though they are just `Resources` under the hood. Their methods should be discoverable.

## Solution

- Provide a simple helpers to send events through `Res<Events<T>>`.
---

## Changelog

> `send_event`, `send_default_event`, and `send_event_batch` methods added to `World`.
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
@mockersf @james7132
rename send_default_event to send_event_default on world (bevyengine#…
3b2b959 
…5383)

after bevyengine#5355, three methods were added on world:
* `send_event`
* `send_event_batch`
* `send_default_event`

rename `send_default_event` to `send_event_default` for better discoverability
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
@aevyrie @ItsDoot
Add helpers to send Events from World (bevyengine#5355)
89768c4 
# Objective

- With access to `World`, it's not obvious how to send an event.
- This is especially useful if you are writing a `Command` that needs to send an `Event`.
- `Events` are a first-class construct in bevy, even though they are just `Resources` under the hood. Their methods should be discoverable.

## Solution

- Provide a simple helpers to send events through `Res<Events<T>>`.
---

## Changelog

> `send_event`, `send_default_event`, and `send_event_batch` methods added to `World`.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
@mockersf @ItsDoot
rename send_default_event to send_event_default on world (bevyengine#…
455f81f 
…5383)

after bevyengine#5355, three methods were added on world:
* `send_event`
* `send_event_batch`
* `send_default_event`

rename `send_default_event` to `send_event_default` for better discoverability
@JoJoJet JoJoJet mentioned this pull request Feb 7, 2023
Closed
bors bot pushed a commit that referenced this pull request Feb 7, 2023
@JoJoJet
Simplify a doc example for EventWriter (#7549)
5d2cd08 
# Objective

Use the `World::send_event` method added in #5355 to simplify a doc example for `EventWriter`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nilirad Nilirad Nilirad requested changes

@mockersf mockersf mockersf requested changes

@IceSentry IceSentry IceSentry approved these changes

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@aevyrie @alice-i-cecile @mockersf @IceSentry @Nilirad