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

refactor!: update to bevy 0.8 and bevy_ecs_tilemap rewrite #106

Merged
merged 41 commits into from Aug 14, 2022

Conversation

Trouv
Copy link
Owner

@Trouv Trouv commented Aug 10, 2022

Thanks to @nfagerlund for the help with the bevy 0.8 update and lots of troubleshooting.

There's a couple extra major changes here worth pointing out

  • no more batch spawning, bevy_ecs_tilemap doesn't really support it in the rewrite. This will probably allow us to simplify the level spawning code, but for now it works.
  • all tiles have SpatialBundles and are all children of layer entities. Before, this was only true for tiles with intgrid values and metadata. This helps with despawning levels, but it also makes the hierarchy a little less confusing. It does compromise some performance though, since potentially thousands of entities will now have components that they didn't have before.

Trouv and others added 30 commits July 28, 2022 22:57
* add TilePos and GridCoords to tiles
* add parenting to ldtk_entity
* do clone-fu for texture handles
…v-dep

Heron's not updated for 0.8 yet; ecs_tilemap got released, and the
rewrite branch deleted.
Cf. bevy_ecs_tilemap 6f08f100ba99e725d3326ef54ddc9d3f0da89876
- Can't construct `Parent` components anymore
- Can't directly access .0 field on a `Parent` component (use `.get()`)
- Can only assign parentage via commands

The `worldly_adoption` system might benefit from using
`EntityCommands.push_children()` instead of `add_children`, but for this
pass I'm trying to keep the logic changes minimal.
Cargo check blows up without this, and it's one of those things where
I'm like "wait why was it EVER working?"
As of Bevy 0.8, visibility gets propagated through the parent/child
hierarchy much like global transforms do.

Basically, whenever you would add Transform and GlobalTransform, you
probably want to add the whole SpatialBundle.
A little bit of help on the refactor/tilemap-rewrite branch
@neocturne
Copy link
Contributor

I'm currently seeing 3 issues when updating to this branch:

  • When spawning the LdtkWorldBundle as a child of another entity (for example to apply a Transform) nothing is displayed at all
  • When spawning the LdtkWorldBundle from a non-startup system (for example a state's on_enter), I get various error messages about missing entities, usually leading to a crash:
2022-08-11T21:33:44.334253Z ERROR bevy_ecs::system::commands: Failed to 'insert or spawn' bundle of type bevy_ecs_tilemap::render::extract::ExtractedTileBundle into the following invalid entities: [9v1, 7v1, 6v1, 3v1, 4v1]
2022-08-11T21:33:44.334279Z ERROR bevy_ecs::system::commands: Failed to 'insert or spawn' bundle of type bevy_ecs_tilemap::render::extract::ExtractedTilemapBundle into the following invalid entities: [10v1, 8v1]
2022-08-11T21:33:44.334289Z ERROR bevy_ecs::system::commands: Failed to 'insert or spawn' bundle of type bevy_ecs_tilemap::render::extract::ExtractedTilemapTextureBundle into the following invalid entities: [10v1, 8v1]
thread 'Compute Task Pool (3)' panicked at 'called `Result::unwrap()` on an `Err` value: NoSuchEntity(8v1)', /home/neoraider/.cargo/registry/src/github.com-1ecc6299db9ec823/bevy_ecs_tilemap-0.7.0/src/render/prepare.rs:82:55
  • When applying a scaling factor to the camera's OrthographicProjection, I'm seeing weird rendering issues at tile edges that weren't there with Bevy 0.7 (here shown with a file based on one of LDtk's example levels and a 4x scaling):

Before:
before
After:
after

@Trouv
Copy link
Owner Author

Trouv commented Aug 12, 2022

Thanks for testing it out @NeoRaider !

When spawning the LdtkWorldBundle as a child of another entity (for example to apply a Transform) nothing is displayed at all

I'm not able to reproduce this. Does your parent entity have a SpatialBundle? It needs one.

When spawning the LdtkWorldBundle from a non-startup system (for example a state's on_enter), I get various error messages about missing entities, usually leading to a crash

Seems to be the same issue as #107. Using bevy_ecs_tilemap on its own this way works perfectly fine so it's definitely an issue with bevy_ecs_ldtk. I'm guessing it has something to do with scheduling ambiguity between the two plugins. I'll look into it now.

When applying a scaling factor to the camera's OrthographicProjection, I'm seeing weird rendering issues at tile edges that weren't there with Bevy 0.7 (here shown with a file based on one of LDtk's example levels and a 4x scaling)

I'm also not able to reproduce this. Using the same example from LDtk, I'm changing the OrthographicProjection::scale and nothing's wrong on my end?

@neocturne
Copy link
Contributor

Thanks for testing it out @NeoRaider !

When spawning the LdtkWorldBundle as a child of another entity (for example to apply a Transform) nothing is displayed at all

I'm not able to reproduce this. Does your parent entity have a SpatialBundle? It needs one.

Thanks, that was the issue - I was only adding Transform/GlobalTransform instead of the whole bundle.

When spawning the LdtkWorldBundle from a non-startup system (for example a state's on_enter), I get various error messages about missing entities, usually leading to a crash

Seems to be the same issue as #107. Using bevy_ecs_tilemap on its own this way works perfectly fine so it's definitely an issue with bevy_ecs_ldtk. I'm guessing it has something to do with scheduling ambiguity between the two plugins. I'll look into it now.

When applying a scaling factor to the camera's OrthographicProjection, I'm seeing weird rendering issues at tile edges that weren't there with Bevy 0.7 (here shown with a file based on one of LDtk's example levels and a 4x scaling)

I'm also not able to reproduce this. Using the same example from LDtk, I'm changing the OrthographicProjection::scale and nothing's wrong on my end?

I'll try to reduce my code to a minimal reproducer.

@Trouv
Copy link
Owner Author

Trouv commented Aug 12, 2022

Closes #91
Closes #107

@Trouv
Copy link
Owner Author

Trouv commented Aug 12, 2022

There seems to be a similar issue w/ hot reloading

@neocturne
Copy link
Contributor

My rendering issue turned out to be bevyengine/bevy#5266. Inserting .insert_resource(ImageSettings::default_nearest()) before DefaultPlugins fixed it.

@Trouv
Copy link
Owner Author

Trouv commented Aug 14, 2022

I've promoted the Hot reloading bug to its own issue, so I'm going to merge this PR. I'll make another PR for that fix when it's ready

@Trouv Trouv merged commit 1d9f0a4 into main Aug 14, 2022
@Trouv Trouv deleted the refactor/tilemap-rewrite branch August 26, 2023 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants