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

Add get_tree_string and get_tree_string_pretty methods to Node to complement printing methods #73421

Closed
wants to merge 1 commit into from

Conversation

vPumpking
Copy link
Contributor

@vPumpking vPumpking commented Feb 16, 2023

Add two methods to Node class:

  • get_tree_string that allows to return a string containing the tree in shape of a list
  • get_tree_string_pretty that makes the same thing but in a more convenient shape.
    these methods complement the print_tree and print_tree_pretty methods.

proposal: godotengine/godot-proposals#6288

(succesfully works with the latest code)

@vPumpking vPumpking requested a review from a team as a code owner February 16, 2023 09:24
@akien-mga akien-mga added this to the 4.x milestone Feb 16, 2023
@akien-mga
Copy link
Member

Is there a feature proposal for this? https://github.com/godotengine/godot-proposals
We need one to gauge interest from the community in adding more core methods.

@vPumpking
Copy link
Contributor Author

vPumpking commented Feb 16, 2023

Is there a feature proposal for this? https://github.com/godotengine/godot-proposals We need one to gauge interest from the community in adding more core methods.

yes it is: godotengine/godot-proposals#6288

@jmb462
Copy link
Contributor

jmb462 commented Feb 16, 2023

Old _print_string_pretty method code is now redondant. You should call your new method inside public print_tree_pretty() to prevent duplicating logic, then _print_string_pretty will also not used anymore and should be removed.

Same thing with the not pretty methods.

@vPumpking
Copy link
Contributor Author

Old _print_string_pretty method code is now redondant. You should call your new method inside public print_tree_pretty() to prevent duplicating logic, then _print_string_pretty will also not used anymore and should be removed.

Same thing with the not pretty methods.

yes but doing that will make my PR destructive and less easy to approve

@vPumpking
Copy link
Contributor Author

Old _print_string_pretty method code is now redondant. You should call your new method inside public print_tree_pretty() to prevent duplicating logic, then _print_string_pretty will also not used anymore and should be removed.
Same thing with the not pretty methods.

yes but doing that will make my PR destructive and less easy to approve

ok no I understand what you means
I will maybe make it

vPumpking

This comment was marked as outdated.

@vPumpking
Copy link
Contributor Author

@akien-mga excuse me, can you relaunch the workflow verifications please? Yesterday I have commited my repository and it has stopped the verification.

@akien-mga
Copy link
Member

I would suggest squashing the commits first, as 12 commits is unnecessary for such a change.
See PR workflow for instructions.

@vPumpking
Copy link
Contributor Author

I would suggest squashing the commits first, as 12 commits is unnecessary for such a change. See PR workflow for instructions.

i will make that this afternoon

@vPumpking
Copy link
Contributor Author

I would suggest squashing the commits first, as 12 commits is unnecessary for such a change. See PR workflow for instructions.

here we go
squashing finished!

@vPumpking
Copy link
Contributor Author

@akien-mga so now can you relaunch the workflows please?

@vPumpking
Copy link
Contributor Author

vPumpking commented Feb 17, 2023

someone has an idea on why it doesn't work with the linux mono editor?

@vPumpking
Copy link
Contributor Author

ok yeah oops I forgot to update the Node doc
it will be made in minute

@vPumpking
Copy link
Contributor Author

finally all is in order.
sorry for all the time I took to mainteners,
the PR is now fully ready to be verified with the workflows. It may not push any error.

@vPumpking
Copy link
Contributor Author

what?! is there a place where I can read all the conditions to verify to be successfully checked?

@vPumpking
Copy link
Contributor Author

vPumpking commented Feb 20, 2023

I will rewrite the method references and verify it with make rst so as to have finally something that works with the workflows and to be merged.
Another time so sorry for the time that I spend to mainteners.

@vPumpking
Copy link
Contributor Author

the fail was in the fact that alphabetical order was not respected.
Now, the commit is truly ready to be run with workflows. I used the make rst command to verify it.
Finally.

@vPumpking
Copy link
Contributor Author

why doesn't it work?!

@Calinou
Copy link
Member

Calinou commented Feb 20, 2023

why doesn't it work?!

You have an extraneous blank line in the doc/classes/Node.xml class reference XML. There must be no blank lines in class reference files.

I recommend running path/to/godot.binary --doctool locally in the Godot repository root, so you can spot those errors earlier. See Contributing to the class reference for more information.

added `get_tree_string()` and `get_tree_string_pretty` to Node class
@vPumpking
Copy link
Contributor Author

vPumpking commented Feb 20, 2023

I think it's fixed up
thanks @Calinou

@vPumpking vPumpking deleted the branch godotengine:master May 14, 2023 15:53
@vPumpking vPumpking closed this May 14, 2023
@vPumpking vPumpking deleted the master branch May 14, 2023 15:53
@vPumpking
Copy link
Contributor Author

Sorry, I accidentally renamed the branch used by this PR, so now the PR is here: #77072
Can you rewrite the labels and topics please?

@vPumpking vPumpking restored the master branch May 14, 2023 16:15
@YuriSizov YuriSizov removed this from the 4.x milestone Dec 2, 2023
@vPumpking vPumpking deleted the master branch October 9, 2024 13:33
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.

5 participants