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!: Use Flame.images in flame_texturepacker #3103

Merged

Conversation

ABausG
Copy link
Contributor

@ABausG ABausG commented Mar 27, 2024

Description

Using Flame.images instead of a custom ImageCache in TexturePackerAtlas allowing users to override the ImageCache's prefix to support loading Atlas Animations from different packages.
Due to this change the prefix of the used ImageCache changed from assets/ to assets/images/

Checklist

  • I have followed the Contributor Guide when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples or docs.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

Migration instructions

Either move all sprites and textures from assets/ to assets/images/ or provide a custom cache using images: Images(prefix: 'assets/')or use Flame.images.prefix = 'assets/' to point to the same Location as before. Be aware that the latter approach might interfere with loading other Image Assets

Related Issues

Closes #3096

@ABausG ABausG changed the title feat!: use Flame.images in flame_texturepacker feat!: Use Flame.images in flame_texturepacker Mar 27, 2024
Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

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

I was going to suggest that we let the user pass in an optional custom cache too, but since that argument will have to go through so many methods I think we can leave that as a follow-up for when someone needs it.

@ABausG
Copy link
Contributor Author

ABausG commented Mar 27, 2024

[...] that argument will have to go through so many methods

If I counted correctly it would be ~6 methods with a maximum call chain of 3. This would then still be kind of reasonable I guess.

If this route is taken would you suggest we keep a Images(prefix: 'assets/') as the default? Then this wouldn't even be a breaking change!

Happy to adjust that @spydon

@spydon
Copy link
Member

spydon commented Mar 27, 2024

If I counted correctly it would be ~6 methods with a maximum call chain of 3. This would then still be kind of reasonable I guess.

Alright, if you want to adjust it you can follow how it's done in the parallax:
https://github.com/flame-engine/flame/blob/main/packages/flame/lib/src/parallax.dart#L147

If this route is taken would you suggest we keep a Images(prefix: 'assets/') as the default? Then this wouldn't even be a breaking change!

I think the breaking change is still worth it, it's good to default to Flame.images like everywhere else.

@ABausG ABausG requested a review from spydon April 2, 2024 07:48
@ABausG
Copy link
Contributor Author

ABausG commented Apr 2, 2024

@spydon adjusted to pass the custom cache through. Also applying the custom cache for the Assets Loading.

Also adjusted the PR description to include the 3rd way of migrating the version

Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

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

Lgtm

@spydon spydon merged commit 418cc57 into flame-engine:main Apr 2, 2024
9 checks passed
@ABausG ABausG deleted the feat/texture-packer-use-flame-images branch April 2, 2024 09:40
spydon added a commit that referenced this pull request Apr 18, 2024
In this [PR](#3103) we updated
the code to use `Flame.images` because of this change now the atlas file
and the sprite sheet both must be under the `assets/images/` directory.

Trying to run the example project will crash because the atlas find
cannot be found.

This MR fixes the problem by loading the `.atlas` file correctly from
the images directory.

Co-authored-by: Lukas Klingsbo <me@lukas.fyi>
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.

texture packer can't override images
2 participants