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

feat!: upgrade to bevy 0.12 #265

Merged
merged 14 commits into from
Jan 30, 2024
Merged

feat!: upgrade to bevy 0.12 #265

merged 14 commits into from
Jan 30, 2024

Conversation

Trouv
Copy link
Owner

@Trouv Trouv commented Nov 14, 2023

Closes #259
Also, I think updating to 0.12 implicitly...
Closes #233
Closes #111

This PR upgrades the bevy dependency to 0.12, as well as bevy_ecs_tilemap to 0.12. For dev-dependencies, it also upgrades bevy_rapier2d to 0.23, and bevy-ecs-egui to 0.21. Most other changes were just necessary to get the lib/tests/doc-tests/examples to compile.

This was a little more complicated for ldtk_external_level.rs and ldtk_project.rs since they are updating to use bevy assets v2. However, using assets v2 did help improve these modules slightly. In particular, it was easier to add dependent assets to the load context, and having an associated type for the error allowed me to remove the dependency on anyhow entirely.

Another slightly more difficult update was the process_ldtk_assets system. However, I'm quite happy with the distinction betwen AssetEvent::Added and AssetEvent::LoadedWithDependencies. bevy_ecs_ldtk now waits for the latter before spawning levels, which is functionally what it meant to do originally.

I've verified that all the examples are visually OK, and also checked out some visual edge cases. Any visual bugs I noticed aren't new.

Will keep this as a draft until bevy_ecs_tilemap 0.12 releases.

@Trouv Trouv changed the title feat: upgrade to bevy 0.12 feat!: upgrade to bevy 0.12 Nov 14, 2023
@Twyker
Copy link

Twyker commented Nov 22, 2023

any chance to make this work with the most current PR branch for the 0.12 fixes on bevy_ecs_tilemap side?
tried it like this but sadly didn't work :x
image

@Trouv
Copy link
Owner Author

Trouv commented Nov 23, 2023

Try depending on bevy_ecs_ldtk version 0.8, and using these branches as a patch rather than as deps:

[patch.crates-io]
bevy_ecs_ldtk = { git = "https://github.com/trouv/bevy_ecs_ldtk", branch = "feat/bevy-0.12" }
bevy_ecs_tilemap = { git = "https://github.com/divark/bevy_ecs_tilemap", branch = "0.12-fixes" }

@Trouv
Copy link
Owner Author

Trouv commented Nov 23, 2023

Also - for those looking to upgrade early, be aware that bevy_ecs_ldtk has had a lot of breaking changes. There's a migration guide for this - it renders better in Mdbook than in github but here it is:
https://github.com/Trouv/bevy_ecs_ldtk/blob/5feb5cc3452e0b7898db8348e0ebcea890c63e15/book/src/how-to-guides/migration-guides/migrate-from-0.8-to-0.9.md

@musjj
Copy link

musjj commented Nov 24, 2023

I'm getting a crash when using a RenderTarget on the camera. It worked just fine when I was at 0.11.

thread '<unnamed>' panicked at $HOME/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.17.2/src/backend/direct.rs:3056:5:
wgpu error: Validation Error

Caused by:
    In a RenderPass
      note: encoder = `<CommandBuffer-(0, 4, Vulkan)>`
    In a set_bind_group command
      note: bind group = `tilemap_view_bind_group`
    Dynamic binding offset index 0 with offset 2304 would overrun the buffer bound to bind group 0 -> binding 0. Buffer size is 1536 bytes, the binding binds bytes 0..608, meaning the maximum the binding can be offset is 928 bytes

Any clue why this is happening?

@Trouv
Copy link
Owner Author

Trouv commented Nov 24, 2023

I'm not sure, does it happen when using a RenderTarget w/ bevy_ecs_tilemap alone?

@musjj
Copy link

musjj commented Nov 24, 2023

It's a really bizzare issue. I made a reproduction crate here: https://github.com/musjj/tilemap_bug

The crash is basically caused by this snippet:

fn add_camera(
    camera: Query<&CanvasCamera>,
    mut commands: Commands,
    level_query: Query<&LevelIid>,
    canvas_query: Query<&Handle<Image>, With<Canvas>>,
) {
    if let Ok(iid) = level_query.get_single() {
        if camera.is_empty() {
            let canvas = canvas_query.single();
            commands.spawn((
                Camera2dBundle {
                    camera: Camera {
                        order: -1,
                        target: RenderTarget::Image(canvas.clone()),
                        ..default()
                    },
                    ..default()
                },
                CanvasCamera,
            ));
        }
    }
}

Changing level_query: Query<&LevelIid> to level_query: Query<&FooBar> (or any other queries) stops it from crashing. So I'm guessing that holding a reference to LevelIid is causing the issue here. But I don't understand why. Maybe some unsafe shenanigans?

Can you try to reproduce it?

@musjj
Copy link

musjj commented Nov 25, 2023

Another thing: it's not the &LevelIid query itself that is causing the crash. You can put the camera spawn logic outside of the scope of &LevelIid and it will not crash. This is some crazy black magic stuff...

@Trouv
Copy link
Owner Author

Trouv commented Nov 25, 2023

Your project crashes for me too. Let me try to see if it happens w/ just bevy_ecs_tilemap

@Trouv
Copy link
Owner Author

Trouv commented Nov 26, 2023

Ok - after a lot of testing, I believe this is caused by spawning the camera w/ the custom render target late. I was able to recreate this on a bevy_ecs_tilemap not by querying for their components, but by waiting until frame 10 to spawn the camera:

fn add_camera(
    camera: Query<&CanvasCamera>,
    mut commands: Commands,
    mut frame_count: Local<usize>,
    canvas_query: Query<&Handle<Image>, With<Canvas>>,
) {
    if *frame_count > 10 {
        if camera.is_empty() {
            let canvas = canvas_query.single();
            println!("spawning rendertarget camera");
            commands.spawn((
                Camera2dBundle {
                    camera: Camera {
                        order: -1,
                        target: RenderTarget::Image(canvas.clone()),
                        ..default()
                    },
                    ..default()
                },
                CanvasCamera,
            ));
        }
    } else {
        *frame_count += 1;
    }
}

I'll comment on the bevy_ecs_tilemap PR. Thanks for bringing this to my attention.

@cscorley
Copy link

Thanks for all your hard work on this plugin!

Got to test this out, and for the most part, everything works! Only thing I noticed that was different is the level layers higher than my main sprites. My solution to this was to set the LdtkWorldBundle to have a transform Transform::from_xyz(0., 0., -0.1)

my current lock for these packages:

[[package]]
name = "bevy_ecs_ldtk"
version = "0.8.0"
source = "git+https://github.com/trouv/bevy_ecs_ldtk?branch=feat/bevy-0.12#5feb5cc3452e0b7898db8348e0ebcea890c63e15"
dependencies = [
 "bevy",
 "bevy_ecs_ldtk_macros",
 "bevy_ecs_tilemap",
 "derive-getters",
 "derive_more",
 "hex",
 "paste",
 "path-clean",
 "regex",
 "serde",
 "serde_json",
 "thiserror",
]

[[package]]
name = "bevy_ecs_ldtk_macros"
version = "0.8.0"
source = "git+https://github.com/trouv/bevy_ecs_ldtk?branch=feat/bevy-0.12#5feb5cc3452e0b7898db8348e0ebcea890c63e15"
dependencies = [
 "proc-macro2",
 "quote",
 "syn 1.0.109",
]

[[package]]
name = "bevy_ecs_tilemap"
version = "0.12.0"
source = "git+https://github.com/divark/bevy_ecs_tilemap?branch=0.12-fixes#50718d6ec60ff29abc0496446a4a1614d1a779fe"
dependencies = [
 "bevy",
 "log",
 "regex",
]

@Trouv
Copy link
Owner Author

Trouv commented Nov 26, 2023

FYI @musjj a fix has been merged to the bevy_ecs_tilemap PR that at least stops the crash. It looks like they're running into other issues as a result though.

@Trouv
Copy link
Owner Author

Trouv commented Nov 26, 2023

Got to test this out, and for the most part, everything works! Only thing I noticed that was different is the level layers higher than my main sprites. My solution to this was to set the LdtkWorldBundle to have a transform Transform::from_xyz(0., 0., -0.1)

Huh. I just tested on the new commit they pushed, and it sounds similar to the new bugs: StarArawn/bevy_ecs_tilemap#489 (comment)

However, your lockfile suggests you're using the previous commit, not the new one.

How are you spawning your camera?

@Trouv
Copy link
Owner Author

Trouv commented Nov 26, 2023

FYI all - the migration guide is now rendered here: https://trouv.github.io/bevy_ecs_ldtk/how-to-guides/migration-guides/migrate-from-0.8-to-0.9.html

@cscorley
Copy link

Got to test this out, and for the most part, everything works! Only thing I noticed that was different is the level layers higher than my main sprites. My solution to this was to set the LdtkWorldBundle to have a transform Transform::from_xyz(0., 0., -0.1)

Huh. I just tested on the new commit they pushed, and it sounds similar to the new bugs: StarArawn/bevy_ecs_tilemap#489 (comment)

However, your lockfile suggests you're using the previous commit, not the new one.

How are you spawning your camera?

I just tested this again (lockfile to follow) and still have the same issue.

The camera spawns with Camera2dBundle::default() only. I also have this system to make sure the whole level fits the window at all times. However, even with this system disabled, things are off. I'm honestly fine with the -0.1, but wanted it to be known.

It definitely seems like the new bug (StarArawn/bevy_ecs_tilemap#489 (comment)).

pub fn fit_level(
    mut camera_query: Query<(&mut OrthographicProjection, &mut Transform)>,
    current_level: Option<Res<CurrentLevel>>,
) {
    if current_level.is_none() {
        return;
    }

    let (mut orthographic_projection, mut camera_transform) = camera_query.single_mut();
    let current_level = current_level.unwrap();

    let width = current_level.pixel_dimensions.x;
    let height = current_level.pixel_dimensions.y;

    camera_transform.translation.x = width / 2.;
    camera_transform.translation.y = height / 2.;

    orthographic_projection.scaling_mode = bevy::render::camera::ScalingMode::AutoMin {
        min_width: width,
        min_height: height,
    };
    orthographic_projection.area = Rect::new(0., 0., width, height);
}

image

[[package]]
name = "bevy_ecs_ldtk"
version = "0.8.0"
source = "git+https://github.com/trouv/bevy_ecs_ldtk?branch=feat/bevy-0.12#5feb5cc3452e0b7898db8348e0ebcea890c63e15"
dependencies = [
 "bevy",
 "bevy_ecs_ldtk_macros",
 "bevy_ecs_tilemap",
 "derive-getters",
 "derive_more",
 "hex",
 "paste",
 "path-clean",
 "regex",
 "serde",
 "serde_json",
 "thiserror",
]

[[package]]
name = "bevy_ecs_ldtk_macros"
version = "0.8.0"
source = "git+https://github.com/trouv/bevy_ecs_ldtk?branch=feat/bevy-0.12#5feb5cc3452e0b7898db8348e0ebcea890c63e15"
dependencies = [
 "proc-macro2",
 "quote",
 "syn 1.0.109",
]

[[package]]
name = "bevy_ecs_tilemap"
version = "0.12.0"
source = "git+https://github.com/divark/bevy_ecs_tilemap?branch=0.12-fixes#dbc6b17365434bd0194d82a5426ce0561829e628"
dependencies = [
 "bevy",
 "log",
 "regex",
]

@Trouv
Copy link
Owner Author

Trouv commented Dec 2, 2023

Made a post about using this for bevy jam 4, it reiterates the patch you need to use: #273 (comment)

@Trouv
Copy link
Owner Author

Trouv commented Dec 13, 2023

@MScottMcBee
Copy link
Contributor

Just tested, #232 still exists on this branch. I'll look into it and update the ticket when I know more

@Trouv
Copy link
Owner Author

Trouv commented Jan 18, 2024

FYI, just pulled some changes into this branch for updating to LDtk 1.5.3. Support for previous versions is being dropped

@Trouv Trouv marked this pull request as ready for review January 30, 2024 06:41
@Trouv
Copy link
Owner Author

Trouv commented Jan 30, 2024

I've decided to merge this into main to try and simplify this repo a bit. So, users may need to change their patches accordingly

@Trouv Trouv merged commit 194731e into main Jan 30, 2024
5 checks passed
@Trouv Trouv mentioned this pull request Jan 30, 2024
Trouv added a commit that referenced this pull request Feb 11, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.9.0](v0.8.0...v0.9.0)
(2024-02-11)


### ⚠ BREAKING CHANGES

* upgrade to bevy 0.12
([#265](#265))
* upgrade to LDtk 1.5.3, dropping support for previous versions
([#295](#295))
* add `SpawnExclusions` to `LdtkSettings` for skipping layers by
identifier ([#275](#275))
* add layer entity for Entity layers, changing the hierarchy
([#257](#257))
* upgrade to LDtk types and examples to 1.4.1 (drop support for <1.4.1)
([#256](#256))
* LdtkLevel renamed to LdtkExternalLevel and is no longer used as a
component ([#244](#244))
* redesign LdtkProject with better level data accessors and correct
modeling of internal/external levels
([#244](#244))
* use the bundle's `Default` implementation rather than the field's in
`LdtkEntity` and `LdtkIntCell` derive macros
([#222](#222))
* add `RawLevelAccessor` trait for `LdtkJson` level borrowing/iteration,
replacing existing methods
([#225](#225))
* add `LevelIndices` type defining a level's location in a project and
use it in `LevelSelection::Indices`
([#221](#221))
* change `LevelEvent` inner types from `String` to `LevelIid`
([#219](#219))
* change `LevelSet` inner type from `String` to `LevelIid`
([#219](#219))
* change `LevelSelection::Iid` inner type from `String` to `LevelIid`
([#219](#219))
* replace `LevelSet::from_iid` with `LevelSet::from_iids`, which can
convert from any collection of strings.
([#219](#219))
* use new LevelIid type in LevelEvent, LevelSet, and LevelSelection,
plus other improvements
([#219](#219))
* `LdtkProject::project` and `LdtkLevel::level` fields have both been
renamed to `data`
([#206](#206))
* All fields of `LdtkProject` and `LdtkLevel` are now privatized, and
have immutable getter methods
([#206](#206))
* `LevelMap` and `TilesetMap` type aliases have been removed
([#206](#206))
* `LdtkAsset` and `LdtkProject` are now exported in new `assets` module
instead of `lib.rs`
([#206](#206))
* asset `Loader` types are now private
([#206](#206))
* `LdtkAsset` renamed to `LdtkProject`
([#206](#206))

### Features

* add `LevelIndices` type defining a level's location in a project and
use it in `LevelSelection::Indices`
([#221](#221))
([59618fe](59618fe))
* add `RawLevelAccessor` trait for `LdtkJson` level borrowing/iteration,
replacing existing methods
([#225](#225))
([d3de2d9](d3de2d9))
* add `SpawnExclusions` to `LdtkSettings` for skipping layers by
identifier ([#275](#275))
([282404d](282404d)),
closes [#272](#272)
* add layer entity for Entity layers, changing the hierarchy
([#257](#257))
([ee20a53](ee20a53))
* add LdtkJsonWithMetadata type for representing internal- and
external-level project data with generics
([#242](#242))
([630434a](630434a))
* add LdtkProjectData for representing either internal- or
external-level project data concretely
([#243](#243))
([c530bc9](c530bc9))
* add level locale types and begin splitting internal_levels and
external_levels features
([#237](#237))
([8129e55](8129e55))
* add LevelIid component and spawn it on every level
([#215](#215))
([ad83455](ad83455))
* add LoadedLevel type that wraps around levels with complete data
([#214](#214))
([3d40c15](3d40c15))
* add types and traits around LevelMetadata
([#229](#229))
([382dea2](382dea2))
* change `LevelEvent` inner types from `String` to `LevelIid`
([#219](#219))
([0039ed7](0039ed7))
* change `LevelSelection::Iid` inner type from `String` to `LevelIid`
([#219](#219))
([0039ed7](0039ed7))
* change `LevelSet` inner type from `String` to `LevelIid`
([#219](#219))
([0039ed7](0039ed7))
* LdtkLevel renamed to LdtkExternalLevel and is no longer used as a
component ([#244](#244))
([670cd4e](670cd4e))
* redesign LdtkProject with better level data accessors and correct
modeling of internal/external levels
([#244](#244))
([670cd4e](670cd4e))
* replace `LevelSet::from_iid` with `LevelSet::from_iids`, which can
convert from any collection of strings.
([#219](#219))
([0039ed7](0039ed7))
* upgrade to bevy 0.12
([#265](#265))
([194731e](194731e))
* upgrade to LDtk 1.5.3, dropping support for previous versions
([#295](#295))
([4926a50](4926a50))
* upgrade to LDtk types and examples to 1.4.1 (drop support for
&lt;1.4.1) ([#256](#256))
([ab21e2c](ab21e2c))
* use new LevelIid type in LevelEvent, LevelSet, and LevelSelection,
plus other improvements
([#219](#219))
([0039ed7](0039ed7))
* use the bundle's `Default` implementation rather than the field's in
`LdtkEntity` and `LdtkIntCell` derive macros
([#222](#222))
([f003127](f003127))


### Bug Fixes

* don't apply level set until project and dependencies are completely
loaded ([#296](#296))
([dbfe1c6](dbfe1c6))
* normalize resolved asset paths using `path_clean`
([#255](#255))
([33a8998](33a8998)),
closes [#240](#240)
* only spawn invisible tiles on first sub-layer of AutoTile+IntGrid
layers ([#231](#231))
([d2873e3](d2873e3))
* recalculate layer offset to adjust for tileset sizes
([#254](#254))
([c00085d](c00085d))
* use entity definition tile size instead of entity instance tile size
as basis when calculating ldtk entity scale
([#271](#271))
([833af01](833af01))


### Documentation Changes

* add 0.8 to 0.9 migration guide
([#266](#266))
([bb91660](bb91660))
* add collectathon cargo example
([#288](#288))
([32dfb85](32dfb85))
* add mdbook with outline and introduction
([#261](#261))
([810b25a](810b25a))
* add tile-based game example w/ a tutorial in the book, replacing
getting-started guide
([#269](#269))
([2d43efa](2d43efa))
* document all-features in docs.rs
([#252](#252))
([321bb07](321bb07))
* reference book in API ref and README.md, replacing redundant sections
([#282](#282))
([e7afdad](e7afdad))
* remove README.md caveat for hot reloading external levels
([#253](#253))
([59eb6b3](59eb6b3))
* write *Anatomy of the World* chapter of book
([#285](#285))
([29d5e33](29d5e33))
* write *Create bevy relations from ldtk entity references* chapter of
book ([#287](#287))
([8080f24](8080f24))
* write *Game Logic Integration* chapter of the book
([#279](#279))
([a62a556](a62a556))
* write *Level Selection* chapter of book
([#284](#284))
([226c60c](226c60c))
* write *Make level selection follow player* chapter of book
([#293](#293))
([201d908](201d908))
* write *Respawn levels and worlds* chapter of book
([#289](#289))
([55ed30f](55ed30f))


### Code Refactors

* `LdtkAsset` and `LdtkProject` are now exported in new `assets` module
instead of `lib.rs`
([#206](#206))
([fe44774](fe44774))
* `LdtkAsset` renamed to `LdtkProject`
([#206](#206))
([fe44774](fe44774))
* `LdtkProject::project` and `LdtkLevel::level` fields have both been
renamed to `data`
([#206](#206))
([fe44774](fe44774))
* `LevelMap` and `TilesetMap` type aliases have been removed
([#206](#206))
([fe44774](fe44774))
* All fields of `LdtkProject` and `LdtkLevel` are now privatized, and
have immutable getter methods
([#206](#206))
([fe44774](fe44774))
* asset `Loader` types are now private
([#206](#206))
([fe44774](fe44774))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: Trevor Lovell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants