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.

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

Fix minor issues with custom_asset example #10337

Merged
merged 1 commit into from
Nov 14, 2023

Conversation

rparrett
Copy link
Contributor

@rparrett rparrett commented Nov 1, 2023

Objective

  • Use bevy's re-exported AsyncReadExt so users don't think they need to depend on futures-lite.
  • Fix a funky error text

@rparrett rparrett added C-Examples An addition or correction to our examples C-Code-Quality A section of code that is hard to understand or change labels Nov 1, 2023
@Selene-Amanita
Copy link
Member

In the same vein, even if Reader https://docs.rs/bevy/latest/bevy/asset/io/type.Reader.html exists, it is not directly used in the signature of AssetLoader's load: https://docs.rs/bevy/latest/bevy/asset/trait.AssetLoader.html and since AsyncRead is not reexported it might lead people to depend on futures-io.

@rparrett
Copy link
Contributor Author

rparrett commented Nov 7, 2023

it is not directly used in the signature of AssetLoader's load:

I may just be confused (if so, please help me out), but I think it is?

reader: &'a mut Reader,

And in the custom_asset example:
reader: &'a mut Reader,

@Selene-Amanita
Copy link
Member

Selene-Amanita commented Nov 7, 2023

Ah, I guess it's rustdoc that expands it then :(

Maybe reexporting AsyncRead would be better?

@rparrett rparrett 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 Nov 7, 2023
@rparrett rparrett added this to the 0.12.1 milestone Nov 7, 2023
@rafalh
Copy link
Contributor

rafalh commented Nov 11, 2023

I had to add futures-io (AsyncRead) and futures_lite ( Stream) to dependencies in order to implement custom AssetReader + import futures_lite::StreamExt to iterate on result from read_directory. It would be nice if all those types could be reexported

@cart cart added this pull request to the merge queue Nov 14, 2023
Merged via the queue into bevyengine:main with commit fe25fc6 Nov 14, 2023
25 checks passed
cart pushed a commit that referenced this pull request Nov 30, 2023
# Objective

- Use bevy's re-exported `AsyncReadExt` so users don't think they need
to depend on `futures-lite`.
- Fix a funky  error text
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
# Objective

- Use bevy's re-exported `AsyncReadExt` so users don't think they need
to depend on `futures-lite`.
- Fix a funky  error text
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Code-Quality A section of code that is hard to understand or change C-Examples An addition or correction to our examples 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
Development

Successfully merging this pull request may close these issues.

6 participants