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: add FutureOr support on SpriteButton #1645

Merged
merged 3 commits into from
May 23, 2022
Merged

Conversation

oloshe
Copy link
Contributor

@oloshe oloshe commented May 20, 2022

It can use Future as its argument

@spydon
Copy link
Member

spydon commented May 20, 2022

I don't see the point of this change, can you please elaborate?

@oloshe
Copy link
Contributor Author

oloshe commented May 20, 2022

I don't see the point of this change, can you please elaborate?

Whether it's a Future or not, it ends up being a Future<List>. It can also be passed directly when you need to load the image asynchronously to create a Sprite or a method return Future like Sprite.load. In addition, it will not affect the case of directly passing Sprite as a parameter

@spydon
Copy link
Member

spydon commented May 20, 2022

Ah, now when I looked at the rest of the code it makes sense!

@spydon spydon merged commit 2e82dc9 into flame-engine:main May 23, 2022
spydon pushed a commit that referenced this pull request May 31, 2022
…#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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants