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

Asset has LoadState::Loaded but Assets::get return None #2224

Closed
kgv opened this issue May 20, 2021 · 12 comments
Closed

Asset has LoadState::Loaded but Assets::get return None #2224

kgv opened this issue May 20, 2021 · 12 comments
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior

Comments

@kgv
Copy link

kgv commented May 20, 2021

Bevy version

0.5.0.

Operating system & version

Windows 10.

What you did

  1. Insert code before this line (using debug branch):
warn!(
    "resource_handle: {:?} {:?} {}",
    resource_handle,
    asset_server.get_load_state(resource_handle.clone()),
    resource_assets.get(resource_handle).is_some(),
);
  1. Run the ui example (cargo run --example=ui).

  2. If you manually click on the language change buttons, then everything is ok.
    But if you use a mouse macro (or something else) for a very fast click, then the example ends with an error.

  3. Log output:

WARN bevy_fluent::assets::localization: resource_handle: StrongHandle<ResourceAsset>(AssetPathId(AssetPathId(SourcePathId(17015584791912148938), LabelId(6298619649789039366)))) Loaded true
WARN bevy_fluent::assets::localization: resource_handle: StrongHandle<ResourceAsset>(AssetPathId(AssetPathId(SourcePathId(2475016618522967226), LabelId(6298619649789039366)))) Loaded true
WARN bevy_fluent::assets::localization: resource_handle: StrongHandle<ResourceAsset>(AssetPathId(AssetPathId(SourcePathId(17610828258912988862), LabelId(6298619649789039366)))) Loaded false
WARN bevy_fluent::assets::localization: resource_handle: StrongHandle<ResourceAsset>(AssetPathId(AssetPathId(SourcePathId(17872065453730569751), LabelId(6298619649789039366)))) Loaded false

You can see that the last two assets will have the status Loaded but get method return None.

What you expected to happen

For each asset that has LoadState::Loaded status, Assets.get always return Some.

What actually happened

You can see that some assets will have the status Loaded but get method return None.

@kgv kgv added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels May 20, 2021
@NathanSWard NathanSWard added A-Assets Load files from disk to use for things like images, models, and sounds and removed S-Needs-Triage This issue needs to be labelled labels May 20, 2021
@NathanSWard
Copy link
Contributor

NathanSWard commented May 20, 2021

There's also a chance that LoadState::Loaded gets set but the asset doesn't get inserted into the Assets collection until a later stage/next frame?

Edit: nvm it appears that the LoadState gets set after the asset is placed inside the Assets collection.

@kgv
Copy link
Author

kgv commented May 21, 2021

The #2226 PR makes changes:

Application goes into an endless loop when assets with dependencies already have a Loaded status, but their dependencies doesn't.

Below are top-level assets, for them we explicitly call the AssetServer::load method.

INFO bevy_fluent::systems::serve: bundle_handle: AssetPathId(AssetPathId(SourcePathId(10958097973059340271), LabelId(6298619649789039366))) Loaded
INFO bevy_fluent::systems::serve: bundle_handle: AssetPathId(AssetPathId(SourcePathId(4953421844741585986), LabelId(6298619649789039366))) Loaded
INFO bevy_fluent::systems::serve: bundle_handle: AssetPathId(AssetPathId(SourcePathId(11723345319984754560), LabelId(6298619649789039366))) Loaded
INFO bevy_fluent::systems::serve: bundle_handle: AssetPathId(AssetPathId(SourcePathId(7183645651604282493), LabelId(6298619649789039366))) Loaded

Below are dependent assets:

  • 17015584791912148938 is a dependence of 10958097973059340271,
  • 2475016618522967226 is a dependence of 4953421844741585986,
  • 17610828258912988862 is a dependence of 11723345319984754560,
  • 17872065453730569751 is a dependence of 7183645651604282493.
INFO bevy_fluent::systems::serve: resource_handle: AssetPathId(AssetPathId(SourcePathId(17015584791912148938), LabelId(6298619649789039366))) Loaded
INFO bevy_fluent::systems::serve: resource_handle: AssetPathId(AssetPathId(SourcePathId(2475016618522967226), LabelId(6298619649789039366))) Loaded
INFO bevy_fluent::systems::serve: resource_handle: AssetPathId(AssetPathId(SourcePathId(17610828258912988862), LabelId(6298619649789039366))) NotLoaded
INFO bevy_fluent::systems::serve: resource_handle: AssetPathId(AssetPathId(SourcePathId(17872065453730569751), LabelId(6298619649789039366))) NotLoaded

Some top-level assets have a Loaded status while it dependent assets have a NotLoaded status.

@NathanSWard
Copy link
Contributor

The #2226 PR makes changes:
Application goes into an endless loop when assets with dependencies already have a Loaded status, but their dependencies doesn't.

Thank you for trying this out! I was worried that something like this may happen.
I'll take a look at it again over the next few days and try to find a correct solution.

@mockersf
Copy link
Member

2. But if you use a mouse macro (or something else) for a very fast click, then the example ends with an error.

Is your macro only doing one click, or are you doing several to go through all languages?

@kgv
Copy link
Author

kgv commented May 22, 2021

Several to go through all languages. The same effect is achieved simply by frequently clicking on one of the buttons (but harder).

@mockersf
Copy link
Member

I could not reproduce with this change in bevy_fluent:

--- a/src/assets/localization.rs
+++ b/src/assets/localization.rs
@@ -25,6 +25,7 @@ use unic_langid::LanguageIdentifier;
 #[uuid = "981fc1ac-4748-4d09-b826-7cdcb7272a99"]
 pub struct Localization {
     bundles: Vec<FluentBundle<Arc<FluentResource>, IntlLangMemoizer>>,
+    handles: IndexSet<Handle<BundleAsset>>,
 }

 impl Localization {
@@ -102,7 +103,7 @@ impl Builder {
         let bundle_assets = world.get_resource::<Assets<BundleAsset>>().unwrap();
         let resource_assets = world.get_resource::<Assets<ResourceAsset>>().unwrap();
         let mut bundles = Vec::new();
-        for bundle_handle in self.handles {
+        for bundle_handle in self.handles.clone() {
             let bundle_asset = bundle_assets.get(bundle_handle).unwrap();
             let locales = bundle_asset.locale().into_iter().cloned().collect();
             let mut bundle = FluentBundle::new_concurrent(locales);
@@ -119,6 +120,9 @@ impl Builder {
             }
             bundles.push(bundle);
         }
-        Localization { bundles }
+        Localization {
+            bundles,
+            handles: self.handles,
+        }
     }
 }

This is probably a bug in Bevy asset around asset cleanup, but I'm not completely sure to follow how handles are used in bevy_fluent

@NiklasEi
Copy link
Member

I could not reproduce with this change in bevy_fluent:

So you could fix it just by keeping the handles around?

I have the same issue when running https://github.com/NiklasEi/bevy_asset_loader_playground with the feature broken and in this case, I am already keeping Handles around. I am also seeing some flakiness here. Sometimes the asset is there, but most of the time it's not.

asset_inconsistencies_cut.mp4

@mockersf
Copy link
Member

I have the same issue when running https://github.com/NiklasEi/bevy_asset_loader_playground with the feature broken and in this case, I am already keeping Handles around. I am also seeing some flakiness here. Sometimes the asset is there, but most of the time it's not.

I couldn't reproduce running cargo run --features broken 20 times

@NiklasEi
Copy link
Member

I couldn't reproduce running cargo run --features broken 20 times

Yeah, chances are quite small. I think I have roughly 1 in 20 runs where the asset correctly loads and renders.

@kgv
Copy link
Author

kgv commented May 24, 2021

I could not reproduce with this change in bevy_fluent

Yes, it fixes the error for me too.

The reason is as follows:

Localization requires Handle<BundleAsset> and Handle<ResourceAsset> only for its building.
Once Localization is created, we no longer need Handle<BundleAsset> and Handle<ResourceAsset>, and bevy decides when to delete them.
If we store Handle<BundleAsset> in Localization, then when changing localization to another, we do not need to load some assets (which is exactly the problem).
What you can see in the log:

WARN bevy_fluent::systems::serve: bundle_handle: AssetPathId(AssetPathId(SourcePathId(10958097973059340271), LabelId(6298619649789039366))) Loading
WARN bevy_fluent::systems::serve: bundle_handle: AssetPathId(AssetPathId(SourcePathId(4953421844741585986), LabelId(6298619649789039366))) Loading
WARN bevy_fluent::systems::serve: bundle_handle: AssetPathId(AssetPathId(SourcePathId(11723345319984754560), LabelId(6298619649789039366))) Loaded
WARN bevy_fluent::systems::serve: bundle_handle: AssetPathId(AssetPathId(SourcePathId(7183645651604282493), LabelId(6298619649789039366))) Loaded

We have just started loading 4 necessary assets for the new Localization, and the last 2 of them have already been loaded, since their handles are stored in the previous Localization.
Thus, this change simply avoids the situation where bevy has to release handles.

@NathanSWard
Copy link
Contributor

NathanSWard commented Jun 8, 2021

@kgv can you please try main?
I'm curious if #2226 changed this behavior at all.

@kgv
Copy link
Author

kgv commented Jun 9, 2021

This issue has been fixed by #2226.
The state of the dependent and main assets are consistent.
If at least one of the dependent assets has the LoadState::Unloaded status, then the main asset also has the LoadState::Unloaded status.

@kgv kgv closed this as completed Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

No branches or pull requests

4 participants