Skip to content

[cpp-tests] SpritePolygonTest invoke UISlide::setPercent after initialization #19676

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 10, 2019
Merged

[cpp-tests] SpritePolygonTest invoke UISlide::setPercent after initialization #19676

merged 3 commits into from
May 10, 2019

Conversation

@PatriceJiang
Copy link
Contributor

@PatriceJiang PatriceJiang commented May 8, 2019 •
edited
Loading

PR #19556 cause crash when invoke setPercent

For example:

cocos2d-x/tests/cpp-tests/Classes/SpritePolygonTest/SpritePolygonTest.cpp

Lines 298 to 303 in d6b8e99

slider->addEventListener(CC_CALLBACK_2(SpritePolygonTestSlider::changeEpsilon, this));
slider->setPercent((int)(sqrtf(1.0f/19.0f)*100));
auto ttfConfig = TTFConfig("fonts/arial.ttf", 8);
_epsilonLabel = Label::createWithTTF(ttfConfig, "Epsilon: 2.0");
addChild(_epsilonLabel);

  • setPercent will trigger changeEpsilon()
  • changeEpsilon() will access _epsilonLabel which is not initialized by then

Fix

delay the trigger of event ON_PERCENTAGE_CHANGED

invoke setPercent after initialization completed.

@PatriceJiang PatriceJiang added this to the metal-support milestone May 8, 2019
@PatriceJiang PatriceJiang changed the title Delay onProgressChange event when setProgress Delay ON_PERCENTAGE_CHANGED event when setPercent May 8, 2019
@PatriceJiang PatriceJiang changed the title Delay ON_PERCENTAGE_CHANGED event when setPercent [UISlider] Delay ON_PERCENTAGE_CHANGED event when setPercent May 8, 2019
@PatriceJiang PatriceJiang marked this pull request as ready for review May 8, 2019 02:08
@minggo
Copy link
Contributor

minggo commented May 9, 2019

So v3 will crash too?

@PatriceJiang
Copy link
Contributor Author

PatriceJiang commented May 9, 2019

yes, v3 will also crash

@minggo
Copy link
Contributor

minggo commented May 9, 2019

I think the modification is not reasonable, the game logic should make sure the variable is valid.

@PatriceJiang
Copy link
Contributor Author

PatriceJiang commented May 9, 2019

I have revert these modifications and fix the bug in test case.

@PatriceJiang PatriceJiang changed the title [UISlider] Delay ON_PERCENTAGE_CHANGED event when setPercent [cpp-tests] SpritePolygonTest int UISlide::setPercent after initialization May 9, 2019
@PatriceJiang PatriceJiang changed the title [cpp-tests] SpritePolygonTest int UISlide::setPercent after initialization [cpp-tests] SpritePolygonTest invoke UISlide::setPercent after initialization May 9, 2019
@minggo minggo merged commit bd76e89 into cocos2d:metal-support May 10, 2019
@minggo
Copy link
Contributor

minggo commented May 10, 2019

please send a PR to 3.17.2 branch.

PatriceJiang added a commit to PatriceJiang/cocos2d-x that referenced this pull request May 13, 2019
@PatriceJiang
[cpp-tests] SpritePolygonTest invoke UISlide::setPercent after init…
8b05e77 
…ialization (cocos2d#19676)

* delay onProgressChange event when setProgress

* Revert "delay onProgressChange event when setProgress"

This reverts commit a9ad090.

* setPercent after initialization
PatriceJiang added a commit to PatriceJiang/cocos2d-x that referenced this pull request May 13, 2019
@PatriceJiang
[cpp-tests] SpritePolygonTest invoke UISlide::setPercent after init…
9f295e2 
…ialization (cocos2d#19676)

* delay onProgressChange event when setProgress

* Revert "delay onProgressChange event when setProgress"

This reverts commit a9ad090.

* setPercent after initialization
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@minggo minggo minggo approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.0

Development

Successfully merging this pull request may close these issues.

2 participants

@PatriceJiang @minggo