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

pygame.Surface docs improvements #2911

Merged
merged 6 commits into from
Jun 15, 2024
Merged

Conversation

REX2626
Copy link
Contributor

@REX2626 REX2626 commented Jun 5, 2024

When reading through the pygame.Surface docs I noticed the following issues/inconsistencies:

  1. One line descriptions of Surface.blits() and Surface.fblits() seemed inconsistent with other descriptions.
  2. Surfaces were sometimes referred to as "images", which didn't make sense to me.
  3. Some of the one line method descriptions started with a capital letter, which was inconsistent with the other descriptions.
  4. Sometimes Surface was captilalized and sometimes it wasn't. I changed these all to capitalized versions.
  5. In the pygame.Surface.convert() description, there was an 'as' when I think there should be an 'a'.

I'm open to any suggestions or thoughts about the changes, whether the Surfaces should be capitalized or not etc..

@REX2626 REX2626 requested a review from a team as a code owner June 5, 2024 19:44
Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

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

I like all the changes, except maybe the surface -> Surface change. "surface" is not a proper noun, and to my eyes capitalization of it looks weird.
However, I'm not native english speaker so maybe we should wait for other members to put in their opinions on this.

Either way, it would be better if that change was a separate PR, the rest of the changes LGTM

@ankith26 ankith26 added docs Surface pygame.Surface labels Jun 6, 2024
@REX2626
Copy link
Contributor Author

REX2626 commented Jun 6, 2024

Yep, makes sense. I've reverted the surface capitalization back to what is previously was.

Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for contributing! 🎉

@Matiiss
Copy link
Member

Matiiss commented Jun 7, 2024

I think we need to decide on the capitalization now, because there are tons of places in this file where it's called a "Surface" and where it's called a "surface", so we can be consistent and lowercase it everywhere (besides type annotations and if they begin a sentence obviously).

| :sl:`draw many surfaces onto the calling surface at their corresponding location and the same special_flags`
| :sl:`draw many surfaces onto this surface at their corresponding location and with the same special_flags`
Copy link
Member

Choose a reason for hiding this comment

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

Personally I find "the calling surface" to be clearer than "this surface" because it might not be clear which surface "this" is referring to exactly. This goes for all such occurrences in this file.

Copy link
Member

Choose a reason for hiding this comment

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

TBH I prefer the current change. To me "calling surface" is not exactly clear, whereas the "this" keyword is used in many programming languages to mean "self"

Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@MyreMylar MyreMylar merged commit a95e809 into pygame-community:main Jun 15, 2024
39 checks passed
@ankith26 ankith26 added this to the 2.5.1 milestone Jun 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Surface pygame.Surface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants