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 scene example #11289

Merged
merged 1 commit into from Jan 22, 2024
Merged

Fix scene example #11289

merged 1 commit into from Jan 22, 2024

Conversation

irate-devil
Copy link
Contributor

@irate-devil irate-devil commented Jan 10, 2024

Since #9907 the generation starts at 1 instead of 0 so Entity::to_bits now returns 4294967296 (ie. u32::MAX + 1) as the lowest number instead of 0.

Without this change scene loading fails with this error message:
ERROR bevy_asset::server: Failed to load asset 'scenes/load_scene_example.scn.ron' with asset loader 'bevy_scene::scene_loader::SceneLoader': Could not parse RON: 8:6: Invalid generation bits

@irate-devil irate-devil added C-Bug An unexpected or incorrect behavior C-Examples An addition or correction to our examples A-Scenes Serialized ECS data stored on the disk labels Jan 10, 2024
@@ -5,7 +5,7 @@
),
},
entities: {
0: (
4294967296: (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use a random number and not 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not random :')
I've updated the first line in the PR description to be more clear.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, maybe add a comment so its more clear why that number is being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly the entire example is flawed and bound to be reworked when proper scene infrastructure rolls around, hopefully soon. Not sure if it's worth documenting at this point, you honestly just should not do serialization the way this example does, but I don't think deleting the example is an option

@alice-i-cecile alice-i-cecile added this to the 0.13 milestone Jan 10, 2024
@Vrixyz
Copy link
Member

Vrixyz commented Jan 11, 2024

If that fixes an example I'm all for it :D

That said ; isn't the root cause using to_bits()1 ? or something else ? in my opinion this complicates the ron format, which I'd like to make sense of easily, seeing these kind of "magic" numbers don't facilitate understanding.

Footnotes

  1. docs of to_bits states it shouldn't be used in serialization ? https://github.com/bevyengine/bevy/blob/main/crates/bevy_ecs/src/entity/mod.rs#L258-L259

@irate-devil
Copy link
Contributor Author

Yep. This entire way of serializing entities is terribly flawed, but the example exists as-is so it's better if it actually works.
Certified jank :')

@matiqo15 matiqo15 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 Jan 19, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 22, 2024
Merged via the queue into bevyengine:main with commit 1e7e6c9 Jan 22, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Scenes Serialized ECS data stored on the disk C-Bug An unexpected or incorrect behavior 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.

None yet

6 participants