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

-[ASNetworkImageNode setURL:resetToDefault:] forget to reset animatedImage #1861

Merged
merged 2 commits into from
Jun 16, 2020

Conversation

IvanChan
Copy link
Contributor

@IvanChan IvanChan commented Jun 2, 2020

If you have a ASNetworkImageNode with animatedImage

Then you call [ImageNode setURL:someLink resetToDefault:true]
Node will still always display Old animatedImage, the new static image will never be display

Related issue #1815

@bolsinga
Copy link
Contributor

Thanks! Please add a unit tests that fails without this code change, and succeeds with it. This will make it harder for the behavior to regress.

@IvanChan
Copy link
Contributor Author

IvanChan commented Jun 16, 2020

@bolsinga unit tests added

I wonder, should also clear animatedImage when a static image is download finish?

@bolsinga
Copy link
Contributor

@bolsinga unit tests added

Thank you!

I wonder, should also clear animatedImage when a static image is download finish?

I don't know enough about that. However, I do think it would be best in a separate PR!

@bolsinga bolsinga merged commit 5651826 into TextureGroup:master Jun 16, 2020
garrettmoon pushed a commit that referenced this pull request Jun 25, 2020
…Image (#1861)

* Fix -[ASNetworkImageNode setURL:resetToDefault:] forget to reset animatedImage

* add ASNetworkImageNodeTest for reset with animatedImage
piotrdebosz pushed a commit to getstoryteller/Texture that referenced this pull request Mar 1, 2021
…Image (TextureGroup#1861)

* Fix -[ASNetworkImageNode setURL:resetToDefault:] forget to reset animatedImage

* add ASNetworkImageNodeTest for reset with animatedImage
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.

2 participants