godotengine / godot-docs

Godot Engine official documentation
https://docs.godotengine.org
Other
3.81k stars 3.12k forks source link

Revise Best Practices #2592

Open TheDuriel opened 5 years ago

TheDuriel commented 5 years ago

@NathanLovato heres your ping.

below is an excert of the discord discussion regarding my issues with the new best practices section. overall its a great addition to the docs, but i think its missing out on explaining some vital principles for tree navigation, and features bad code examples.

unfiltered, raw, copy and paste:

the scene organization page... honestly i'd like it nuked and reworked from scratch using https://cdn.discordapp.com/attachments/319225525052899328/585172151913676800/HowToSceneTree.jpg

especially the funref example in that is terrible, and listing it above node paths is weird.

there are tons of examples of get_parent or ".." in the best practices section. getting a parent node is never a good practice unless you are specifically making something like a tooltip ui element. (where you can get the parent so the tooltip can position itself correctly.)

the godot interfaces page should make it explicit that it is not talking about Nodes, but Reference types in the funref example. and again, its using the scene tree badly with get_child(0) and get_parent() calls.

http://docs.godotengine.org/en/latest/getting_started/workflow/best_practices/godot_interfaces.html relevant pages [12:42 PM] TheDuriel [TTG]: calling out @willnationsdev since he wrote the page. and @gdquest cuz you're also a big docs contributer :P [12:52 PM] TheDuriel [TTG]: (just to be clear. the rest of the best practices is fairly good and i'm pretty happy with it) [2:02 PM] willnationsdev: I just wrote up my own thoughts and no one prompted me to change those things in the PR. Feel free to submit a PR to improve it! That's the whole reason I put the info in the publicly maintained docs rather than just my own blog or something. I'm not gonna think of/know everything. :-) @TheDuriel [TTG] [2:03 PM] willnationsdev: Happy to hear your feedback on it [2:19 PM] willnationsdev: I would recommend that if new docs for those are made, they should highlight all the options available to users (as I tried to do) and then recommend what out of those options is best for different situations, that way we can have comprehensive information. [2:20 PM] willnationsdev: I would recommend that if new docs for those are made, they should highlight all the options available to users (as I tried to do) and then recommend what out of those options is best for different situations, that way we can have comprehensive information. [2:21 PM] willnationsdev: Not sure if the existing ones need nuking, as you say, but it if there is faulty info, we should definitely amend or remove it. [3:43 PM] gdquest: Actually it's my fault, I said I'd merge and do editing work, as it's way too much work to leave individual comments and do review loops, but never found the time to get to it [3:44 PM] gdquest: Overall the pr was really valuable and I didn't want to let it stall for a long time because of the review process - with several pages in a single pr [3:46 PM] TheDuriel [TTG]: yeah most of it is good. just some bad practice examples mixed in with the lack of a good explanation of tree navigation. which imho is pretty vital. [3:46 PM] gdquest: Could you put an issue up and ping me there? I'll take care of it

[3:46 PM] TheDuriel [TTG]: will do [3:47 PM] gdquest: In practice Godot is really flexible and if there are some go-to ways to organize your projects with object oriented design in mind, e.g. getting parent nodes could be okay if you make it a rule (component-based code) [3:48 PM] gdquest: Even though I wouldn't recommend it, now we experimented with Razvan I can see how some things I thought would be bad ideas can totally work 3:50 PM] gdquest: E.g. signals can easily lead to spaghetti code, and bubbling information up the tree while keeping scenes tidy and encapsulated can be really tedious and complicate your code [3:50 PM] gdquest: Yet you could argue it's a best practice, that'd be the object oriented way to work with them, applying encapsulation [3:50 PM] TheDuriel [TTG]: aye, some situations require specialized solutions. but you need to have a good grasp on what "is proper", to be able to identify when you need a custom solution. [3:51 PM] TheDuriel [TTG]: and i think conveying that "proper" way is what the docs need most in this regard. since those questions keep comming up. [3:51 PM] gdquest: Yup, do these best practices mention that? I need a refresher on the articles [3:52 PM] TheDuriel [TTG]: not sure. but i did notice that they could be more explicit with "things you should do, and things you can do" [3:53 PM] gdquest: What I meant above is I'm not sure there are strong intended ways to use e.g. the node tree. I.e. that it was designed to be used in a really specific way [3:53 PM] TheDuriel [TTG]: like funcrefs, probably pointless with a static scenetree. but can be a great addition to a larger more complex structure. (theyre also listed before node paths, which is odd.) [3:53 PM] gdquest: But rather to be quite flexible and allow radically different structures [3:54 PM] TheDuriel [TTG]: ah i see [3:54 PM] gdquest: Even reading Juan I got that feeling [3:55 PM] TheDuriel [TTG]: tbh i, and the people i work with, pretty much swear by my chart https://cdn.discordapp.com/attachments/319225525052899328/585172151913676800/HowToSceneTree.jpg

and honestly: juan does not know how to use his own engine. :/ at least from the perspective of someone who is not going to modify source to get what they want.

[3:56 PM] TheDuriel [TTG]: note that this chart does not list every possible way. just the ones you should consider first, and which are worth to refactor for. before trying a different solution. [3:56 PM] gdquest: That's mostly how we work as well [3:57 PM] gdquest: Ah right so that's that, there's no precise intended way to use the nodes, but we're talking about teaching what works well in practice from experience? [3:58 PM] gdquest: Owner is a really important one to know, I almost haven't seen anyone use it [3:59 PM] TheDuriel [TTG]: yup. it really cleaned up my code when i started using it. [4:00 PM] gdquest: Time to go to sleep, but please ping me if you open an issue in the docs repo :slight_smile: [4:00 PM] gdquest: Good... Day, night, evening... [4:00 PM] TheDuriel [TTG]: :D see ya [4:00 PM] gdquest: :sleeping:

NathanLovato commented 5 years ago

First article done: https://github.com/godotengine/godot-docs/pull/2597

I'll slowly work my way through the entire section