Skip to content

Conversation

@FelixBurkhard
Copy link
Contributor

Hey everyone,

I just started exploring Godot and went through the Getting Started part. I noticed a few thing that I think should be changed:

  1. The Dodge the Creeps! string was not consistent through the page.
  2. The \n should be removed from the Label Text because the Label should do the wrapping.
  3. There were two reminders to remove the new_game() call with only a few lines in between.
  4. The async method in C# should probably return a Task instead of void. I am not 100% sure about this, as I didn't dig into how the method is actually called. Also I think this is fine if done intentionally to not confuse users who are new not just to Godot but to coding in general.

As someone who is new to game development I must say I really liked the Getting Started docs! A big thanks to all the contributors!

@paulloz
Copy link
Member

paulloz commented Oct 2, 2023

Hey 🙂

4. The async method in C# should probably return a Task instead of void. I am not 100% sure about this, as I didn't dig into how the method is actually called. Also I think this is fine if done intentionally to not confuse users who are new not just to Godot but to coding in general.

The callstack basically goes <signal> -> Main.GameOver() -> HUD.ShowGameOver().
If HUD.ShowGameOver() returns a Task, then we'd want to await it in the caller (and make it async too). It makes sense "idiomatically" I think, since it's the real end of the callback trail. In this particular case, I worry about it being confusing, since we declare Main.GameOver() in the previous chapter of the tutorial. We'd preemptively declare it as async, and would have to explain why that is 🤔 (and IMHO when async void is and isn't acceptable breaks the scope of the tutorial).

@FelixBurkhard
Copy link
Contributor Author

yeah I agree. didn't really think the callstack through 😄 -> reverted async part

@Calinou Calinou added bug cherrypick:4.1 area:getting started Issues and PRs related to the Getting Started section of the documentation labels Nov 9, 2023
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Thanks!

@Calinou Calinou merged commit d99a882 into godotengine:master Nov 9, 2023
mhilbrunner pushed a commit to mhilbrunner/godot-docs that referenced this pull request Nov 11, 2023
- The `Dodge the Creeps!` string was not consistent through the page.
- The `\n` was removed from the Label Text because the Label should do the wrapping.
- There were two reminders to remove the `new_game()` call with only a few lines in between.
@mhilbrunner
Copy link
Member

Cherry-picked to 4.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:getting started Issues and PRs related to the Getting Started section of the documentation bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants