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

Fix button node in the onLoad callback setting scale is invalid #5740

Merged
merged 2 commits into from Nov 21, 2019
Merged

Fix button node in the onLoad callback setting scale is invalid #5740

merged 2 commits into from Nov 21, 2019

Conversation

zhefengzhang
Copy link
Contributor

@zhefengzhang zhefengzhang commented Nov 14, 2019

Re: cocos/2d-tasks#2077
这样修改的原因是因为 button 在 _preload 阶段执行了 this._updateState(); 这时候把 _transitionFinished 改成了 false。这句代码优先于用户代码的 onLoad 回调的执行顺序,所以当用户在脚本 onLoad 回调修改节点 scale 时,因为条件语句:

if (this._originalScale && (this.transition !== Transition.SCALE || this._transitionFinished)) {

的限制,导致无法执行。
所以现在将 _preload 中的 this._updateState(); 移到 onLoad 回调中,这样就可以解决问题。

@holycanvas holycanvas requested a review from PPpro Nov 15, 2019
@holycanvas
Copy link
Contributor

@holycanvas holycanvas commented Nov 15, 2019

这个改法,感觉不太好,感觉得看下,这里为什么要加这个判断
image
@umbrellaPP 加了这个判断之后,originalScale就没法变了

@holycanvas holycanvas requested a review from jareguo Nov 18, 2019
@PPpro
Copy link
Contributor

@PPpro PPpro commented Nov 18, 2019

@umbrellaPP 加了这个判断之后,originalScale就没法变了

这个 判断改为

if (!(this.transition === Transition.SCALE && !this._transitionFinished))

这样比较清晰,之前这样做是为了确保 scale 在 transition 的过程不会一直改变 originalScale

这里矛盾的点在于 onLoad 阶段还处于 transition 状态,__preload 阶段应该要调 _resetState 才对,而不是 _updateState

@holycanvas
Copy link
Contributor

@holycanvas holycanvas commented Nov 18, 2019

这里矛盾的点在于 onLoad 阶段还处于 transition 状态,__preload 阶段应该要调 _resetState 才对,而不是 _updateState

应该把_resetState放在__preload下,把 _updateState 放 onEnable 下,这样感觉要合理一些?

@PPpro
Copy link
Contributor

@PPpro PPpro commented Nov 18, 2019

应该把_resetState放在__preload下,把 _updateState 放 onEnable 下,这样感觉要合理一些?

感觉没必要调 _updateState 了

@zhefengzhang
Copy link
Contributor Author

@zhefengzhang zhefengzhang commented Nov 18, 2019

嗯 直接换成 _resetState() 就行

@holycanvas
Copy link
Contributor

@holycanvas holycanvas commented Nov 18, 2019

感觉没必要调 _updateState 了

为什么不用啊,不需要获取 button 状态,处理过渡么

@PPpro
Copy link
Contributor

@PPpro PPpro commented Nov 18, 2019

为什么不用啊,不需要获取 button 状态,处理过渡么

_preload 阶段只需要初始化一些数据,不需要处理过度,感觉 _resetState 会更合适 @holycanvas

@zhefengzhang
Copy link
Contributor Author

@zhefengzhang zhefengzhang commented Nov 18, 2019

仔细看了下,_resetState 还是不能替代 _updateState 的,比如:
_updateState 中的 _updateDisabledState;
目前看来还是需要找个地方调用 _updateState。
因为当 button 的状态发生任何改变都会调用 _updateState ,其调用频率已经很高,放在 onLoad 或 start 会好点。start 的执行顺序会更晚点,放在这能够尽量避免与节点的 onLoad 冲突。

@PPpro
Copy link
Contributor

@PPpro PPpro commented Nov 18, 2019

_updateState 中的 _updateDisabledState;

_updateDisabledState 就是更新下 gray 材质,编辑器里已经将这部分配置序列化了,保存在 sprite 组件的材质属性里

@zhefengzhang
Copy link
Contributor Author

@zhefengzhang zhefengzhang commented Nov 21, 2019

我测试一遍,如果没问题的话就这样吧?

@holycanvas
Copy link
Contributor

@holycanvas holycanvas commented Nov 21, 2019

看了下,初始化的时候确实不需要 _updateState, _reset 就好了

@holycanvas holycanvas merged commit 99f126c into cocos:v2.2.1-release Nov 21, 2019
1 check passed
@zty8023ys
Copy link
Contributor

@zty8023ys zty8023ys commented Mar 11, 2020

有bug
使用Transition.SPRITE, 先修改interactable再添加节点, 节点不会替换为指定的sprite
因为修改interactable时, 还没有到__preload, _sprite没有赋值; 而后续没有调用_applyTransition的地方了
@holycanvas @Jno1995 @umbrellaPP

@zhefengzhang
Copy link
Contributor Author

@zhefengzhang zhefengzhang commented Mar 11, 2020

收到。

@zhefengzhang
Copy link
Contributor Author

@zhefengzhang zhefengzhang commented Mar 25, 2020

节点不会替换为指定的sprite

麻烦给我一个 demo?

@zty8023ys
Copy link
Contributor

@zty8023ys zty8023ys commented Mar 25, 2020

节点不会替换为指定的sprite

麻烦给我一个 demo?

归档.zip

@zhefengzhang
Copy link
Contributor Author

@zhefengzhang zhefengzhang commented Mar 26, 2020

修复了:
#6421

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.

None yet

4 participants