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

cocos2d-x 3.7.1 scale-9 sprite works different from the previous one #13560

Closed
ryule opened this issue Aug 25, 2015 · 7 comments
Closed

cocos2d-x 3.7.1 scale-9 sprite works different from the previous one #13560

ryule opened this issue Aug 25, 2015 · 7 comments
Assignees
Labels
Milestone

Comments

@ryule
Copy link
Contributor

ryule commented Aug 25, 2015

In new cocos2d-x, 9-patch functionality has been added to ui::Scale9Sprite.
But the changed code affects the way of working of the original Scale9Sprite.
(Reference commit: 0352a7a)

1. The Scale9Sprite works weird

because the following code:

  • UIScale9Sprite.cpp:526
    • if(_capInsets.equals(Rect::ZERO))
  • UIScale9Sprite.cpp:549
    • if(!capInsets.equals(Rect::ZERO))

For example, setInsetLeft/Top/Right/Bottom methods call updateWithSprite.
(setInset* > updateCapInset > setCapInsets > updateWithSprite finally)
If a series of setInset* method call is happened then
the above ifs will work as an unintended way.

I don't know why those if conditions needed exactly, but if they are for
9-patch functionality then the other condition check should be added such as
&& _isPatch9.

2. CCTexture2D contains private methods related to 9-patch sprite

  • isContain9PatchInfo, getSpriteFrameCapInset, removeSpriteFrameCapInset, and addSpriteFrameCapInset.

Due to this structure there is no way to write and extend the codes that manage cocos2d::SpriteFrame as 9-patch sprites manually.
I think those methods mentioned above would be declared as public.

@zilongshanren
Copy link
Member

@ryule
Thanks, I will resolve the issue 1, my bad.

For the issue 2:
At the first beginning, I designed all the APIs to be public. But @minggo suggest that there are no mandatory reasons for public these APIs.

Could you give an example? Or could we just make them as protected such that the subclass could use it.

Thanks.

@zilongshanren
Copy link
Member

Hi, @ryule
Does the PR fix the 1st issue?
Thanks.

@ryule
Copy link
Contributor Author

ryule commented Aug 25, 2015

@zilongshanren, your commit 6cea66f fixed the first issue, thanks!

For the second issue, a stand alone texture cache manager that independent to the given CCSpriteFrameCache would be an example. The manager should have an access right to the methods related to 9-patch to make its functionalities intact.
Even with the protected methods, I think that the code cannot have a completely separated structure without helping sort of friend class things. If any other better way exist then it would be great.

Thanks.

@zilongshanren
Copy link
Member

@minggo What do you think about public these APIs?
I don't like the friend class either as @ryule

@zilongshanren
Copy link
Member

@ryule
For the second issue, we decide to not public these APIs. If these APIs are public, we should maintain them and it's hard to evolve when the design is not good enough. And these APIs introduce implicit dependency between SpriteFrame and Texture2D object, we still think these APIs are not suitable for public.

If most developers want these APIs, then we would consider public them.

Thanks.

@ryule
Copy link
Contributor Author

ryule commented Aug 26, 2015

Sure, it's fine. I really appreciate your effort for this issue.
Thanks, @zilongshanren.

@ryule ryule closed this as completed Aug 26, 2015
@zilongshanren
Copy link
Member

@ryule You're welcome.

edwinkcw pushed a commit to oursky/cocos2d-x that referenced this issue Oct 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants