The `DoubleEndedIterator` impls produce incorrect results on subsequent calls to `iter()` if the iterator is only partially consumed.
The following code shows what happens
```rust
fn next_back_is_bad() {
let mut events = Events::<TestEvent>::default();
events.send(TestEvent { i: 0 });
events.send(TestEvent { i: 1 });
events.send(TestEvent { i: 2 });
let mut reader = events.get_reader();
let mut iter = reader.iter(&events);
assert_eq!(iter.next_back(), Some(&TestEvent { i: 2 }));
assert_eq!(iter.next(), Some(&TestEvent { i: 0 }));
let mut iter = reader.iter(&events);
// `i: 2` event is returned twice! The `i: 1` event is missed.
assert_eq!(iter.next(), Some(&TestEvent { i: 2 }));
assert_eq!(iter.next(), None);
}
```
I don't think this can be fixed without adding some very convoluted bookkeeping.
## Migration Guide
`ManualEventIterator` and `ManualEventIteratorWithId` are no longer `DoubleEndedIterator`s.
Co-authored-by: devil-ira <justthecooldude@gmail.com>
# Objective
Clearing the reader doesn't require iterating the events. Updating the `last_event_count` of the reader is enough.
I rewrote part of the documentation as some of it was incorrect or harder to understand than necessary.
## Changelog
Added `ManualEventReader::clear()`
Co-authored-by: devil-ira <justthecooldude@gmail.com>
# Objective
- Be able to name the type that `ManualEventReader::iter/iter_with_id` returns and `EventReader::iter/iter_with_id` by proxy.
Currently for the purpose of https://github.com/bevyengine/bevy/pull/5719
## Solution
- Create a custom `Iterator` type.
The PR fixes the interface of `EventReader::clear`. Currently, the method consumes the reader, which makes it unusable.
## Changelog
- `EventReader::clear` now takes a mutable reference instead of consuming the event reader.
## Migration Guide
`EventReader::clear` now takes a mutable reference instead of consuming the event reader. This means that `clear` now needs explicit mutable access to the reader variable, which previously could have been omitted in some cases:
```rust
// Old (0.9)
fn clear_events(reader: EventReader<SomeEvent>) {
reader.clear();
}
// New (0.10)
fn clear_events(mut reader: EventReader<SomeEvent>) {
reader.clear();
}
```
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
Currently, the `SystemParam` derive forces you to declare the lifetime parameters `<'w, 's>`, even if you don't use them.
If you don't follow this structure, the error message is quite nasty.
### Example (before):
```rust
#[derive(SystemParam)]
pub struct EventWriter<'w, 's, E: Event> {
events: ResMut<'w, Events<E>>,
// The derive forces us to declare the `'s` lifetime even though we don't use it,
// so we have to add this `PhantomData` to please rustc.
#[system_param(ignore)]
_marker: PhantomData<&'s ()>,
}
```
## Solution
* Allow the user to omit either lifetime.
* Emit a descriptive error if any lifetimes used are invalid.
### Example (after):
```rust
#[derive(SystemParam)]
pub struct EventWriter<'w, E: Event> {
events: ResMut<'w, Events<E>>,
}
```
---
## Changelog
* The `SystemParam` derive is now more flexible, allowing you to omit unused lifetime parameters.
# Objective
- Reverts #5730.
- Fixes#6173, fixes#6596.
## Solution
Remove the warning entirely.
## Changelog
You will no longer be spammed about
> Missed 31 `bevy_input:🐭:MouseMotion` events. Consider
reading from the `EventReader` more often (generally the best
solution) or calling Events::update() less frequently
(normally this is called once per frame). This problem is most
likely due to run criteria/fixed timesteps or consuming events
conditionally. See the Events documentation for
more information.
when you miss events. These warnings were often (but not always) a false positive. You can still check this manually by using `ManualEventReader::missed_events`
# Objective
- Reduce debugging burden when using events by telling user when they missed an event.
## Solution
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
*This PR description is an edited copy of #5007, written by @alice-i-cecile.*
# Objective
Follow-up to https://github.com/bevyengine/bevy/pull/2254. The `Resource` trait currently has a blanket implementation for all types that meet its bounds.
While ergonomic, this results in several drawbacks:
* it is possible to make confusing, silent mistakes such as inserting a function pointer (Foo) rather than a value (Foo::Bar) as a resource
* it is challenging to discover if a type is intended to be used as a resource
* we cannot later add customization options (see the [RFC](https://github.com/bevyengine/rfcs/blob/main/rfcs/27-derive-component.md) for the equivalent choice for Component).
* dependencies can use the same Rust type as a resource in invisibly conflicting ways
* raw Rust types used as resources cannot preserve privacy appropriately, as anyone able to access that type can read and write to internal values
* we cannot capture a definitive list of possible resources to display to users in an editor
## Notes to reviewers
* Review this commit-by-commit; there's effectively no back-tracking and there's a lot of churn in some of these commits.
*ira: My commits are not as well organized :')*
* I've relaxed the bound on Local to Send + Sync + 'static: I don't think these concerns apply there, so this can keep things simple. Storing e.g. a u32 in a Local is fine, because there's a variable name attached explaining what it does.
* I think this is a bad place for the Resource trait to live, but I've left it in place to make reviewing easier. IMO that's best tackled with https://github.com/bevyengine/bevy/issues/4981.
## Changelog
`Resource` is no longer automatically implemented for all matching types. Instead, use the new `#[derive(Resource)]` macro.
## Migration Guide
Add `#[derive(Resource)]` to all types you are using as a resource.
If you are using a third party type as a resource, wrap it in a tuple struct to bypass orphan rules. Consider deriving `Deref` and `DerefMut` to improve ergonomics.
`ClearColor` no longer implements `Component`. Using `ClearColor` as a component in 0.8 did nothing.
Use the `ClearColorConfig` in the `Camera3d` and `Camera2d` components instead.
Co-authored-by: Alice <alice.i.cecile@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: devil-ira <justthecooldude@gmail.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
Improve documentation, information users of the limitations in bevy's idiomatic patterns, and suggesting alternatives for when those limitations are encountered.
## Solution
* Add documentation to `Commands` informing the user of the option of writing one-shot commands with closures.
* Add documentation to `EventWriter` regarding the limitations of event types, and suggesting alternatives using commands.
# Objective
- It's pretty common to want to check if an EventReader has received one or multiple events while also needing to consume the iterator to "clear" the EventReader.
- The current approach is to do something like `events.iter().count() > 0` or `events.iter().last().is_some()`. It's not immediately obvious that the purpose of that is to consume the events and check if there were any events. My solution doesn't really solve that part, but it encapsulates the pattern.
## Solution
- Add a `.clear()` method that consumes the iterator.
- It takes the EventReader by value to make sure it isn't used again after it has been called.
---
## Migration Guide
Not a breaking change, but if you ever found yourself in a situation where you needed to consume the EventReader and check if there was any events you can now use
```rust
fn system(events: EventReader<MyEvent>) {
if !events.is_empty {
events.clear();
// Process the fact that one or more event was received
}
}
```
Co-authored-by: Charles <IceSentry@users.noreply.github.com>
# Objective
- The code in `events.rs` was a bit messy. There was lots of duplication between `EventReader` and `ManualEventReader`, and the state management code is not needed.
## Solution
- Clean it up.
## Future work
Should we remove the type parameter from `ManualEventReader`?
It doesn't have any meaning outside of its source `Events`. But there's no real reason why it needs to have a type parameter - it's just plain data. I didn't remove it yet to keep the type safety in some of the users of it (primarily related to `&mut World` usage)
Required for https://github.com/bevyengine/bevy/pull/4402.
# Objective
- derived `SystemParam` implementations were never `ReadOnlySystemParamFetch`
- We want them to be, e.g. for `EventReader`
## Solution
- If possible, 'forward' the impl of `ReadOnlySystemParamFetch`.
# Objective
- Remove `Resource` binding on events, introduce a new `Event` trait
- Ensure event iterators are `ExactSizeIterator`
## Solution
- Builds on #2382 and #2969
## Changelog
- Events<T>, EventWriter<T>, EventReader<T> and so on now require that the underlying type is Event, rather than Resource. Both of these are trivial supertraits of Send + Sync + 'static with universal blanket implementations: this change is currently purely cosmetic.
- Event reader iterators now implement ExactSizeIterator
What is says on the tin.
This has got more to do with making `clippy` slightly more *quiet* than it does with changing anything that might greatly impact readability or performance.
that said, deriving `Default` for a couple of structs is a nice easy win
# Objective
When using empty events, it can feel redundant to have to specify the type of the event when sending it.
## Solution
Add a new `fire()` function that sends the default value of the event. This requires that the event derives Default.
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
Provide a non-consuming method of checking if there are events in an `EventReader`.
Fixes#2967
## Solution
Implements the `len` and `is_empty` functions for `EventReader` and `ManualEventReader`, giving users the ability to check for the presence of new events without consuming any.
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
Currently, simply calling `iter` on an event reader will mark all of it's events as read, even if the returned iterator is never used
## Solution
With this, the cursor will simply move to the last unread, but available event when iter is called, and incremented by one per `next` call.
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
This PR extends the `Events` documentation by:
- informing user about the possible race condition
- explicitly explaining the unusual double buffer implementation
Fixes#3305
Co-authored-by: MiniaczQ <jakub.motyka.2000@gmail.com>
Co-authored-by: MiniaczQ <MiniaczQ@gmail.com>
#3457 adds the `doc_markdown` clippy lint, which checks doc comments to make sure code identifiers are escaped with backticks. This causes a lot of lint errors, so this is one of a number of PR's that will fix those lint errors one crate at a time.
This PR fixes lints in the `bevy_ecs` crate.
This implements the most minimal variant of #1843 - a derive for marker trait. This is a prerequisite to more complicated features like statically defined storage type or opt-out component reflection.
In order to make component struct's purpose explicit and avoid misuse, it must be annotated with `#[derive(Component)]` (manual impl is discouraged for compatibility). Right now this is just a marker trait, but in the future it might be expanded. Making this change early allows us to make further changes later without breaking backward compatibility for derive macro users.
This already prevents a lot of issues, like using bundles in `insert` calls. Primitive types are no longer valid components as well. This can be easily worked around by adding newtype wrappers and deriving `Component` for them.
One funny example of prevented bad code (from our own tests) is when an newtype struct or enum variant is used. Previously, it was possible to write `insert(Newtype)` instead of `insert(Newtype(value))`. That code compiled, because function pointers (in this case newtype struct constructor) implement `Send + Sync + 'static`, so we allowed them to be used as components. This is no longer the case and such invalid code will trigger a compile error.
Co-authored-by: = <=>
Co-authored-by: TheRawMeatball <therawmeatball@gmail.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
## Objective
The upcoming Bevy Book makes many references to the API documentation of bevy.
Most references belong to the first two chapters of the Bevy Book:
- bevyengine/bevy-website#176
- bevyengine/bevy-website#182
This PR attempts to improve the documentation of `bevy_ecs` and `bevy_app` in order to help readers of the Book who want to delve deeper into technical details.
## Solution
- Add crate and level module documentation
- Document the most important items (basically those included in the preludes), with the following style, where applicable:
- **Summary.** Short description of the item.
- **Second paragraph.** Detailed description of the item, without going too much in the implementation.
- **Code example(s).**
- **Safety or panic notes.**
## Collaboration
Any kind of collaboration is welcome, especially corrections, wording, new ideas and guidelines on where the focus should be put in.
---
### Related issues
- Fixes#2246
# Objective
Enable using exact World lifetimes during read-only access . This is motivated by the new renderer's need to allow read-only world-only queries to outlive the query itself (but still be constrained by the world lifetime).
For example:
115b170d1f/pipelined/bevy_pbr2/src/render/mod.rs (L774)
## Solution
Split out SystemParam state and world lifetimes and pipe those lifetimes up to read-only Query ops (and add into_inner for Res). According to every safety test I've run so far (except one), this is safe (see the temporary safety test commit). Note that changing the mutable variants to the new lifetimes would allow aliased mutable pointers (try doing that to see how it affects the temporary safety tests).
The new state lifetime on SystemParam does make `#[derive(SystemParam)]` more cumbersome (the current impl requires PhantomData if you don't use both lifetimes). We can make this better by detecting whether or not a lifetime is used in the derive and adjusting accordingly, but that should probably be done in its own pr.
## Why is this a draft?
The new lifetimes break QuerySet safety in one very specific case (see the query_set system in system_safety_test). We need to solve this before we can use the lifetimes given.
This is due to the fact that QuerySet is just a wrapper over Query, which now relies on world lifetimes instead of `&self` lifetimes to prevent aliasing (but in systems, each Query has its own implied lifetime, not a centralized world lifetime). I believe the fix is to rewrite QuerySet to have its own World lifetime (and own the internal reference). This will complicate the impl a bit, but I think it is doable. I'm curious if anyone else has better ideas.
Personally, I think these new lifetimes need to happen. We've gotta have a way to directly tie read-only World queries to the World lifetime. The new renderer is the first place this has come up, but I doubt it will be the last. Worst case scenario we can come up with a second `WorldLifetimeQuery<Q, F = ()>` parameter to enable these read-only scenarios, but I'd rather not add another type to the type zoo.
This is extracted out of eb8f973646476b4a4926ba644a77e2b3a5772159 and includes some additional changes to remove all references to AppBuilder and fix examples that still used App::build() instead of App::new(). In addition I didn't extract the sub app feature as it isn't ready yet.
You can use `git diff --diff-filter=M eb8f973646476b4a4926ba644a77e2b3a5772159` to find all differences in this PR. The `--diff-filtered=M` filters all files added in the original commit but not in this commit away.
Co-Authored-By: Carter Anderson <mcanders1@gmail.com>
## Objective
- Fixes: #2275
- `Assets` were being flagged as 'changed' each frame regardless of if the assets were actually being updated.
## Solution
- Only have `Assets` change detection be triggered when the collection is actually modified.
- This includes utilizing `ResMut` further down the stack instead of a `&mut Assets` directly.
The previous implementation of `Events::extend` iterated through each event and manually `sent` it via `Events:;send`.
However, this could be a minor performance hit since calling `Vec::push` in a loop is not optimal.
This refactors the code to use `Vec::extend`.
Fixes#1809. It makes it also possible to use `derive` for `SystemParam` inside ECS and avoid manual implementation. An alternative solution to macro changes is to use `use crate as bevy_ecs;` in `event.rs`.
2021-04-13 20:36:37 +00:00
Renamed from crates/bevy_app/src/event.rs (Browse further)