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

[doc/classes/int] fix description of operator '~' #75325

Closed
wants to merge 1 commit into from

Conversation

umarcor
Copy link
Contributor

@umarcor umarcor commented Mar 25, 2023

The description of the ~ operation in class int is not correct. It states that a bitwise NOT operation results in -int + 1. In fact, due to how 2's complement works, the operation results in -(int +1). This PR updates the description and fixes the examples in the codeblock.

@umarcor umarcor requested a review from a team as a code owner March 25, 2023 16:20
@AThousandShips
Copy link
Member

AThousandShips commented Mar 25, 2023

I believe it's [url=address]text[/url], and your link tag is unterminated

@umarcor
Copy link
Contributor Author

umarcor commented Mar 25, 2023

@AThousandShips thanks! ❤️ I updated it with the fixed syntax.

@AThousandShips
Copy link
Member

You need to escape the arrows for html, ->, I'd recommend running --doctool locally to get these things

@MewPurPur
Copy link
Contributor

MewPurPur commented Mar 25, 2023

Related: #70046. I didn't catch this mistake in particular, but there are other mistakes the PR fixes.

As for this PR, I'm not too fond of the examples. I don't think we need that many of them, and the binary representations are also weird. Having only 6 bits to represent complement numbers is awkward, moreover I'm not sure if we should concern users with complement numbers in the first place.

@umarcor
Copy link
Contributor Author

umarcor commented Mar 25, 2023

You need to escape the arrows for html, ->, I'd recommend running --doctool locally to get these things

I tried running --doctool with the same release version of godot I use for developing games. Unfortunately that changed the version in many of the files in the doc subdir. Hence, I had to build godot from sources. That took some time. Nevertheless, I must say I was very gratefully surprised by how easy it was.

I now changed -> to -> through doctool and updated this PR. I wanted to do it with the tool rather than fixing it manually.

@umarcor
Copy link
Contributor Author

umarcor commented Mar 25, 2023

As for this PR, I'm not too fond of the examples. I don't think we need that many of them, and the binary representations are also weird. Having only 6 bits to represent complement numbers is awkward, moreover I'm not sure if we should concern users with complement numbers in the first place.

I reduced the examples to just one positive and one negative.

With regard to the word-length of the bit representation, since the sign is extended we can use any number of bits >=4. I'm good with using just 4 or 8, if 6 feels uncommon.

With regard to concerning users with complement numbers, that's fundamental for understanding logical operations on integers. The mistake this PR fixes would be avoided if the binary representation had been written/reviewed. I find it possitive (not concerning) that the reference might trigger the curiosity of users expecting the more human readable but uncommon sign+magnitude or ones' complement.

@umarcor umarcor force-pushed the umarcor/doc/int/not branch 2 times, most recently from a7c91a3 to b808651 Compare March 26, 2023 02:12
@AThousandShips
Copy link
Member

My bad I should have been clear that it had to be the built version!

doc/classes/int.xml Outdated Show resolved Hide resolved
Copy link
Contributor

@YuriSizov YuriSizov 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, except for a formatting issue.

@umarcor
Copy link
Contributor Author

umarcor commented Mar 27, 2023

I updated the comments to use the same literal syntax as in https://docs.godotengine.org/en/stable/tutorials/scripting/gdscript/gdscript_basics.html#literals (six bits prefixed with 0b). I also replaced -> with = (in a mathematical sense, not as an assignment).

@YuriSizov
Copy link
Contributor

Superseded by #70046. Thanks for your contribution nevertheless!

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.

4 participants