bevy 0.15

[alxgnon]

I've been watching progress for bevy_ecs_tilemap's migration to 0.15. It isn't quite ready yet but I decided to start making the required changes for bevy 0.15 in this repository.

For this branch, The examples aren't quite fixed yet. Also I'd like to fix up the LdtkProjectHandle thing a bit better. But the base library compiles so I'd say that's a good start.

[neocturne]

@alxgnon Nice work! I've done some improvements based on your branch: https://github.com/neocturne/bevy_ecs_ldtk/tree/bevy-0.15 and got all doctests and examples to run (although I haven't tested the examples very thoroughly so far)

TODOs I'm aware of:

• rapier2d currently uses a temporary PR branch, must be updated once Update bevy to 0.15 dimforge/bevy_rapier#595 is merged
• Needs doc updates and migration guide
• Evaluate moving from Bundles to more idiomatic required components (probably better left for a future PR)

[undrscor]

Hello, im a rookie rust developer here, I was playing around with bevy_eck_ldtk to make a small 2d platformer game and took a small break, recently came back to to it only to find a new version of Bevy (0.15), with a bunch of new errors and issues in my code. Is using bevy 0.15 viable with the current released version of bevy_eck_ldtk? Obviously ill need to do a bunch of migration actions, but I want to make sure im not doing them in vain if this library wont work at the moment anyways. Otherwise ill simply roll back to bevy 0.14 and continue from there, since I do have a deadline for this game. Thank you in advance, and also im a huge fan of this project, its been super fun and useful!

[neocturne]

@undrscor At the moment, Bevy is still in an "early development" state, where each major release is a breaking change, and all plugins need to be updated to be compatible with the new version (and become incompatible with older versions). The currently released version of bevy_ecs_ldtk is for Bevy 0.14; this PR will add support for Bevy 0.15 when it's done.

[undrscor]

@neocturne Thank you for the quick response! I'm really excited to see support for Bevy 0.15 being added.

I'm just starting out with both Rust and game development, so I'm still a rookie, but I'd love the opportunity to contribute to this project if that's at all possible. I'm not experienced with a lot of concepts applied in this, but I would be happy to try to understand and help with any smaller tasks that you feel like sharing.

[alxgnon]

@neocturne I went ahead and included your commits in the PR. I'm still unsure about what I did with LdtkProjectHandle. Perhaps there's a cleaner way but I don't know. I'd love to hear your thoughts! Otherwise, I'm pretty happy with this! Once bevy_ecs_tilemap releases in 0.15 I hope we can move towards getting this merged!

[neocturne]

Regarding the handle, the current newtype solution seems fine - I've also seen the pattern in other plugins. Adding a derive for Deref would allow to revert a lot of the code changes where the handle field is accessed. With Required Components, the type might be called LdtkWorld instead of LdtkProjectHandle, as it could replace the LdtkWorldBundle as the "driving concept" component.

An alternative would be to merge LdtkProjectHandle and LevelSet into a single component, but that seems less convenient to use when you don't want to interact with the LevelSet directly.

Another thing I was wondering about is whether the #[sprite_sheet] attribute should be merged into #[sprite], instead distinguishing the cases by a new argument, as both apply to a Sprite component now.

It would be good to get @Trouv's opinion on these questions, as they are the maintainer (and we should get them resolved before updating the book / writing the migration guide).

[alxgnon]

I managed to get the Deref coercion working in my latest commit. I was able to revert the changes where the project handle is used. No more handle.handle!

[JosefKuchar]

Bevy 0.15 support for bevy_ecs_tilemap was merged. Is this is ready?

[alxgnon]

I replaced my fork of bevy_ecs_tilemap with the 0.15 release version

[alxgnon]

@JosefKuchar All that's left is for @Trouv to get back to us and to update the documentation once everything's decided. Though you can probably just use my branch as a dependency in the meantime.

[Trouv]

Thanks for your great efforts on this, everyone. I'm finally catching up

With Required Components, the type might be called LdtkWorld instead of LdtkProjectHandle, as it could replace the LdtkWorldBundle as the "driving concept" component.

I like the idea of embracing Required Components here. I'm okay with the name remaining LdtkProjectHandle though, as the concept of a World has been adjusted in LDtk since I originally named it (now technically multiple ldtk worlds could be spawned with a single world bundle).

An alternative would be to merge LdtkProjectHandle and LevelSet into a single component, but that seems less convenient to use when you don't want to interact with the LevelSet directly.

My instinct here is to keep them separate. In my imagination, most use cases mutating the level set will not be mutating the handle and vice versa.

Another thing I was wondering about is whether the #[sprite_sheet] attribute should be merged into #[sprite], instead distinguishing the cases by a new argument, as both apply to a Sprite component now.

Actually, in the past I've gone so far to consider deprecating the #[sprite] entirely. I think you're right that the correct design that keeps both macro behaviors would combine them. Something like like #[sprite(no_atlas)]? However, I'm okay merging and releasing a version that doesn't have this improvement to get bevy 0.15 support out there sooner

[alxgnon]

I like the idea of embracing Required Components here.

I'm not sure how to do this, since Handles cannot be used as components in Bevy 0.15 onward.

I'll go ahead and update the docs and create a migration guide so we can get this merged.

[Trouv]

I'm not sure how to do this, since Handles cannot be used as components in Bevy 0.15 onward

What about the new type LdtkProjectHandle?

[alxgnon]

Oh yeah, I suppose LdtkWorldBundle could have its components as required components instead of struct fields, though maybe that would be for a different pull request.

[Trouv]

Yeah that's fine. Happy to get something working for 0.15 released and make subsequent improvements to it

[alxgnon]

@Trouv I've updated the existing documentation and I added a migration guide for the new version.

[alxgnon]

@neocturne I figured you'd like to have a look as well, since I documented your changes in the migration guide, too.

[neocturne]

I think the doc changes and migration guide look good, I only have a few nits.

[alxgnon]

@neocturne I pushed some changes to resolve the points you brought up.(!)

[alxgnon]

@Trouv This is probably ready to get merged & released as 0.11 (unless you have anything else to add)

[Trouv]

Ran the CI and there is some clippy and mdbook failures (apologies that it doesn't actually mark this job as red when it fails). I think many of the code examples in the old migration guides need to be marked ignore instead of no_run. I should have just marked them all ignore in the first place after I wrote them and verified they worked.

Otherwise, code looks good.

[alxgnon]

Alright, I added the documentation you wrote. I also added the changes clippy was asking for. I didn't see any failures on the mdbook side, at least not in the CI run results.

[alxgnon]

@Trouv Okay I see what you mean now by the mdbook errors. I totally glanced over them since it was green. I added rust,ignores where there were test failures, and got mdbook to run successfully.

[alxgnon]

@Trouv Okay, should be good, now.