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

Migrated to bevy 0.9 #138

Merged
merged 8 commits into from
Nov 19, 2022
Merged

Migrated to bevy 0.9 #138

merged 8 commits into from
Nov 19, 2022

Conversation

John-Toohey
Copy link
Contributor

Does not apply to the platformer example (rapier2d) because we are waiting on dimforge/bevy_rapier#272. When that PR is merged I will update this.

Does not apply to the platformer example (rapier2d)
Copy link
Owner

@Trouv Trouv left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

src/resources.rs Outdated Show resolved Hide resolved
src/utils.rs Show resolved Hide resolved
@Trouv
Copy link
Owner

Trouv commented Nov 18, 2022

When that PR is merged I will update this.

Feel free to depend on that PR in main. I'm also down to update that example in a separate PR if it's a lot of changes.

@johanhelsing
Copy link
Contributor

I think

app.add_plugin(bevy_ecs_tilemap::TilemapPlugin)

should probably be removed from LdtkPlugin::build. If more than one plugins does this, the user will get panics like:

thread 'main' panicked at 'Error adding plugin bevy_ecs_tilemap::TilemapPlugin: : plugin was already added in application', C:\Users\Johan\.cargo\registry\src\github.com-1ecc6299db9ec823\bevy_app-0.9.0\src\app.rs:841:63
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Alternatively, put it behind a app.is_plugin_added check.

@johanhelsing
Copy link
Contributor

Not sure if it's this PR, or an existing bevy_ecs_ldtk issue, but I have problems with the background and tiles not lining up:

image

The white box is the background for the level with the sunnyland grass textures, so either the positions of the background or the tiles is off.

image

@Trouv
Copy link
Owner

Trouv commented Nov 18, 2022

I have problems with the background and tiles not lining up:

This should be fixed by #136, this PR hasn't pulled from main since that was merged

@John-Toohey
Copy link
Contributor Author

I think

app.add_plugin(bevy_ecs_tilemap::TilemapPlugin)

should probably be removed from LdtkPlugin::build. If more than one plugins does this, the user will get panics like:

thread 'main' panicked at 'Error adding plugin bevy_ecs_tilemap::TilemapPlugin: : plugin was already added in application', C:\Users\Johan\.cargo\registry\src\github.com-1ecc6299db9ec823\bevy_app-0.9.0\src\app.rs:841:63
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Alternatively, put it behind a app.is_plugin_added check.

Fixed in 90092d8

@John-Toohey John-Toohey marked this pull request as ready for review November 19, 2022 09:39
@John-Toohey
Copy link
Contributor Author

Rebased based of upstream

Copy link
Contributor

@johanhelsing johanhelsing left a comment

Choose a reason for hiding this comment

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

Alignment issues gone :)

No other issues

Cargo.toml Outdated Show resolved Hide resolved
@Trouv
Copy link
Owner

Trouv commented Nov 19, 2022

Pushing a formatting change...

Copy link
Owner

@Trouv Trouv left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for your hard work @greenfierydragon and @johanhelsing - I really appreciate it

@Trouv Trouv merged commit 048285c into Trouv:main Nov 19, 2022
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 this pull request may close these issues.

3 participants