axmolengine / axmol

Axmol Engine – A Multi-platform Engine for Desktop, XBOX (UWP) and Mobile games. (A fork of Cocos2d-x-4.0)
https://axmol.dev
MIT License
868 stars 195 forks source link

`Sprite::setPolygonInfo()` doesn't set content size #2112

Closed smilediver closed 1 month ago

smilediver commented 1 month ago

Sprite::setPolygonInfo() doesn't call setContentSize(), while Sprite::initWithPolygon() does. It looks like Sprite::setPolygonInfo() should also call it, but I'm not 100% certain so would like some external input.

https://github.com/axmolengine/axmol/blob/284fcf243a5d81862c7cca623ec1a0fbf7e8304e/core/2d/Sprite.cpp#L1715-L1719

https://github.com/axmolengine/axmol/blob/284fcf243a5d81862c7cca623ec1a0fbf7e8304e/core/2d/Sprite.cpp#L260-L274

rh101 commented 1 month ago

Sprite::initWithPolygon is called from Sprite* Sprite::create(const PolygonInfo& info), so it's something used only on creation, and since this is the case, the content size needs to be initialized to something valid.

Sprite::setPolygonInfo(), on the other hand, can be called at any time to set specific polygon info, but the sprite may already have a custom content size set, one that is used with _stretchEnabled = true, so setPolygonInfo should not be overwriting the existing content size.

smilediver commented 1 month ago

_stretchEnabled doesn't work with polygon. And I think, in most cases you probably want to set content size if you're changing the polygon anyway. That's how I stumbled upon this, since we call setPolygonInfo() and then setContentSize(), but it rises a warning. I wanted to remove the warning first, but then thought that maybe there's a bug in setPolygonInfo().

https://github.com/axmolengine/axmol/blob/284fcf243a5d81862c7cca623ec1a0fbf7e8304e/core/2d/Sprite.cpp#L1361-L1371

Edit: made a #2113 PR for warning workaround.

rh101 commented 1 month ago

_stretchEnabled doesn't work with polygon.

I didn't realize that was the case, since I can't recall ever seeing that warning come up, so it's unlikely I've ever used the polygon rendering.

Edit: made a #2113 PR for warning workaround.

That's a reasonable change to avoid the warning spam.

smilediver commented 1 month ago

Closing the issue for now. I can make a fix PR if later this is deemed to be a bug.