-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Add better scene tests #6834
Comments
Also related, sprites don't currently save to scenes out of the box: #3746 (comment) Probably a good one to have a test for. |
If I recall, serializing and deserializing handles currently aren't well supported. I suspect that could cause issues when trying to incorporate sprites 🤔 |
Ah yeah you're right, I forgot I had that working using a custom AssetHandle component I replace handles with when saving scenes that loads the handle from a relative path when loaded. |
We can consider it, but it might be difficult to justify testing that something only partially works. On the other hand, though, if we make sure to leave a comment indicating this, it might not be a bad idea to just ensure general serialization/deserialization works on the other parts of the component (especially to test things like |
What problem does this solve or what need does it fill?
When 0.9 was initially released, there were some scene-related bugs that went unnoticed. These included:
DynamicScene
(de)serialization to postcard and bincode fails for some components. #6713SmallVec<[Entity; 8]>
#6578ReflectSerialize
andReflectDeserialize
registrations from most glam types #6580bevy_render
#6725Unfortunately, these were not rare edge cases that slipped through. These were just the result of people using the new scene format in actual projects.
The reason we didn't catch these bugs right away is that our current tests don't relate to real-world scenarios— or at least not a decent handful of them.
What solution would you like?
In the
tests
folder in the root of the project, we should include some tests for scenes. These tests should generally:Vec
/HashMap
usage, etc.)Additionally, they should all ensure that we can:
While we do want to have really complex scenes, we should also try to break things up so we can isolate particular areas of interest. So here are some of the test files I think we should have:
scenes/relations.rs
- Focus on hierarchies and basic relations (e.g.struct Pet(Entity)
).scenes/ui.rs
- Focus on (de)serializing UI (if possible).scenes/components.rs
- Focus on components of all shapes and sizes— both from Bevy and custom-made.These are not a requirement and there may be more tests (or fewer) that we want to add. The basic idea is to at least have enough that we can be confident changes to the scene format or other code won't be immediately broken at the next release.
What alternative(s) have you considered?
There are a couple alternatives we could try:
The reason why I don't think we should go with the first one is that these really need to be more like tests: slightly more complex, checking edge cases, doing multiple things, etc. I don't think examples are really meant for that and wouldn't really benefit those reading through it.
And the reason I don't think we should go with the second is that we're not really using the same API as users. Not only might there be differences in how the API is used, but we might be missing some types/components that we can't pull in without adding them as a dependency. We could maybe do it, but I think just handling it in the
tests
directory is going to be much simpler/more accurate.Additional context
Reference comment: #6580 (comment)
The text was updated successfully, but these errors were encountered: