Skip to content

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

Merged
bolsinga merged 2 commits into
TextureGroup:masterfrom
IvanChan:FixImageNode
Jun 16, 2020
Merged

-[ASNetworkImageNode setURL:resetToDefault:] forget to reset animatedImage#1861
bolsinga merged 2 commits into
TextureGroup:masterfrom
IvanChan:FixImageNode

Conversation

@IvanChan

@IvanChan IvanChan commented Jun 2, 2020

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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

IvanChan commented Jun 16, 2020

Copy link
Copy Markdown
Contributor Author

@bolsinga unit tests added

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

@bolsinga

Copy link
Copy Markdown
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