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.

Sign up for GitHub

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

Jump to bottom

Split SpriteWidget according to whether it loads an image or not #1667

Closed
Hwan-seok opened this issue May 28, 2022 · 1 comment · Fixed by #1674
Closed

Split SpriteWidget according to whether it loads an image or not #1667

Hwan-seok opened this issue May 28, 2022 · 1 comment · Fixed by #1674
Labels
enhancement

Comments

@Hwan-seok
Copy link
Contributor

Hwan-seok commented May 28, 2022 •
edited
Loading

What could be improved

Remove loading when SpriteWidget does not need to load the image.

Why should this be improved

SpriteWidget is unified by BaseFutureBuilder whether the Image(Sprite) has already loaded or not.
This forces every SpriteWidget to show a loading widget even though it has already loaded Sprite.
This leads to preventing gapless playback when Image(Sprite) changes.

Any risks?

No

More information

Are you interested in working on a PR for this? Yes

Before

2022-05-28.8.58.31.mov

After

2022-05-28.8.57.33.mov
@Hwan-seok Hwan-seok changed the title Add support gapless playback to SpriteWidget Distinguish between SpriteWidget with and without image load. May 28, 2022
@Hwan-seok Hwan-seok changed the title Distinguish between SpriteWidget with and without image load. Distinguish between SpriteWidget with and without loading image. May 28, 2022
@Hwan-seok Hwan-seok changed the title Distinguish between SpriteWidget with and without loading image. Split SpriteWidget according to whether they load an image or not May 28, 2022
@Hwan-seok Hwan-seok changed the title Split SpriteWidget according to whether they load an image or not Split SpriteWidget according to whether it loads an image or not May 28, 2022
@spydon
Copy link
Member

spydon commented May 28, 2022

Nice, will you put up a PR for this?

@Hwan-seok Hwan-seok mentioned this issue May 28, 2022
Merged
6 tasks
spydon pushed a commit that referenced this issue May 31, 2022
feat!: Add ability to render without loading on image related widgets (…
40a061b 
…#1674)

At first, In #1667, I was planning to change only SpriteWidget to be able to remove showing LoadingBuilder when non-future sprite was passed.
But after some digging, I found that BaseFutureBuilder causes the loading because it only accepts Future as a parameter.
And BaseFutureBuilder are the very base of all image-related widgets such as NineTileBoxWidget, SpriteAnimationWidget, ... etc.

So the key point of this change should allow BaseFutureBuilder to FutureOr as a parameter and split the build function according to the parameter type.

After #1645, the Default constructor of SpriteButton can pass Future but its constructor is not consistent with other image-related widgets.
Plus, type of FutureOr<List<Sprite>> _buttonsFuture only compatible with Future<List<Sprite>> but not List<Sprite>.
So I added secondary constructor SpriteButton.future that accepts only Future and Replaced the FutureOr<Sprite> to Sprite in the default constructor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat!: Add ability to render without loading on image related widgets Hwan-seok/flame
2 participants
@spydon @Hwan-seok