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

Update node.ts setSiblingIndex support negative index #17592

Merged
merged 2 commits into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cocos/scene-graph/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@

const idGenerator = new js.IDGenerator('Node');

function getConstructor<T> (typeOrClassName: string | Constructor<T> | AbstractedConstructor<T>): Constructor<T> | AbstractedConstructor<T> | null | undefined {

Check warning on line 54 in cocos/scene-graph/node.ts

View workflow job for this annotation

GitHub Actions / Run ESLint

This line has a length of 160. Maximum allowed is 150
if (!typeOrClassName) {
errorID(3804);
return null;
Expand Down Expand Up @@ -291,7 +291,7 @@
return null;
}

protected static _findComponents<T extends Component> (node: Node, constructor: Constructor<T> | AbstractedConstructor<T>, components: Component[]): void {

Check warning on line 294 in cocos/scene-graph/node.ts

View workflow job for this annotation

GitHub Actions / Run ESLint

This line has a length of 159. Maximum allowed is 150
const cls = constructor;
const comps = node._components;
// NOTE: internal rtti property
Expand Down Expand Up @@ -630,7 +630,7 @@
return;
}
const siblings = this._parent._children;
index = index !== -1 ? index : siblings.length - 1;
index = index >= 0 ? index : siblings.length + index;
const oldIndex = siblings.indexOf(this);
kaokei marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

@dumganhar dumganhar Sep 6, 2024

Choose a reason for hiding this comment

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

Would it run well if passing a negative value that smaller than -siblings.length?

Copy link
Contributor

Choose a reason for hiding this comment

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

@kaokei
Could you take a look at this question?
If index < -siblings.length, for example, it's -siblings.length - 10,
After this logic index = index >= 0 ? index : siblings.length + index;
The index will be siblings.length + (-siblings.length - 10) = -10, -10 will be a invalid index.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems splice will check whether the index is in the range and do a clamp operation.

if (index !== oldIndex) {
siblings.splice(oldIndex, 1);
Expand Down
3 changes: 2 additions & 1 deletion native/cocos/core/scene-graph/Node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,8 @@ void Node::setSiblingIndex(index_t index) {
return;
}
ccstd::vector<IntrusivePtr<Node>> &siblings = _parent->_children;
index = index != -1 ? index : static_cast<index_t>(siblings.size()) - 1;
index = index >= 0 ? index : static_cast<index_t>(siblings.size()) + index;
index = index >= 0 ? index : 0;
minggo marked this conversation as resolved.
Show resolved Hide resolved
index_t oldIdx = getIdxOfChild(siblings, this);
if (index != oldIdx) {
if (oldIdx != CC_INVALID_INDEX) {
Expand Down
Loading