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

PascalCase naming inconsistencies with C# #28748

Closed
aaronfranke opened this issue May 8, 2019 · 4 comments · Fixed by #69547
Closed

PascalCase naming inconsistencies with C# #28748

aaronfranke opened this issue May 8, 2019 · 4 comments · Fixed by #69547
Assignees
Milestone

Comments

@aaronfranke
Copy link
Member

aaronfranke commented May 8, 2019

Godot version: 3.1.1 and master

Issue description: There are a lot of cases in the Mono module where the generated C# methods and properties are not using PascalCase properly. This is understandably due to this information not being present when these names are written out in snake_case. See also: #32734

In some cases, this also effects the editor, as the displayed name could be improved:

1

Here's my attempt to document what could be fixed:

Node:

  • Callv -> CallV
  • Editor__displayFolded -> Don't expose this?
  • Rpc etc -> RPC etc
  • Rset etc -> RSet etc?

Node2D:

  • GetWorld2d -> GetWorld2D

Label:

  • Valign -> VAlign
  • Autowrap -> AutoWrap

Particles and Particles2D:

  • FixedFps -> FixedFPS

PathFollow:

  • RotationModeEnum Xy -> XY and Xyz -> XYZ

PathFollow2D:

  • Lookahead -> LookAhead

Polygon2D:

  • Antialiased -> AntiAliased
  • Uv -> UV

RichTextLabel:

  • Bbcode etc -> BBCode etc

Sprite:

  • Hframes -> HFrames and get/set
  • Vframes -> VFrames and get/set

TextEdit:

  • Readonly -> ReadOnly

TileMap:

  • CellClipUv -> CellClipUV
  • GetCellv etc -> GetCellV or just GetCell (overloaded method)

TouchScreenButton:

  • Bitmask -> BitMask
  • TouchscreenOnly -> TouchScreenOnly

Tree:

  • AllowRmbSelect -> AllowRMBSelect

Various players:

  • Autoplay -> AutoPlay

VideoPlayer:

  • BufferingMsec -> BufferingMSec

VisibilityNotifier:

  • Aabb -> AABB

Misc:

  • Aot -> AOT
  • Rid -> RID in many places.
@neikeq neikeq added this to the 3.2 milestone May 8, 2019
@aaronfranke
Copy link
Member Author

aaronfranke commented Nov 8, 2019

Probably not going to be done for 3.2, and it would break C# compatibility, I suggest bumping to 4.0.

@akien-mga akien-mga modified the milestones: 3.2, 4.0 Nov 8, 2019
@neikeq
Copy link
Contributor

neikeq commented Mar 27, 2020

Guess we can add a list of custom casing for words like these.

@neikeq
Copy link
Contributor

neikeq commented Apr 7, 2022

I'm not keen of going all upper case for acronyms and some other words, like the following: AABB, AOT, RID, XYZ, RPC, HTML, HTTP, etc.

It can result in hard to read names:

  • IsValidHTMLColor vs IsValidHtmlColor
  • RPCCallMode vs RpcCallMode

I tend to agree with what is said here: https://www.approxion.com/capital-offenses-how-to-handle-abbreviations-in-camelcase/

The convention in .NET seems to be capitalizing only the first letter (e.g.: HttpClient), except for two-letter acronyms (e.g.: IO and 2D) where all letters are capitalized: https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/capitalization-conventions

Personally, I'd prefer it without the two-letter exception (I don't have a problem with Io and 2d). However, in this case, it's better to follow the convention. Specially now that so many node classes have 2D and 3D suffixes, which would make the difference with 2d and 3d in members too visible.

The only downside of following this convention is that currently we keep the original casing of Godot class names (e.g.: HTTPRequest). But for me that little inconsistency is ok, and I'd prefer not to keep that casing.

@marcinWlodarski
Copy link

100% agree with @neikeq - also bear in mind that https://en.wikipedia.org/wiki/Filename is a real word, having "file name" (and file_name, FileName, fileName etc.) was always a bit strange for me, since we have a single common word describing exactly what we have, and not some abstract "name" of a file.

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

Successfully merging a pull request may close this issue.

4 participants