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.

Sign up for GitHub

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

Jump to bottom

Clarify Area2D warning behavior #10173

Merged
merged 2 commits into from
Nov 21, 2024
Merged

Clarify Area2D warning behavior #10173

merged 2 commits into from
Nov 21, 2024

Conversation

Kushal-Dev94
Copy link
Contributor

Improvements:

  1. Explained why the warning icon appears on the Area2D node and why it can be ignored at first.
  2. Added a note that the warning will go away once the CollisionShape2D is added.
  3. Included an image (no_shape_warning.webp) to show the warning icon for clarity.

@tetrapod00
Copy link
Contributor

This addition should maybe be in a .. note:: block, not in the main body text of the page. Many readers will be okay with ignoring the warning until the tutorial fixes it. Only some readers will care enough about why the warning exists, and the deeper explanation of the note bock is there for them.

tetrapod00
tetrapod00 approved these changes Nov 2, 2024
View reviewed changes
Copy link
Contributor

@tetrapod00 tetrapod00 left a comment

Choose a reason for hiding this comment

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

This seems basically fine, though I'm not familiar with specific conventions or standards used by the Getting Started section.

I would consider splitting off information about the warning into a note block, since the page can be read just fine without this information.

getting_started/first_2d_game/02.player_scene.rst Outdated Show resolved Hide resolved
getting_started/first_2d_game/02.player_scene.rst Outdated Show resolved Hide resolved
getting_started/first_2d_game/02.player_scene.rst Outdated Show resolved Hide resolved
@skyace65 skyace65 added enhancement area:getting started Issues and PRs related to the Getting Started section of the documentation cherrypick:4.3 labels Nov 3, 2024
tetrapod00
tetrapod00 approved these changes Nov 21, 2024
View reviewed changes
Copy link
Contributor

@tetrapod00 tetrapod00 left a comment

Choose a reason for hiding this comment

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

Looks good to me. I think this addition clarifies some confusion in the Getting Started section, and is not too much of a side-tangent.

Optionally I can see ways to improve it with a note block, but it is good as-is.

Calinou
Calinou approved these changes Nov 21, 2024
View reviewed changes
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.

Looks good to me.

@tetrapod00 tetrapod00 merged commit b82187d into godotengine:master Nov 21, 2024
1 check passed
@tetrapod00
Copy link
Contributor

Thank you! Congratulations on your first merged contribution 🎉

mhilbrunner pushed a commit that referenced this pull request Nov 30, 2024
@Kushal-Dev94 @tetrapod00 @mhilbrunner
Clarify Area2D warning behavior (#10173)
032be9e 
* Clarify Area2D warning behavior

* Apply suggestions from code review

---------

Co-authored-by: tetrapod <[email protected]>
@mhilbrunner
Copy link
Member

Cherrypicked to 4.3 in #10347.

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

@tetrapod00 tetrapod00 tetrapod00 approved these changes

@Calinou Calinou Calinou approved these changes

Assignees
No one assigned
Labels
area:getting started Issues and PRs related to the Getting Started section of the documentation enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Kushal-Dev94 @tetrapod00 @mhilbrunner @Calinou @skyace65