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

Chore/bevy 0.12 #95

Merged
merged 13 commits into from
Nov 13, 2023
Merged

Chore/bevy 0.12 #95

merged 13 commits into from
Nov 13, 2023

Conversation

thombruce
Copy link
Owner

@thombruce thombruce commented Nov 8, 2023

Closes #85

By submitting this pull request, you agree to follow our Code of Conduct: https://github.com/thombruce/.github/blob/main/CODE_OF_CONDUCT.md


Internal use. Do not delete.

  • Tests passing
  • Coverage sufficient
  • Manual review
  • No A11y regression
  • Translations provided or not needed

@thombruce
Copy link
Owner Author

thombruce commented Nov 8, 2023

@thombruce
Copy link
Owner Author

The big holdout here is bevy-ui-navigation. The PR for this depends on bevy_mod_picking which depends on bevy_eventlistener, bevy_mod_raycast and bevy_rapier.

bevy_eventlistener is already merged; the other two look to be complete, just awaiting merges and version bumps themselves.

I attempted to resolve the dependencies myself, but I'm not sure this is supported by the Cargo framework. To achieve the desired result, I believe I would have to fork bevy-ui-navigation and bevy_mod_picking and point each of their dependencies to the branches/commits I would like, then point Verse's dependencies to my own forks... just until everything is brought up to date.

We're also uncertain as of now if bevy_tiling_background works as of the Bevy 0.12 update. This will have to wait until the above dependencies can be resolved, but I don't at all mind forking this one and submitting a PR if necessary. Only a minor complication there in that I'm already using a forked PR that is yet to be merged.

@thombruce
Copy link
Owner Author

Last blocker for bevy-ui-navigation is aevyrie/bevy_mod_picking#276 which is now only waiting on aevyrie/bevy_mod_raycast#100

@thombruce
Copy link
Owner Author

thombruce commented Nov 9, 2023

Couple of errors left in the codebase; I'm pretty sure one of them will be resolved when bevy-ui-navigation is updated.

The other is this:

mut materials: ResMut<Assets<BackgroundMaterial>>,

BackgroundMaterial does not implement Asset. I'm pretty sure this requires a small update to bevy_tiling_background, which I may handle and submit a PR for myself. However, I currently depend on a forked branch. To handle this...

  1. Clone the base repository
  2. Add the fork as a remote
  3. Checkout the tex-typo branch with an earlier fix
  4. Checkout a new branch based off tex-typo and apply fixes
  5. Submit a PR...

...against what base? Not entirely sure. tex-typo's commit shas won't have changed, been overwritten or affected in any way. But merging into main would make the other PR redundant... Not sure what the best practise is, but the PR is a courtesy; until its merged...

  1. Use the new branch from own remote

I.e. Make this change:

- bevy_tiling_background = { git = "https://github.com/rparrett/bevy_tiling_background.git", branch = "tex-typo" }
+ bevy_tiling_background = { git = "https://github.com/thombruce/bevy_tiling_background.git", branch = "bevy-0.12-fix" }

On second thoughts... not such an easy fix. I'm messing around with it now, but I'm somewhat out of my depth; the crate implements its own custom asset loading (assets are changed in Bevy 0.12) and makes use of shaders. These are somewhat alien to me, and I don't think I can fix this myself without a lot of trial, error and pain... 🤔 This may mean sticking with Bevy 0.11 for the time being.

@thombruce
Copy link
Owner Author

A recent (0.12) comment on an old issue about handling repeating backgrounds in Bevy: bevyengine/bevy#399 (comment)

@thombruce
Copy link
Owner Author

Removed bevy_tiling_background; replaced with solution in comment above - it doesn't scale infinitely but it does the trick for now.

Everything else is up to date and appears to be working.

@thombruce thombruce marked this pull request as ready for review November 12, 2023 02:46
@thombruce thombruce mentioned this pull request Nov 13, 2023
@thombruce
Copy link
Owner Author

So far as I can see, the only regression, missing feature or bug is #96 and I'm happy to leave that be for now and circle back to it separately.

@thombruce thombruce merged commit 0b6d1be into main Nov 13, 2023
@thombruce thombruce deleted the chore/bevy-0.12 branch November 13, 2023 01:22
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.

Upgrade to Bevy v0.12
1 participant