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.xml: specify that normal processing happens in tree order #97905

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

0x53A
Copy link
Contributor

@0x53A 0x53A commented Oct 6, 2024

For _ready it was already spelled out in a few places that it happens bottom to top (child first parent later).
I found it hard to find information about the order of _process and _physics_process, the only place I found was here in the tutorial for SceneTree which someone linked me to.

@0x53A 0x53A requested a review from a team as a code owner October 6, 2024 23:45
@AThousandShips AThousandShips added enhancement documentation cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Oct 7, 2024
@AThousandShips AThousandShips added this to the 4.x milestone Oct 7, 2024
doc/classes/Node.xml Outdated Show resolved Hide resolved
@0x53A
Copy link
Contributor Author

0x53A commented Oct 13, 2024

Thank you for fixing the typo!

I found that my suggested change is incomplete, for _process and _physics_process, there are the properties process_priority and process_physics_priority, respectively.

@0x53A
Copy link
Contributor Author

0x53A commented Oct 13, 2024

Edit: When Priority is changed inside a process callback, the order will be wrong for this one iteration, then correct at the next iteration. That should be fine.

@AThousandShips
Copy link
Member

I'd say this is a bit too complex to describe in the class docs here, and should be a detailed page in the docs instead, and linked to

Related:

@0x53A
Copy link
Contributor Author

0x53A commented Oct 13, 2024

I've removed it from the top paragraph since as you said that would get pretty long, mentioning both priority and tree order.

Personally I would prefer to still add it to the actual method documentation, if only for parity with _ready, but the call is obviously yours.

To be fully honest I don't have the knowledge, time, or motivation to create the full processing diagram requested in the issue.

Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

LGTM

doc/classes/Node.xml Outdated Show resolved Hide resolved
@Repiteo Repiteo modified the milestones: 4.x, 4.4 Oct 16, 2024
@Repiteo
Copy link
Contributor

Repiteo commented Oct 16, 2024

The commits need to be squashed into one before this can be merged. See here for one explanation on how to do do that.

@Repiteo Repiteo merged commit 22822f7 into godotengine:master Oct 21, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Oct 21, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release documentation enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants