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

Fix description of Vector2/3.dot #93195

Merged
merged 1 commit into from
Jun 17, 2024
Merged

Conversation

shak2
Copy link
Contributor

@shak2 shak2 commented Jun 15, 2024

Corrected mathmatical language - Straight angle is 180 degrees. Right angle is 90 degrees.

@shak2 shak2 requested a review from a team as a code owner June 15, 2024 14:47
@AThousandShips AThousandShips changed the title Update Vector3.xml Fix description of Vector3.dot Jun 15, 2024
@AThousandShips AThousandShips added bug documentation cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Jun 15, 2024
@AThousandShips AThousandShips added this to the 4.3 milestone Jun 15, 2024
Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

Please update your commit to have a more descriptive message, like the title of the PR

This will also need to be done for Vector2

@AThousandShips
Copy link
Member

If you need help with fixing the commit please ask

@shak2
Copy link
Contributor Author

shak2 commented Jun 15, 2024

You fixed the commit title, then asked for the commit title to be changed. I am new and not sure what this all means.

Please auto fix all that needs fixing, thanks.

@AThousandShips
Copy link
Member

AThousandShips commented Jun 15, 2024

The commit hasn't changed, you updated the description of the commit, and I updated the title of the PR (that's why i asked you to match it after I fixed the title)

If you're unsure how to do something please say so, it's impossible to tell that you don't know what I'm talking about without you saying so

I'll update your commit for you but in the future please do so yourself, and ask if you need instructions for how to do it (it's not auto fixing I have to do so manually)

A 90 degree angle is a right angle.
@AThousandShips
Copy link
Member

There fixed! But in the future please use the command line or desktop version to make updates and edits, see here

@AThousandShips AThousandShips changed the title Fix description of Vector3.dot Fix description of Vector2/3.dot Jun 15, 2024
@shak2
Copy link
Contributor Author

shak2 commented Jun 15, 2024

Last question: I don't understand this:

"The commit hasn't changed, you updated the description of the commit, and I updated the title of the PR (that's why i asked you to match it after I fixed the title)"

and

"Please update your commit to have a more descriptive message, like the title of the PR"

I am not sure what should have matched what here? should the description match the title? Is pr title different from the commit title and those were not matching?

I just followed the web UI process...

@AThousandShips
Copy link
Member

AThousandShips commented Jun 15, 2024

The commit message is the thing that's above my earlier message that says 451d099 at the end, you didn't change that by updating the branch on your fork of godot, there are:

  • The PR title (that's at the top of the page)
  • The PR description (the "Corrected mathmatical language - Straight angle is 180 degrees. Right angle is 90 degrees.") part
  • The commit message, which is separate

It's okay, but we prefer not using the web UI because it's very limited and you should instead use the desktop feature (see the instructions above) for your contributions

At least the title of the commit (when you press the "Commit changes..." when editing) should be a good description and not just the "Updated FILE" :)

But the most important lesson for the future is:

  • If you don't understand an instruction or feedback, ask!

@timothyqiu
Copy link
Member

The dot product will be [code]0[/code] for a straight angle (90 degrees), greater than 0 for angles narrower than 90 degrees and lower than 0 for angles wider than 90 degrees.

I think the original wording can be simplified. It mentions and explains "a right angle", but uses degrees directly for acute and obtuse angles.

Either

The dot product will be [code]0[/code] for a 90-degree angle, greater than 0 for angles narrower than 90 degrees and lower than 0 for angles wider than 90 degrees.

or

The dot product will be [code]0[/code] for a right angle (90 degrees), greater than 0 for acute angles (narrower than 90 degrees) and lower than 0 for obtuse angles (wider than 90 degrees).

looks better.

@AThousandShips
Copy link
Member

Let's do that in a follow up though

@akien-mga akien-mga merged commit 4671bbc into godotengine:master Jun 17, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@shak2 shak2 deleted the patch-2 branch June 17, 2024 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants