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

bevy_gltf_components sometimes does not do anything #111

Closed
janhohenheim opened this issue Feb 3, 2024 · 20 comments · Fixed by #134
Closed

bevy_gltf_components sometimes does not do anything #111

janhohenheim opened this issue Feb 3, 2024 · 20 comments · Fixed by #134

Comments

@janhohenheim
Copy link
Contributor

janhohenheim commented Feb 3, 2024

See Foxtrot at exactly this commit. When starting the game, every now and then (about 1/3), the loading screen stays up forever. Press G to open the editor_pls window and you'll see in the hierarchy that no entity has their components that should be inserted by bevy_gltf_components.

I hotfixed this by using my own little implementation here and it works every time.

@kaosat-dev
Copy link
Owner

kaosat-dev commented Feb 3, 2024

Thanks for the bug report @janhohenheim !
Very weird, I had that issue back in Bevy 0.11, have not had it popup since (then again, I use bevy_gltf_blueprints + bevy_asset_loader usually, and these issues seem related to timing issues with assets). I'll investigate.
Btw, I really like your use of querying for gltf_extras !
I have two big PRs coming up for this repo that are almost done, I will take a look at this right afterwards.

@janhohenheim
Copy link
Contributor Author

janhohenheim commented Feb 3, 2024

Thanks! :)

@kaosat-dev
Copy link
Owner

Btw one of the PRs drops a lot of the boilerplate present in bevy_gltf_components (soft depreciation, to make it easier for existing users), so I am very tempted to change things up to use your gltf_extras approach as it gets rid of some of the issues of loading scenes vs loading gltf files.
(When I saw your code I went "d'oh why did I not think of that" 😅)

@janhohenheim
Copy link
Contributor Author

janhohenheim commented Feb 4, 2024

hehehe :D Glad to help, I wouldn't have been able to write that if I wasn't able to read your code to begin with. I had no idea how to insert reflect-deserialized components!
Btw, a little caveat of the way I deserialize is that the structs in question need to reflect(Deserialize). Do you happen to know if that's because I use a TypedReflectDeserializer?

@kaosat-dev
Copy link
Owner

Hehe mutually beneficial , open source ftw :)
It has been a long time since I wrote that part, but from what I remember (take this with a pinch of salt or two), I seem to recall I had the same issue , and that is why I moved to UntypedReflectDeserializer but my brain is in Python / Blender mode today , so no guarantees

@kaosat-dev
Copy link
Owner

Hi @janhohenheim !
Just wanted to let you know, I'll experiment in the next few days with a hybrid solution using your idea of "gltf_extras" to see if I can use it as a replacement without breaking the rest (I see no reason why it should not work). I'll ping you once I have something concrete.
I have not been able to reproduce the issue reliably, (I had it pop up once in around 15 tries), but a solution that sidesteps the "load whole gltf" vs "load a scene" issue is an improvement in my book :)

@janhohenheim
Copy link
Contributor Author

janhohenheim commented Feb 9, 2024

Sounds good! :) I can also try out the change once you've got them on a branch since I can fairly reliably reproduce the issue (1/3 times)

@kaosat-dev
Copy link
Owner

Hi @janhohenheim ! Just a short update, finally finished the PR I had going on, so I am switching back to this issue, the use of gltfextras as you did seems completely usable !
I'll fire up a PR tomorrow for you to test out since you have the error reliably :)

@kaosat-dev
Copy link
Owner

@janhohenheim , went faster than expected, still wip as I need to double check a few things, but the PR is here , would you mind trying it out to see if it solves your issues ?

@janhohenheim
Copy link
Contributor Author

@kaosat-dev I just tried to test it, but Cargo is telling me that it can't find the crate when specified like this:

bevy_gltf_components = { git = "https://github.com/kaosat-dev/Blender_bevy_components_workflow", branch = "bevy_gltf_components_gltf_extras" }

do you perhaps see my mistake?

@kaosat-dev
Copy link
Owner

@janhohenheim I think it might be the missing ".git" at the end of the url:
ie , this should work ?
bevy_gltf_components = { git = "https://github.com/kaosat-dev/Blender_bevy_components_workflow.git", branch = "bevy_gltf_components_gltf_extras" }

@janhohenheim
Copy link
Contributor Author

Nope:
image

@kaosat-dev
Copy link
Owner

hmmm.... @GitGhillie , sorry to tag you, but how where you able to try the branch out ? Thanks !

@GitGhillie
Copy link
Contributor

GitGhillie commented Feb 19, 2024

No problem! If I can believe the stackoverflow post I saw it should just work but I ran into the same issue :(
So what I did:

  1. Clone this repo and switch to the branch
  2. Then use path: bevy_gltf_components = { path = "../Blender_bevy_components_workflow/crates/bevy_gltf_components"}
  3. It will complain about something linting related, so I temporarily removed all the linting stuff from the Cargo.toml in the workspace root and in the Cargo.toml of bevy_gltf_components

@janhohenheim
Copy link
Contributor Author

@GitGhillie I'll try that then, thanks!
@kaosat-dev sounds like somewhere in the repo some misconfiguration happened :/

@janhohenheim
Copy link
Contributor Author

@kaosat-dev found the issue! The lints issue that @GitGhillie ran into is the reason why the crate resolution fails.
Seems like the following is not valid:

[lints]
workspace = true

because after this commit, my crate resolution works!

@janhohenheim
Copy link
Contributor Author

janhohenheim commented Feb 19, 2024

Alright, found the actual mistake, sending a PR.
Edit: plz merge

@kaosat-dev
Copy link
Owner

Thanks a lot @GitGhillie & @janhohenheim ! :)
@janhohenheim, I merged your PR onto main & onto the branch for this issue, should be good to go for testing :)

@janhohenheim
Copy link
Contributor Author

@kaosat-dev can confirm that the branch in question fixes the bug 🎉 everything loads perfectly every time!

@kaosat-dev
Copy link
Owner

Awesome @janhohenheim ! Merging now :)
v0.3.2 of bevy_gltf_components is also out on crates.io

Thanks a lot for your help, much appreciated :) (added you to the list of contributors)

kaosat-dev added a commit that referenced this issue Feb 19, 2024
…ltf extras (#134)

 * not using / tracking gltf files anymore (should avoid issues with scenes vs gltf files)
 * restructured accordingly
 * closes #102
 * closes #111
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants