bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
35.38k stars 3.49k forks source link

Platform independent wrappers for `TaskPool` and `Scope` #4304

Open MiniaczQ opened 2 years ago

MiniaczQ commented 2 years ago

What problem does this solve or what need does it fill?

Currently TaskPool and Scope (from bevy_tasks) are implemented completely separately for wasm and non-wasm platforms, the correct ones are selected during re-export of the module. This means that their documentation depends on the platform in use, but in practice the wasm one was unattended for more than a year. It also means that mismatch in implementations will leak out into other creates rather than be contained within bevy_tasks.

Another reason is that tests can be centralized. Currently only non-wasm has tests.

What solution would you like?

As suggested by @TheRawMeatball, we should create wrappers for both types that contain all the user-oriented documentation, while the hidden platform specific ones can be dev-oriented.

What alternative(s) have you considered?

I originally suggested creating traits for both types. Both methods allow us to scope implementation problems inside the bevy_tasks crate, instead of leaking them outside, as well as centralize user-oriented documentation, but traits create a weird dependency chain of: platform independent trait -> platform dependent impl -> platform independent usage while the wrapper way provides us with: platform dependent impl -> platform independent wrapper -> platform independent usage which is much more straightforward.

Questions

Assuming we keep the names for the wrappers, what would the platform specific types be called?

More context

Ideally we wanna get this done before #4078 comes out with PR for wasm tasks.

MiniaczQ commented 2 years ago

I should mention, this is quite the opposite case to #4286, where the Task was already platform independent, yet had a wrapper.

MiniaczQ commented 2 years ago

this is a problem wasm implementation just straight up hardcoded the return type to fit the single use case of asset server Trying to return an actual Task that is unrelated to the closure is completely wrong, so I'll fix this by: