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

Tweak SceneTree Documentation #68649

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

Mickeon
Copy link
Contributor

@Mickeon Mickeon commented Nov 14, 2022

Introduction

Continuation of #67072, #67100, #67208, #67718,... #67880 and #68560...

Here a big list of changes. Someone reading this may take it as a future point of reference. Not just that, but some information deemed important by the maintainers could have been lost during the rewrite. Criticism is encouraged.

Do note that some exceptions or unexplained changes may be present all around. These are typically done to improve the flow of the sentence.

General

  • Made the wording match how other classes are documented a lot more closely.

  • Made use of [param] and several other strong references more often.

  • Stripped awkward, needlessly long sentences.

  • Changed words where it felt necessary, where they could be mistaken for another concept of Godot's API, where technical terms are more appropriate. For example:

    • "whenever" -> "when";
    • "group members"/"members of group" -> "nodes added to".
      • Saying "members" can make it seem like a new concept or "mechanic" is being introduced, but it really is something exclusive between Node and SceneTree, so it is very worth being specific here.
  • Replaced several self-links to this class ([SceneTree]) with "tree".

    • The consensus is that self-links within the same class end up making descriptions much less readable, when the subject is "this current object". From RocketChat:

    KoBeWi: It's used, but not consistently. Probably better to not overuse it.

    Me: Like, it works and may be nice for methods like get_node_and_resource() because there, you have to explain the return value accurately. So you do not say "the node" or "a node", you say [Node] because you refer to a Node class object, and in those cases it makes it a lot clearer.

    YuriSizov: I agree that sometimes this gets obnoxious and that we shouldn’t use it when talking about specific instances of a class [...] this looks a bit misleading, as if [Tween] is a singleton of sorts.

Methods

create_timer

    ...
  • Made parameter descriptions match the rest of the documentation.
  • Merged footnote into the first paragraph.

get_frame

"Returns the current frame number [...]"
  • Thank you...?

get_nodes_in_group

"Returns a list of all nodes assigned to the given group."
  • So all of them? Every single one in the whole engine?

has_group

"Returns true if the given group exists."
  • This is closer to how it works internally. Saying it like this is incredibly vague. You cannot even fetch all groups present in the SceneTree.

Properties

paused

"_process, _physics_process and _input will not be called anymore in nodes."
  • Not quite. This is not accounting for all of the various Node.process_modes available.

root

  • Expanded description considerably.
  • Added crash warning.

Signals

tree_process_mode_changed Made consistent with other signal descriptions.

Constants

GROUP_CALL_DEFAULT
GROUP_CALL_REVERSE
GROUP_CALL_DEFERRED
GROUP_CALL_UNIQUE
"Call a group"
  • Groups are not objects and cannot be called. Let's not muddy the waters.

Feedback is very, very welcome.

doc/classes/SceneTree.xml Outdated Show resolved Hide resolved
doc/classes/SceneTree.xml Outdated Show resolved Hide resolved
doc/classes/SceneTree.xml Outdated Show resolved Hide resolved
doc/classes/SceneTree.xml Outdated Show resolved Hide resolved
doc/classes/SceneTree.xml Outdated Show resolved Hide resolved
@Mickeon Mickeon force-pushed the doc-peeves-sweet-tree-scenery branch from 9d7829f to eafff97 Compare November 14, 2022 16:28
@Mickeon
Copy link
Contributor Author

Mickeon commented Nov 14, 2022

@RedMser Check revisions of the issues above if you'd like.

Copy link
Contributor

@RedMser RedMser left a comment

Choose a reason for hiding this comment

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

The changes look great! :)

@Mickeon Mickeon marked this pull request as ready for review November 20, 2022 10:38
@Mickeon Mickeon requested a review from a team as a code owner November 20, 2022 10:38
@Mickeon Mickeon force-pushed the doc-peeves-sweet-tree-scenery branch from 9e3d9dd to 5a84261 Compare November 20, 2022 10:39
doc/classes/SceneTree.xml Outdated Show resolved Hide resolved
@Mickeon Mickeon force-pushed the doc-peeves-sweet-tree-scenery branch 3 times, most recently from 3fc60b6 to 3a68f52 Compare November 25, 2022 14:02
@Mickeon Mickeon marked this pull request as draft November 25, 2022 14:43
@Mickeon
Copy link
Contributor Author

Mickeon commented Nov 25, 2022

Noticed a few mistakes, feeling like more can be done about them in the upcoming days.

@Mickeon Mickeon force-pushed the doc-peeves-sweet-tree-scenery branch from 3a68f52 to c94b78f Compare December 2, 2022 21:05
@Mickeon Mickeon marked this pull request as ready for review December 2, 2022 21:05
@Mickeon Mickeon force-pushed the doc-peeves-sweet-tree-scenery branch from c94b78f to 878cb54 Compare December 2, 2022 21:12
@Mickeon Mickeon force-pushed the doc-peeves-sweet-tree-scenery branch from 878cb54 to 01f35a2 Compare February 11, 2023 21:33
@Mickeon
Copy link
Contributor Author

Mickeon commented Feb 11, 2023

Rebased.

@Mickeon Mickeon force-pushed the doc-peeves-sweet-tree-scenery branch from 01f35a2 to 4137004 Compare April 5, 2023 14:30
@Mickeon
Copy link
Contributor Author

Mickeon commented Apr 5, 2023

Rebased (again)...!

@Mickeon Mickeon force-pushed the doc-peeves-sweet-tree-scenery branch from 4137004 to 988086e Compare July 31, 2023 19:20
@Mickeon
Copy link
Contributor Author

Mickeon commented Jul 31, 2023

Rebased. I'm quite content with this.

@Mickeon Mickeon force-pushed the doc-peeves-sweet-tree-scenery branch from 988086e to 552f7d9 Compare September 17, 2023 09:02
@Mickeon
Copy link
Contributor Author

Mickeon commented Sep 17, 2023

Rebased after further tweaks to the documentation had been made.

doc/classes/SceneTree.xml Outdated Show resolved Hide resolved
doc/classes/SceneTree.xml Outdated Show resolved Hide resolved
doc/classes/SceneTree.xml Outdated Show resolved Hide resolved
doc/classes/SceneTree.xml Outdated Show resolved Hide resolved
@Mickeon Mickeon force-pushed the doc-peeves-sweet-tree-scenery branch from 552f7d9 to e6e7be7 Compare September 17, 2023 13:00
@Mickeon
Copy link
Contributor Author

Mickeon commented Sep 17, 2023

Pushed changes for the C# suggestions above. Thank you for the additional attention to this PR.

@Mickeon
Copy link
Contributor Author

Mickeon commented Feb 26, 2024

Shockingly there's no conflicts. Are we okay to have this for 4.3?

Copy link
Member

@mhilbrunner mhilbrunner left a comment

Choose a reason for hiding this comment

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

LGTM. Maybe a rebase to current master would be a good idea still, but this looks good to go.

@Mickeon Mickeon force-pushed the doc-peeves-sweet-tree-scenery branch from e6e7be7 to 03400d0 Compare February 26, 2024 15:28
@Mickeon
Copy link
Contributor Author

Mickeon commented Feb 26, 2024

Gonna go through a second pass through history myself to see if any information was lost accidentally.

@Mickeon
Copy link
Contributor Author

Mickeon commented Feb 26, 2024

Pheeeuw. There are several commits such as:

that added new documentation and I'm not particularly fond of the way they are worded. I would tweak it more, but no more delaying this PR.

With that said, yes. 0ce0c11 was "lost". For SceneTree's has_group:

Returns [code]true[/code] if the given group exists.
A group exists if any [Node] in the tree belongs to it (see [method Node.add_to_group]). Groups without nodes are removed automatically.

Which my PR simply summarizes... I'd say a bit better without needing to spell it out too much? I can let people decide:

Returns [code]true[/code] if a node added to the given group [param name] exists in the tree.

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Feb 27, 2024
@akien-mga akien-mga merged commit 7eddf84 into godotengine:master Feb 27, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@Mickeon Mickeon deleted the doc-peeves-sweet-tree-scenery branch February 27, 2024 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants