# Objective
Fix#6453.
## Solution
Use the solution mentioned in the issue by catching the unwind and dropping the error. Wrap the `executor.try_tick` calls with `std::catch::unwind`.
Ideally this would be moved outside of the hot loop, but the mut ref to the `spawned` future is not `UnwindSafe`.
This PR only addresses the bug, we can address the perf issues (should there be any) later.
# Objective
Attempting to build `bevy_tasks` produces the following error:
```
error[E0599]: no method named `is_finished` found for struct `async_executor::Task` in the current scope
--> /[...]]/bevy/crates/bevy_tasks/src/task.rs:51:16
|
51 | self.0.is_finished()
| ^^^^^^^^^^^ method not found in `async_executor::Task<T>`
```
It looks like this was introduced along with `Task::is_finished`, which delegates to `async_task::Task::is_finished`. However, the latter was only introduced in `async-task` 4.2.0; `bevy_tasks` does not explicitly depend on `async-task` but on `async-executor` ^1.3.0, which in turn depends on `async-task` ^4.0.0.
## Solution
Add an explicit dependency on `async-task` ^4.2.0.
# Objective
Right now, the `TaskPool` implementation allows panics to permanently kill worker threads upon panicking. This is currently non-recoverable without using a `std::panic::catch_unwind` in every scheduled task. This is poor ergonomics and even poorer developer experience. This is exacerbated by #2250 as these threads are global and cannot be replaced after initialization.
Removes the need for temporary fixes like #4998. Fixes#4996. Fixes#6081. Fixes#5285. Fixes#5054. Supersedes #2307.
## Solution
The current solution is to wrap `Executor::run` in `TaskPool` with a `catch_unwind`, and discarding the potential panic. This was taken straight from [smol](404c7bcc0a/src/spawn.rs (L44))'s current implementation. ~~However, this is not entirely ideal as:~~
- ~~the signaled to the awaiting task. We would need to change `Task<T>` to use `async_task::FallibleTask` internally, and even then it doesn't signal *why* it panicked, just that it did.~~ (See below).
- ~~no error is logged of any kind~~ (See below)
- ~~it's unclear if it drops other tasks in the executor~~ (it does not)
- ~~This allows the ECS parallel executor to keep chugging even though a system's task has been dropped. This inevitably leads to deadlock in the executor.~~ Assuming we don't catch the unwind in ParallelExecutor, this will naturally kill the main thread.
### Alternatives
A final solution likely will incorporate elements of any or all of the following.
#### ~~Log and Ignore~~
~~Log the panic, drop the task, keep chugging. This only addresses the discoverability of the panic. The process will continue to run, probably deadlocking the executor. tokio's detatched tasks operate in this fashion.~~
Panics already do this by default, even when caught by `catch_unwind`.
#### ~~`catch_unwind` in `ParallelExecutor`~~
~~Add another layer catching system-level panics into the `ParallelExecutor`. How the executor continues when a core dependency of many systems fails to run is up for debate.~~
`async_task::Task` bubbles up panics already, this will transitively push panics all the way to the main thread.
#### ~~Emulate/Copy `tokio::JoinHandle` with `Task<T>`~~
~~`tokio::JoinHandle<T>` bubbles up the panic from the underlying task when awaited. This can be transitively applied across other APIs that also use `Task<T>` like `Query::par_for_each` and `TaskPool::scope`, bubbling up the panic until it's either caught or it reaches the main thread.~~
`async_task::Task` bubbles up panics already, this will transitively push panics all the way to the main thread.
#### Abort on Panic
The nuclear option. Log the error, abort the entire process on any thread in the task pool panicking. Definitely avoids any additional infrastructure for passing the panic around, and might actually lead to more efficient code as any unwinding is optimized out. However gives the developer zero options for dealing with the issue, a seemingly poor choice for debuggability, and prevents graceful shutdown of the process. Potentially an option for handling very low-level task management (a la #4740). Roughly takes the shape of:
```rust
struct AbortOnPanic;
impl Drop for AbortOnPanic {
fn drop(&mut self) {
abort!();
}
}
let guard = AbortOnPanic;
// Run task
std::mem::forget(AbortOnPanic);
```
---
## Changelog
Changed: `bevy_tasks::TaskPool`'s threads will no longer terminate permanently when a task scheduled onto them panics.
Changed: `bevy_tasks::Task` and`bevy_tasks::Scope` will propagate panics in the spawned tasks/scopes to the parent thread.
This reverts commit 53d387f340.
# Objective
Reverts #6448. This didn't have the intended effect: we're now getting bevy::prelude shown in the docs again.
Co-authored-by: Alejandro Pascual <alejandro.pascual.pozo@gmail.com>
# Objective
- Right now re-exports are completely hidden in prelude docs.
- Fixes#6433
## Solution
- We could show the re-exports without inlining their documentation.
# Objective
In some scenarios it can be useful to check if a task has been finished without polling it. I added a function called `is_finished` to check if a task has been finished.
## Solution
Since `async_task` supports it out of the box, it is just a simple wrapper function.
---
# Objective
- fix new clippy lints before they get stable and break CI
## Solution
- run `clippy --fix` to auto-fix machine-applicable lints
- silence `clippy::should_implement_trait` for `fn HandleId::default<T: Asset>`
## Changes
- always prefer `format!("{inline}")` over `format!("{}", not_inline)`
- prefer `Box::default` (or `Box::<T>::default` if necessary) over `Box::new(T::default())`
# Objective
- Proactive changing of code to comply with warnings generated by beta of rustlang version of cargo clippy.
## Solution
- Code changed as recommended by `rustup update`, `rustup default beta`, `cargo run -p ci -- clippy`.
- Tested using `beta` and `stable`. No clippy warnings in either after changes made.
---
## Changelog
- Warnings fixed were: `clippy::explicit-auto-deref` (present in 11 files), `clippy::needless-borrow` (present in 2 files), and `clippy::only-used-in-recursion` (only 1 file).
# Objective
- #4466 broke local tasks running.
- Fixes https://github.com/bevyengine/bevy/issues/6120
## Solution
- Add system for ticking local executors on main thread into bevy_core where the tasks pools are initialized.
- Add ticking local executors into thread executors
## Changelog
- tick all thread local executors in task pool.
## Notes
- ~~Not 100% sure about this PR. Ticking the local executor for the main thread in scope feels a little kludgy as it requires users of bevy_tasks to be calling scope periodically for those tasks to make progress.~~ took this out in favor of a system that ticks the local executors.
# Objective
Fixes https://github.com/bevyengine/bevy/issues/6306
## Solution
Change the failing assert and expand example to explain when ordering is deterministic or not.
Co-authored-by: Mike Hsu <mike.hsu@gmail.com>
# Objective
- Add ability to create nested spawns. This is needed for stageless. The current executor spawns tasks for each system early and runs the system by communicating through a channel. In stageless we want to spawn the task late, so that archetypes can be updated right before the task is run. The executor is run on a separate task, so this enables the scope to be passed to the spawned executor.
- Fixes#4301
## Solution
- Instantiate a single threaded executor on the scope and use that instead of the LocalExecutor. This allows the scope to be Send, but still able to spawn tasks onto the main thread the scope is run on. This works because while systems can access nonsend data. The systems themselves are Send. Because of this change we lose the ability to spawn nonsend tasks on the scope, but I don't think this is being used anywhere. Users would still be able to use spawn_local on TaskPools.
- Steals the lifetime tricks the `std:🧵:scope` uses to allow nested spawns, but disallow scope to be passed to tasks or threads not associated with the scope.
- Change the storage for the tasks to a `ConcurrentQueue`. This is to allow a &Scope to be passed for spawning instead of a &mut Scope. `ConcurrentQueue` was chosen because it was already in our dependency tree because `async_executor` depends on it.
- removed the optimizations for 0 and 1 spawned tasks. It did improve those cases, but made the cases of more than 1 task slower.
---
## Changelog
Add ability to nest spawns
```rust
fn main() {
let pool = TaskPool::new();
pool.scope(|scope| {
scope.spawn(async move {
// calling scope.spawn from an spawn task was not possible before
scope.spawn(async move {
// do something
});
});
})
}
```
## Migration Guide
If you were using explicit lifetimes and Passing Scope you'll need to specify two lifetimes now.
```rust
fn scoped_function<'scope>(scope: &mut Scope<'scope, ()>) {}
// should become
fn scoped_function<'scope>(scope: &Scope<'_, 'scope, ()>) {}
```
`scope.spawn_local` changed to `scope.spawn_on_scope` this should cover cases where you needed to run tasks on the local thread, but does not cover spawning Nonsend Futures.
## TODO
* [x] think real hard about all the lifetimes
* [x] add doc about what 'env and 'scope mean.
* [x] manually check that the single threaded task pool still works
* [x] Get updated perf numbers
* [x] check and make sure all the transmutes are necessary
* [x] move commented out test into a compile fail test
* [x] look through the tests for scope on std and see if I should add any more tests
Co-authored-by: Michael Hsu <myhsu@benjaminelectric.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
As of Rust 1.59, `std:🧵:available_parallelism` has been stabilized. As of Rust 1.61, the API matches `num_cpus::get` by properly handling Linux's cgroups and other sandboxing mechanisms.
As bevy does not have an established MSRV, we can replace `num_cpus` in `bevy_tasks` and reduce our dependency tree by one dep.
## Solution
Replace `num_cpus` with `std:🧵:available_parallelism`. Wrap it to have a fallback in the case it errors out and have it operate in the same manner as `num_cpus` did.
This however removes `physical_core_count` from the API, though we are currently not using it in any way in first-party crates.
---
## Changelog
Changed: `bevy_tasks::logical_core_count` -> `bevy_tasks::available_parallelism`.
Removed: `bevy_tasks::physical_core_count`.
## Migration Guide
`bevy_tasks::logical_core_count` and `bevy_tasks::physical_core_count` have been removed. `logical_core_count` has been replaced with `bevy_tasks::available_parallelism`, which works identically. If `bevy_tasks::physical_core_count` is required, the `num_cpus` crate can be used directly, as these two were just aliases for `num_cpus` APIs.
# Objective
Fixes Issue #6005.
## Solution
Replaced WorldQuery with ReadOnlyWorldQuery on F generic in Query filters and QueryState to restrict its trait bound.
## Migration Guide
Query filter (`F`) generics are now bound by `ReadOnlyWorldQuery`, rather than `WorldQuery`. If for some reason you were requesting `Query<&A, &mut B>`, please use `Query<&A, With<B>>` instead.
# Objective
- Added a bunch of backticks to things that should have them, like equations, abstract variable names,
- Changed all small x, y, and z to capitals X, Y, Z.
This might be more annoying than helpful; Feel free to refuse this PR.
Right now, a direct reference to the target TaskPool is required to launch tasks on the pools, despite the three newtyped pools (AsyncComputeTaskPool, ComputeTaskPool, and IoTaskPool) effectively acting as global instances. The need to pass a TaskPool reference adds notable friction to spawning subtasks within existing tasks. Possible use cases for this may include chaining tasks within the same pool like spawning separate send/receive I/O tasks after waiting on a network connection to be established, or allowing cross-pool dependent tasks like starting dependent multi-frame computations following a long I/O load.
Other task execution runtimes provide static access to spawning tasks (i.e. `tokio::spawn`), which is notably easier to use than the reference passing required by `bevy_tasks` right now.
This PR makes does the following:
* Adds `*TaskPool::init` which initializes a `OnceCell`'ed with a provided TaskPool. Failing if the pool has already been initialized.
* Adds `*TaskPool::get` which fetches the initialized global pool of the respective type or panics. This generally should not be an issue in normal Bevy use, as the pools are initialized before they are accessed.
* Updated default task pool initialization to either pull the global handles and save them as resources, or if they are already initialized, pull the a cloned global handle as the resource.
This should make it notably easier to build more complex task hierarchies for dependent tasks. It should also make writing bevy-adjacent, but not strictly bevy-only plugin crates easier, as the global pools ensure it's all running on the same threads.
One alternative considered is keeping a thread-local reference to the pool for all threads in each pool to enable the same `tokio::spawn` interface. This would spawn tasks on the same pool that a task is currently running in. However this potentially leads to potential footgun situations where long running blocking tasks run on `ComputeTaskPool`.
# Objective
- The single threaded task pool is not documented
- This doesn't warn in CI as it's feature gated for wasm, but I'm tired of seeing the warnings when building in wasm
## Solution
- Document it
# Objective
Fixes#1529
Run bevy_ecs in miri
## Solution
- Don't set thread names when running in miri rust-lang/miri/issues/1717
- Update `event-listener` to `2.5.2` as previous versions have UB that is detected by miri: [event-listener commit](1fa31c553e)
- Ignore memory leaks when running in miri as they are impossible to track down rust-lang/miri/issues/1481
- Make `table_add_remove_many` test less "many" because miri is really quite slow :)
- Make CI run `RUSTFLAGS="-Zrandomize-layout" MIRIFLAGS="-Zmiri-ignore-leaks -Zmiri-tag-raw-pointers -Zmiri-disable-isolation" cargo +nightly miri test -p bevy_ecs`
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
This PR is part of the issue #3492.
# Objective
- Add and update the bevy_tasks documentation to achieve a 100% documentation coverage (sans `prelude` module)
- Add the #![warn(missing_docs)] lint to keep the documentation coverage for the future.
## Solution
- Add and update the bevy_math documentation.
- Add the #![warn(missing_docs)] lint.
- Added doctest wherever there should be in the missing docs.
#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_tasks` crate.
Fixes#2566Fixes#3005
There are only READMEs in the 4 crates here (with the exception of bevy itself).
Those 4 crates are ecs, reflect, tasks, and transform.
These should each now include their respective README files.
Co-authored-by: Hoidigan <57080125+Hoidigan@users.noreply.github.com>
Co-authored-by: Daniel Nelsen <57080125+Hoidigan@users.noreply.github.com>
# Objective
- Document that the error codes will be rendered on the bevy website (see bevyengine/bevy-website#216)
- Some Cargo.toml files did not include the license or a description field
## Solution
- Readme for the errors crate
- Mark internal/development crates with `publish = false`
- Add missing license/descriptions to some crates
- [x] merge bevyengine/bevy-website#216
Objective
During work on #3009 I've found that not all jobs use actions-rs, and therefore, an previous version of Rust is used for them. So while compilation and other stuff can pass, checking markup and Android build may fail with compilation errors.
Solution
This PR adds `action-rs` for any job running cargo, and updates the edition to 2021.
This relicenses Bevy under the dual MIT or Apache-2.0 license. For rationale, see #2373.
* Changes the LICENSE file to describe the dual license. Moved the MIT license to docs/LICENSE-MIT. Added the Apache-2.0 license to docs/LICENSE-APACHE. I opted for this approach over dumping both license files at the root (the more common approach) for a number of reasons:
* Github links to the "first" license file (LICENSE-APACHE) in its license links (you can see this in the wgpu and rust-analyzer repos). People clicking these links might erroneously think that the apache license is the only option. Rust and Amethyst both use COPYRIGHT or COPYING files to solve this problem, but this creates more file noise (if you do everything at the root) and the naming feels way less intuitive.
* People have a reflex to look for a LICENSE file. By providing a single license file at the root, we make it easy for them to understand our licensing approach.
* I like keeping the root clean and noise free
* There is precedent for putting the apache and mit license text in sub folders (amethyst)
* Removed the `Copyright (c) 2020 Carter Anderson` copyright notice from the MIT license. I don't care about this attribution, it might make license compliance more difficult in some cases, and it didn't properly attribute other contributors. We shoudn't replace it with something like "Copyright (c) 2021 Bevy Contributors" because "Bevy Contributors" is not a legal entity. Instead, we just won't include the copyright line (which has precedent ... Rust also uses this approach).
* Updates crates to use the new "MIT OR Apache-2.0" license value
* Removes the old legion-transform license file from bevy_transform. bevy_transform has been its own, fully custom implementation for a long time and that license no longer applies.
* Added a License section to the main readme
* Updated our Bevy Plugin licensing guidelines.
As a follow-up we should update the website to properly describe the new license.
Closes#2373
This was tested using cargo generate-lockfile -Zminimal-versions.
The following indirect dependencies also have minimal version
dependencies. For at least num, rustc-serialize and rand this is
necessary to compile on rustc versions that are not older than 1.0.
* num = "0.1.27"
* rustc-serialize = "0.3.20"
* termcolor = "1.0.4"
* libudev-sys = "0.1.1"
* rand = "0.3.14"
* ab_glyph = "0.2.7
Based on https://github.com/bevyengine/bevy/pull/2455
# Objective
Reduce compilation time
# Solution
Remove unused dependencies. While this PR doesn't remove any crates from `Cargo.lock`, it may unlock more build parallelism.
# Objective
- CI jobs are starting to fail due to `clippy::bool-assert-comparison` and `clippy::single_component_path_imports` being triggered.
## Solution
- Fix all uses where `asset_eq!(<condition>, <bool>)` could be replace by `assert!`
- Move the `#[allow()]` for `single_component_path_imports` to `#![allow()]` at the start of the files.
This PR is easiest to review commit by commit.
Followup on https://github.com/bevyengine/bevy/pull/1309#issuecomment-767310084
- [x] Switch from a bash script to an xtask rust workspace member.
- Results in ~30s longer CI due to compilation of the xtask itself
- Enables Bevy contributors on any platform to run `cargo ci` to run linting -- if the default available Rust is the same version as on CI, then the command should give an identical result.
- [x] Use the xtask from official CI so there's only one place to update.
- [x] Bonus: Run clippy on the _entire_ workspace (existing CI setup was missing the `--workspace` flag
- [x] Clean up newly-exposed clippy errors
~#1388 builds on this to clean up newly discovered clippy errors -- I thought it might be nicer as a separate PR.~ Nope, merged it into this one so CI would pass.
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
I have run the VSCode Extension [markdownlint](https://marketplace.visualstudio.com/items?itemName=DavidAnson.vscode-markdownlint) on all Markdown Files in the Repo.
The provided Rules are documented here: https://github.com/DavidAnson/markdownlint/blob/v0.23.1/doc/Rules.md
Rules I didn't follow/fix:
* MD024/no-duplicate-heading
* Changelog: Here Heading will always repeat.
* Examples Readme: Platform-specific documentation should be symmetrical.
* MD025/single-title
* MD026/no-trailing-punctuation
* Caused by the ! in "Hello, World!".
* MD033/no-inline-html
* The plugins_guidlines file does need HTML, so the shown badges aren't downscaled too much.
* ~~MD036/no-emphasis-as-heading:~~
* ~~This Warning only Appears in the Github Issue Templates and can be ignored.~~
* ~~MD041/first-line-heading~~
* ~~Only appears in the Readme for the AlienCake example Assets, which is unimportant.~~
---
I also sorted the Examples in the Readme and Cargo.toml in this order/Priority:
* Topic/Folder
* Introductionary Examples
* Alphabetical Order
The explanation for each case, where it isn't Alphabetical :
* Diagnostics
* log_diagnostics: The usage of inbuild Diagnostics is more important than creating your own.
* ECS (Entity Component System)
* ecs_guide: The guide should be read, before diving into other Features.
* Reflection
* reflection: Basic Explanation should be read, before more advanced Topics.
* WASM Examples
* hello_wasm: It's "Hello, World!".
