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

refactor codes #17793

Merged
merged 3 commits into from
Nov 4, 2024
Merged

refactor codes #17793

merged 3 commits into from
Nov 4, 2024

Conversation

minggo
Copy link
Contributor

@minggo minggo commented Oct 31, 2024

Re: #17715

  • use self to reduce package size
  • simplify codes

@minggo minggo requested a review from qiuguohua October 31, 2024 06:38
@minggo
Copy link
Contributor Author

minggo commented Oct 31, 2024

@cocos-robot run test cases

return result;
if (!this.horizontal) vector.x = 0;
if (!this.vertical) vector.y = 0;
return vector;
Copy link
Contributor Author

@minggo minggo Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplify codes.

@@ -1536,96 +1580,100 @@ export class ScrollView extends ViewGroup {
_tempVec3_1.set(_tempVec2_1.x, _tempVec2_1.y, 0);
uiTransformComp.convertToNodeSpaceAR(_tempVec3, _tempVec3);
uiTransformComp.convertToNodeSpaceAR(_tempVec3_1, _tempVec3_1);
Vec3.subtract(vec, _tempVec3, _tempVec3_1);
Vec3.subtract(out, _tempVec3, _tempVec3_1);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use out instead, don't have to create a new temple object vec.

Copy link

github-actions bot commented Oct 31, 2024

👍 Package size ⤵ -1030 bytes, old: 5429602, new: 5428572

Interface Check Report

This pull request does not change any public interfaces !

const item = listener;

if (this.node === item) {
if (this.node === listener) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

item is not needed, just delete it.

clonedDeltaMove.multiplyScalar(factor);
targetDelta.set(clonedDeltaMove);
targetDelta.set(deltaMove);
targetDelta.multiplyScalar(factor + 1);
Copy link
Contributor Author

@minggo minggo Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// clonedDeltaMove = deltaMove
const clonedDeltaMove = deltaMove.clone(); 
// clonedDeltaMove = clonedDeltaMove * factor = deltaMove * factor
clonedDeltaMove.multiplyScalar(factor); 
// targetDelta = clonedDeltaMove = deltaMove * factor          
targetDelta.set(clonedDeltaMove);     
// targetDelta = targetDelta + deltaMove = deltaMove * factor + deltaMove = deltaMove * (factor + 1)              
targetDelta.add(deltaMove);                              

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the factor need to be increased?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please refer to the comment above.

@@ -1240,13 +1276,12 @@ export class ScrollView extends ViewGroup {

const originalMoveLength = deltaMove.length();
let factor = targetDelta.length() / originalMoveLength;
targetDelta.add(deltaMove);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as targetDelta will be set in if (this.brake > 0 && factor > 7), so move it to else block bellow.

Copy link

@minggo, Please check the result of run test cases:

Task Details

Copy link

@minggo, Please check the result of run test cases:

Task Details

@minggo minggo merged commit 651f013 into cocos:v3.8.5 Nov 4, 2024
12 checks passed
@minggo minggo deleted the scrollview-improve branch November 4, 2024 02:21
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