godot: [TRACKER] Methods, properties and signals to consider for renaming in next planned compatibility breakage
This issue is meant to keep track of awkwardly named or deprecated methods, properties and signals that we would like to rename next time we have the opportunity.
This can’t be done lightly as it breaks compatibility for users using their old names, so any such change has to be done:
- either after following a deprecation procedure: marked as deprecated - using it shows a warning - for at least one full minor release cycle (e.g. 3.1.x), then removed in a future minor or major version bump,
- or within a major compatibility breaking release (such as 3.0 was to 2.1; but such situations won’t happen often - if ever again - so the deprecation workflow should be preferred).
To properly deprecate methods, properties and signals, we need #4397 implemented.
A 🎉 reaction added by @akien-mga or @Calinou means the suggestion in the comment was incorporated into this post.
Classes
~- [ ] OS*
could be renamed to Platform*
https://github.com/godotengine/godot/issues/16863#issuecomment-403253127~ (See https://github.com/godotengine/godot/pull/47789#issuecomment-854149653)
~- [ ] Label
: Consider renaming to TextLabel
https://github.com/godotengine/godot/issues/16863#issuecomment-502517425~ (see https://github.com/godotengine/godot/pull/40124#issuecomment-751741421)
-
PHashTranslation
should be renamed to ~CompressedTranslation
~OptimizedTranslation
(matching its header) PR:#45234 -
ResourceFormat*
: review all classes inheritingResourceFormatLoader
,ResourceFormatSaver
andImageFormatLoader
to check that they follow the same naming convention (both class and filename) -
ShortCut
should be renamed toShortcut
https://github.com/godotengine/godot/issues/16863#issuecomment-685236980 #41926 -
TCP_Server
andIP_Unix
should be renamed toTCPServer
andIPUnix
PR:#37700 -
Viewport
should be looked over, it is very complex and could likely be simplified https://github.com/godotengine/godot/issues/16863#issuecomment-631789777
Methods
-
@GDScript
(and several other places):instance
when used as a verb/action should beinstantiate
https://github.com/godotengine/godot/issues/16863#issuecomment-367061984 PR:#49693 -
@GDscript
:decimals
should be renamed tostep_decimals
#21425 -
@GDscript
:stepify
should be renamed tosnapped
for consistency with vectors https://github.com/godotengine/godot/issues/16863#issuecomment-655258916 PR:#44586 -
AcceptDialog
andConfirmationDialog
:get_ok
andget_cancel
should beget_ok_button
andget_cancel_button
(matchingWindowDialog.get_close_button
) https://github.com/godotengine/godot/issues/16863#issuecomment-421071469 PR:#44389 -
AnimatedSprite2D
andAnimatedSprite3D
:stop
should bepause
https://github.com/godotengine/godot/issues/31168 PR:#44369 -
Animation
:track_remove_key_at_position
should betrack_remove_key_at_time
PR:#44372 -
AnimationPlayer
:play_backwards
could be removed https://github.com/godotengine/godot/issues/16863#issuecomment-490298187 PR:#44345 -
Area
:set_audio_bus
andget_audio_bus
should be renamed toset_audio_bus_name
andget_audio_bus_name
to match the related property and theirArea2D
counterparts #29670 PR:#44260 -
Array
(some changes also apply to PackedArrays) https://github.com/godotengine/godot/issues/16863#issuecomment-441376010:- Rename
remove
toremove_at
(remove by index) to avoid ambiguity - Rename
erase
toremove_value
(remove by value) to avoid ambiguity - Rename
invert
toreverse
to use more established naming PR:#46991 - ~Rename
duplicate
toclone
to use more established naming~ (See https://github.com/godotengine/godot/pull/46996#issuecomment-854195903) - Rename
empty
tois_empty
to clearly indicate it’s a boolean check and not an operation emptying the array PR:#44401
- Rename
-
Camera
:set_znear
andset_zfar
should be renamed to match thenear
andfar
properties https://github.com/godotengine/godot/issues/16863#issuecomment-412810788 PR:#44434 -
Control
: Discrepancy between property names and their setter/getter names https://github.com/godotengine/godot/issues/16863#issuecomment-381420942 PR:#47248 -
CollisionShape
:make_convex_from_brothers
should bemake_convex_from_siblings
https://github.com/godotengine/godot/issues/16863#issuecomment-493794313 PR:#41044 -
EditorInterface
:get_editor_viewport
could beget_editor_main_screen
https://github.com/godotengine/godot/issues/16863#issuecomment-634258502 + 2 following comments PR:#44524 -
GridMap
:set_cell_item
(3int
s) should be replaced by a version withVector3i
#39958 -
InputMap
:load_from_globals
should beload_from_project_settings
https://github.com/godotengine/godot/issues/16863#issuecomment-426457888 PR:#43661 -
ItemList
:unselect
andunselect_all
should bedeselect
anddeselect_all
, matching other classes with similar methods. There’s alsodeselect_items
inFileDialog
, harmonize this #28558 PR:#44569 -
JSON
:print
should be rename to something else https://github.com/godotengine/godot/issues/16863#issuecomment-557657413 + the following 6 comments PR:~#44574~ #44806 -
KinematicBody
andKinematicBody2D
: The API grew organically and is becoming quite convoluted, a refactor/reorder of some method arguments might be welcome (see e.g. #16757 #19648). -
MainLoop
:_iteration
should be renamed to_physics_process
,_idle
should be_process
. Non virtual methods should be unexposed, andinput_text
does nothing and should be removed #30096 PR:#44593 ~- [ ]Mesh
:surface_get_material
->get_surface_material
andsurface_set_material
->set_surface_material
https://github.com/godotengine/godot/issues/16863#issuecomment-652067884~ See https://github.com/godotengine/godot/issues/16863#issuecomment-747269154 -
Node
:get_index
andget_position_in_parent
are synonyms, one should be removed #21802 #37556 -
Node
:is_a_parent_of
should be replaced byis_ancestor_of
oris_descendant_of
https://github.com/godotengine/godot/issues/16863#issuecomment-564532042 -
Node
:set_as_toplevel
could beset_as_top_level
, but its behavior may also need tweaking https://github.com/godotengine/godot/issues/16863#issuecomment-498428024 https://github.com/godotengine/godot/issues/24154 #42451 -
Node2D
andNode3D
: Inconsistency with object-local translation code https://github.com/godotengine/godot/issues/16863#issuecomment-530519327 ~- [ ]OptionButton
:get_selected_id
might be obsolete after #21837~ See https://github.com/godotengine/godot/issues/16863#issuecomment-747413919 -
OS
:can_draw
would be better suited in theEngine
singleton -
OS
:execute
should be split in two different methods, one blocking and the other non-blocking. e.g. names:execute
/exec_process
(blocking) andspawn_process
(non-blocking) #19302 PR:#44514 - Physics (various classes):
add_force
andapply_impulse
methods need harmonization of their arguments order https://github.com/godotengine/godot/issues/16863#issuecomment-416791048 #37350 -
Quat
: Consider deprecating set methods https://github.com/godotengine/godot/issues/16863#issuecomment-515892880 -
Rect2
: Renameclip
tointersection
for consistency withAABB
. https://github.com/godotengine/godot/issues/16863#issuecomment-655299536 PRs: #44521 #44582 -
RichTextLabel
:set_use_bbcode
andset_bbcode
should be renamed toset_use_fixed_bbcode
andset_fixed_bbcode
. Warnings should be thrown if bbcode is modified by another function #19118 -
Skeleton
:set_bone_global_pose
should be renamed toset_bone_skeleton_pose
(same for the getter).get_bone_transform
should also be renamed, or dropped if unnecessary #19551 -
Sprite
,Sprite3D
:set_region
andis_region
should be renamed toset_region_enabled
andis_region_enabled
https://github.com/godotengine/godot/issues/16863#issuecomment-380225780 PR:#47001 -
String
:ord_at
considered unclear, proposal to rename it tounicode_at
#15519 PR: #43790 -
String:
right
behaviour is unintuitive and mostly duplicate ofsubstr
https://github.com/godotengine/godot/issues/16863#issuecomment-419635744 PR: ~#47434~ #36180 -
String
: Renamehttp_escape
touri_encode
,http_unescape
touri_decode
https://github.com/godotengine/godot/issues/16863#issuecomment-503491938 PR:#43978 -
String
: Renameempty
tois_empty
PR: #44401 -
Texture2D
:get_data
should beget_image
https://github.com/godotengine/godot/issues/16863#issuecomment-652064475 PR: #47435 -
TileMap
:set_y_sort_mode
andis_y_sort_mode_enabled
should be renamed toset_y_sort_enabled
andis_y_sort_enabled
https://github.com/godotengine/godot/issues/16863#issuecomment-380250110 #38635 -
TileMap
: Discrepancy between property names and their setter/getter names https://github.com/godotengine/godot/issues/16863#issuecomment-382545668 -
TileMap
:set_cell
(2int
s) andset_cellv
(Vector2
) should be replaced by a version withVector2i
#39976 -
Tree
:get_selected
should be renamed to something likeget_active
https://github.com/godotengine/godot/issues/16863#issuecomment-425712040 -
Tween
: Many methods returnbool
for no reason, should be changed tovoid
https://github.com/godotengine/godot/issues/16863#issuecomment-420286639 #36844 -
UndoRedo
:is_commiting_action
has a typo, should be “committing” -
VisualServer
:sync
anddraw
bindings should be deprecated in favour of the existingforce_sync
andforce_draw
#15892 -
Vector2
:tangent
is considered ambiguous/inaccurate, it should beperpendicular
https://github.com/godotengine/godot/issues/16863#issuecomment-618294043 PR: #39685 or #44149 -
XRPositionalTracker
:get_type
->get_tracker_type
andget_name
->get_tracker_name
https://github.com/godotengine/godot/issues/16863#issuecomment-571283702 https://github.com/godotengine/godot/issues/15763#issuecomment-568908661 #36382 https://github.com/godotengine/godot/issues/16863#issuecomment-494437342
Properties
-
BoxShape
,CubeMesh
andCSGBox
: their dimension properties are inconsistent, andCubeMesh
should maybe be renamed toBoxMesh
https://github.com/godotengine/godot/issues/16863#issuecomment-403253127 PR:#44091 and #44183 -
Camera2D
:offset
andoffset_h
/offset_v
are confusingly named as they refer to completely different things. It should likely be harmonized withCamera
too which hash_offset
andv_offset
https://github.com/godotengine/godot/issues/16863#issuecomment-407133383 #7489 PR:#44232 -
Camera2D
:limit_
values could be changed to aRect2i
or similar https://github.com/godotengine/godot/issues/16863#issuecomment-595585595 -
Control:
Renamemargin
tooffset
now that they can be negative? https://github.com/godotengine/godot/issues/16863#issuecomment-401824810 PR:#44605 -
Control:
rect_rotation
is in degrees, so it should be renamed torect_rotation_degrees
to matchNode2D
’srotation_degrees
, and a newrect_rotation
property should be added which uses radians. PR:#44607 ~- [ ]CPUParticles2D
: Renamenormalmap
tonormal_map
for consistency https://github.com/godotengine/godot/issues/16863#issuecomment-615254863~ Removed as part of #43052 -
Engine
: Renameiterations_per_second
tophysics_fps
to match the project setting of the same name https://github.com/godotengine/godot/issues/16863#issuecomment-554682554 https://github.com/godotengine/godot/pull/41956 -
ImageTexture
:lossy_quality
should be changed to an enum (low, mid, high, etc.) instead of a float ratio interpreted as arbitrary plateaus (same inImage::compress()
) https://github.com/godotengine/godot/pull/21167#issuecomment-414234160 -
LightOccluder2D
:light_mask
->occluder_light_mask
https://github.com/godotengine/godot/issues/16863#issuecomment-571283702 https://github.com/godotengine/godot/issues/15763#issuecomment-568908661 https://github.com/godotengine/godot/pull/36382 -
Label
andButton
:clip_text
is not necessary anymore, as all Controls haverect/clip_content
#20228 -
LineEdit
:cursor_*
properties should be renamed tocaret_*
#16116 PR:#47448 -
LineEdit
andTextEdit
: Their respective APIs could be homogenized as far as possible https://github.com/godotengine/godot/issues/16863#issuecomment-409058538 #20611 -
Node2D
,Spatial
: inconsistency betweenposition
(2D) andtranslation
(3D) #9128 PR:#44198 -
NoiseTexture
: Renameas_normalmap
toas_normal_map
for consistency https://github.com/godotengine/godot/issues/16863#issuecomment-615254863 PR:#44614 -
RayCast
: Renamecast_to
totarget_position
https://github.com/godotengine/godot/issues/16863#issuecomment-676512989 -
RayCast
and others: Changedisabled
properties toenabled
ones https://github.com/godotengine/godot/issues/16863#issuecomment-511037393 + the following 2 comments PR:#44141 -
Resource
:resource_name
is not used, should be dropped #13358 -
TileMap
:collision_friction
property should be renamed tofriction
#18191
Signals
-
CanvasItem
:hide
should be renamed tohidden
PR:#44189 -
Tabs
:tab_close
andtab_hover
should be spelledtab_closed
andtab_hovered
PR:#44188 -
Tree
:item_activated
(label double-click) anditem_double_clicked
(icon double-click), names don’t properly convey what they do #16839: PR:#44185 -
XRController
:button_release
should be spelledbutton_released
(likebutton_pressed
) PR:#44184
Enums
-
ArrayMesh
:ArrayType
enum is a duplicate, delete it https://github.com/godotengine/godot/issues/16863#issuecomment-571283702 https://github.com/godotengine/godot/issues/15763#issuecomment-568908661 https://github.com/godotengine/godot/pull/36382 -
CPUParticles
:Flags
enum ->ParticleFlags
https://github.com/godotengine/godot/issues/16863#issuecomment-571283702 https://github.com/godotengine/godot/issues/15763#issuecomment-568908661 https://github.com/godotengine/godot/pull/36382 -
Mesh
:BlendShapeMode
enum is only used byArrayMesh
, so give toArrayMesh
https://github.com/godotengine/godot/issues/16863#issuecomment-571283702 https://github.com/godotengine/godot/issues/15763#issuecomment-568908661 https://github.com/godotengine/godot/pull/36382 -
Viewport
:ClearMode
andUpdateMode
enums should use the same suffixes (#19404) PR:#44267 -
XRPositionalTracker
:TRACKER_LEFT_HAND
->TRACKER_HAND_LEFT
etc https://github.com/godotengine/godot/issues/16863#issuecomment-494437342 PR:#44261 - Rename the
ButtonList
enum toMouseButton
https://github.com/godotengine/godot/issues/16863#issuecomment-612792875 PR:#38054
Constants
- Global Scope:
PROPERTY_USAGE_STORE_IF_NONZERO
andPROPERTY_USAGE_STORE_IF_NONONE
should be fully dropped, also from GDNative #37693
Theme items
-
ItemList
,LineEdit
,RichTextLabel
,TextEdit
andTree
:font_color_selected
should be renamed tofont_selected_color
to match other_color
properties. #30018 PR:#44194
Objects
-
CapsuleShape
should be vertical by default #36488
Shading language
-
WORLD_MATRIX
is actually a model-view matrix and should be renamed. @reduz suggests to replace it (andCAMERA_MATRIX
, which is a view matrix) by actual view and model matrices, e.g.CANVAS_MATRIX
andITEM_MATRIX
.
Project settings
-
display/window/size/test_width
andtest_height
should be renamed towindow_width
andwindow_height
. We should also consider renaming thewidth
andheight
settings, mayberender_width
andrender_height
. https://github.com/godotengine/godot/issues/16863#issuecomment-412308210 PR:#47522
File formats
- Discuss what we want to do with custom binary resource extensions (
RES_BASE_EXTENSION
) https://github.com/godotengine/godot/issues/16863#issuecomment-413620204
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Reactions: 75
- Comments: 517 (448 by maintainers)
Commits related to this issue
- Rename various TileMap methods/properties for clarity and consistency The YSort renames were tracked in https://github.com/godotengine/godot/issues/16863. This closes https://github.com/godotengine/... — committed to Calinou/godot by Calinou 4 years ago
- Rename various TileMap methods/properties for clarity and consistency The YSort renames were tracked in https://github.com/godotengine/godot/issues/16863. This closes https://github.com/godotengine/... — committed to kappa54m/godot by Calinou 4 years ago
- Rename ShortCut to Shortcut which is more grammatically correct See https://github.com/godotengine/godot/issues/16863#issuecomment-685236980. — committed to Calinou/godot by Calinou 4 years ago
- Rename ShortCut to Shortcut which is more grammatically correct See https://github.com/godotengine/godot/issues/16863#issuecomment-685236980. — committed to MarcusElg/godot by Calinou 4 years ago
- Rename various TileMap methods/properties for clarity and consistency The YSort renames were tracked in https://github.com/godotengine/godot/issues/16863. This closes https://github.com/godotengine/... — committed to edg1000/https-github.com-godotengine-godot by Calinou 4 years ago
Too tedious for me, but Godspeed to whoever fixes
instance
asinstantiate
where used as a verb.make_convex_from_brothers()
I guess “brothers” should be changed to “siblings”, as that’s the word used everywhere for sibling nodes.I would like if VBoxContainer/HBoxContainer/GridContainer is renamed to simple VBox/HBox/Grid…
AABB has
intersection
, while Rect2 hasclip
. They do the same thing. One name should be chosen, but which one?Poll: ❤️ = both should be
intersection
, 🚀 = both should beclip
, 👎 = don’t rename.For single numbers, we have
stepify
. For Vector2/Vector3, we havesnapped
.They do the same thing, the vectors call
stepify
for each component. One name should be chosen, but which one?Poll: ❤️ = both should be
stepify
, 🚀 = both should besnapped
, 👎 = don’t rename.Should
Label
be changed toTextLabel
? Several times now I’ve wanted to put down a text object and forgot what it was called, so I search for “text” and onlyRichTextLabel
shows up.TextLabel
would be more consistent withRichTextLabel
as it’s still text but without the rich.For reference, Unity has
Text
andTextMesh
, and I personally refer to it as text rather than label.Array
we should renameempty()
tois_empty()
to better illustrate it’s booleanI recommend Resource’s
local_to_scene
should belocal_to_instance
orunique_per_instance
. “Local to Scene” denotes the behavior as local to the scene, when it’s actually per instance of a scene.Loving this issue right now. May hate it later when the breakage actually hits 😉
Some proposals for the humble
Array
class:duplicate
→ eithercopy
orclone
. I don’t know of any language that calls this concept “duplicating”.copy
is what it’s called in Python and in C++ so it would be the natural choice for Godot.clone
is from Java and JavaScript and is maybe a bit more precise.invert
→reverse
. The documentation even describes it as “reverse” so there is really no excuse.remove
vs.erase
is confusing. Suggestion:remove_at
andremove_value
.The last two also apply to all
Pool*Array
classes.Curve.clean_dupes()
->clear_duplicate_points()
In both the API and the engine code, there are a lot of places where
idx
,position
,at
andindex
are used to refer to an index. I propose to useindex
everywhere when referring to an index (or at least in the API).While working on https://github.com/godotengine/godot/pull/48542, I noticed we could rename
String.is_abs_path()
toString.is_absolute_path()
. I think it’s better to use plain English rather than abbreviations, and NodePath already has a method calledis_absolute()
.bool
,float
,int
are the only types/classes whose names start with a lowercase letter. I think they should be renamed (in GDScript) toBool
,Float
,Int
. This will automatically fix the problem with incorrect type syntax highlighting.And
bool
,float
,int
should be used only for built-in Python-like functions from @GDScript page (they also includelen
andstr
).Note that the situation is similar with
str
/String
:str(x)
andString(x)
give the same result.ADD. It should have looked like this:
The project settings
display/window/size/test_width
andtest_height
should be renamed towindow_width
andwindow_height
. We should also consider renaming thewidth
andheight
settings, mayberender_width
andrender_height
.In various places in the engine there are properties or methods containing the word
priority
. In some cases, objects with a higherpriority
value are processed first, in others - with a lower one.I suggest making it consistent everywhere (this only applies to
priority
, notlayer
,weight
, etc).(This list may be incomplete or inaccurate.)
Rename
ShortCut
toShortcut
Edit: https://github.com/godotengine/godot/commit/3e0226515eec5f904701fce36c7856617d3ca6b8
String::begins_with
toString::starts_with
.Like in serious programming languages (Java, Python etc).
Area
should really beVolume
, since it’s 3D. AndArea2D
should then beArea
. An area is 2D by nature.GridMap
should maybe beCubeMap
. Not sure on this one, just that “grid” sounds like a 2D thing to me.CheckButton
control should beSwitchButton
or something. It’s confusing because there is alsoCheckbox
. Or maybe it should be removed altogether. It serves the same function as Checkbox anyway, as far as I can tell.Since we have e.g. HTML5 and UWP as targets, which aren’t exactly operating systems, I propose renaming OS to Platform (PlatformWindows, PlatformUnix,…). Makes sense with the OS/Display split too.
I’ll raise #9128 again:
translation
in 3D vsposition
in 2D is an odd dissimilarity. It was raised ages before 3.0 but was closed after 3.0 went out due to… 3.0 being out.I prefer
index
overidx
for a couple reasons:idx
is harder/less natural to say (“eye-dee-ex” vs “in-dex”), unlike abbreviations likeint
andfloat
(and even stuff like Vec, Rect and Quat) which are one syllableindex
is more universal. To someone not familiar with whatidx
stood for they might think it’s some kind of ID.This suggestion does make sense to be fair, although more for clarity than consistency because the word ‘back’ has several different meanings depending on the context. For example, if you said ‘move back’, it could mean move backwards but could also mean move/return to your previous location, which is a different thing (you could have side-stepped right for instance 😃 ). Backward however is more clearly the direction opposite to forward. And left, right, up and down are clear directions so I think they are fine also!
I think Vector3*
BACK
property could be renamed toBACKWARD
to look relevantly with already existedFORWARD
.TextEdit
:cursor
could becaret
to not be confused with mouse cursor, in these methodscursor_get_column()
cursor_get_line()
cursor_set_column()
cursor_set_line()
etcSome class renames worth considering for consistency with other 3D renames:
BakedLightmap
->BakedLightmap3D
BakedLightmapData
->BakedLightmapData3D
Decal
->Decal3D
GIProbe
->GIProbe3D
LightmapProbe
->LightmapProbe3D
Lightmapper
->Lightmapper3D
LightmapDenoiser
->LightmapDenoiser3D
LightmapRaycaster
->LightmapRaycaster3D
ReflectionProbe
->ReflectionProbe3D
Voxelizer
->Voxelizer3D
Skin
->Skin3D
RootMotionView
->RootMotionView3D
Editor only:
BakedLightmapEditorPlugin
->BakedLightmap3DEditorPlugin
GIProbeEditorPlugin
->GIProbe3DEditorPlugin
GPUParticlesCollisionSDFEditorPlugin
->GPUParticles3DCollisionSDFEditorPlugin
EditorSceneImporter
->EditorSceneImporter3D
EditorSceneImporterMesh
->EditorSceneImporterMesh3D
EditorScenePostImport
->EditorScenePostImport3D
ResourceImporterScene
->ResourceImporterScene3D
SceneImportSettings
->SceneImportSettings3D
For the lighting ones, 2D lighting is a thing, so it makes sense to have the 3D ones with the “3D” suffix.
For the scene import ones, it’s not like I’m against 2D scene importing or anything, but the current scene importers are hard-coded to work with 3D (they import from things like OBJ, Collada, GLTF, and FBX to 3D), so the “3D” suffix makes sense to me. If we want a 2D scene importer in the future (from what format…?), we would likely need a separate codebase anyway.
Should we remove Node2D/Node3D/Control’s
hide()
andshow()
methods in favor of thevisible
property?Pros:
hide()
/show()
andset_visible()
methods are used interchangeably right now.visible
property is more concise when you want to toggle something’s visibility.Cons:
hide()
andshow()
won’t work anymore. Sometimes, I wonder if we should list removed properties/methods somewhere so that the GDScript compiler can tell you to use the “correct” property/method.Probably, we need to resolve the conversion method naming inconsistency (
a_to_b
vsa2b
).List
I didn’t expect this to spawn a debate, but I’m personally against keeping
Vector2.tangent()
since it’s an incorrect usage of the usual math term. The product ofVector2.tangent()
is just wrong by definition. The name of this thread is next planned compatibility breakage so I don’t see any reason why we should bend over backwards to keep compatibility. I’m personally fine withVector2.perpendicular()
.Tangentially related to #30736, we should rename the two Godot physics engines to something like “GodotPhysics2D” and “GodotPhysics3D”, since saying that something’s broken with “GodotPhysics” can mean two very different things.
I’ve updated the OP with most of the suggestions given so far.
I’m not convinced, in Godot we try to give everything explicit names, and for example
Grid
doesn’t tell me that it’s a Container. ForVBox
andHBox
one could argue that boxes are containers by definition, but since we haveBoxContainer
which is not the same asContainer
, I think the verbosity is still warranted.I don’t think it would be useful to add
new_text
to TextEdit. On LineEdit, it simply contains the whole text of the LineEdit, not the text that changed, so I’d even argue that it should not be there in LineEdit’stext_changed
. Yet, it’s more common that you want to use the full text of a LineEdit on input, than to do things with the full text of a multiline TextEdit when a new character is pressed.We should rename Camera2D
zoom
to e.g.view_scale
. Or reverse the behavior, because that’s not how zoom works.I noticed some methods are virtual but their names don’t start with
_
(for example drag and drop-related methods in Control). I think we should rename them all to have_
at the beginning, for consistency and maybe less confusion.runner
doesn’t seem very clear to me.standalone
is ok. Removing it would be ok too, because you can just use!OS.get_feature("editor")
.How about removing
sort_custom
and makingsort
take an optional Callable?I don’t know if it was raised, but I didnt realise how often I stop to peek into the doc for this one:Array.remove
=>remove_at
(like C#) to remove by indexArray.erase
=>remove_value
, or justremove
(like C#) to remove by value(or variants of that with
erase
)Because right now,
erase
andremove
mean the same to me, except one is by index, the other by value, I keep forgetting which is which.Was raised already, my bad. Didn’t notice, Github is folding the thread, my Ctrl+F search missed it xD Not in the OP yet
@groud In that case, it should be called
is_ancestor_of()
. If there’s also a corresponding function for the other way around, then it should be calledis_descendant_of()
.@Calinou I agree with @Zylann double negatives are confusing and should be avoided whenever possible. We should instead change “Disabled” bools to “Enabled” ones. It’s fine if the default value of something is “true”. I had this discussion in #17212 and #21822.
@Calinou I dunno if that would require a separate issue, but if we need to be consistent, I’d really prefer such booleans to be
Enabled
always. Setting a boolean totrue
to disable something confused me often.String::validate_node_name()
->String::to_valid_node_name()
Because
validate_node_name()
actually doesn’t validate anything, it just mutates the string by removing a set of illegal characters and gives the string back. From the name, I’d have expected it returning a boolean.https://github.com/godotengine/godot/blob/92f20fd70e6957cd65ccb7837fdc28f9b1e4a315/core/string/ustring.cpp#L4489
As per https://github.com/godotengine/godot-proposals/issues/2939#issuecomment-871566586, we should probably rename the
GLOBAL_GET
andGLOBAL_DEF
C++ engine macros toPROJECT_GET
andPROJECT_DEF
. The Globals singleton was renamed to ProjectSettings in 3.0 after all.Edit: Pull request opened: https://github.com/godotengine/godot/pull/50350
SpriteLight for 2D, maybe? I am definitely in favor of renaming Omni to Point in 3D. This is how it is called in Blender, so to me the meaning of Point is clear and cannot be confused with Spot.
Also it seems we prefer direct/natural word order in functions names so
caret_get_column
could beget_caret_column
etcI think InputEventWithModifiers’
control
property should be renamed toctrl
. “control” sounds like a gui element, I got pretty confused when I saw!b->get_control()
while reading the source code. Probably alsocommand
->cmd
for consistency.@me2beats This has already been implemented in #44401.
TreeItem:
get_children()
->get_first_child()
https://github.com/godotengine/godot/issues/19796AudioStream
—AudioStreamPlayer{,2D,3D}
VideoStream
—VideoPlayer
We should probably rename
VideoPlayer
toVideoStreamPlayer
. Or we can renameAudioStreamPlayer{,2D,3D}
toAudioPlayer{,2D,3D}
.I don’t know if this has been mentioned elsewhere (is there a way to force GitHub to show all hidden items?) but I think
Control::set_focus_neighbour
and friends should be renamed to the AmE spelling to keep it consistent with the docs which use “neighbor,”Color
, etc. I would be happy to open a pull request for this.EDIT: Resolved by #44412
“Less verbose” is definitely not on the menu if we want to have robust codebases across our Godot projects. Modern coding tools provide autocompletion and other intelligent features that allow you to type quickly even if the name is long. Plus, if you read the argumentations for those changes, there is a goal to make those functions less ambiguous to developers using them. Short name might be sweet, but confusing and less discoverable.
And always remember: typing code is the quick part of the software development. Reading and understanding code afterwards is much more important. It’s like conceiving and raising a child respectively.
Should we rename
Control.MOUSE_FILTER_PASS
toControl.MOUSE_FILTER_PASSTHROUGH
? This would make it more obvious that the event will be passed through the Control node to nodes located below it. I’m not sure if it’s actually worth renaming it, though.These methods (
find
/findn
etc.) basically do the same. We could just add an argument whether they should be case sensitive or not.https://www.merriam-webster.com/dictionary/circle
(1 b) is a mathematical circle (perimeter), (1 c) is a mathematical disk (surface).
In terms of API, it’s IMO more user-friendly to have
draw_rect(bool filled)
anddraw_circle(bool filled)
thandraw_rect()
,draw_filled_rect()
,draw_circle()
anddraw_disk()
(or what would be the mathematical name for a filled rectangle?).Edit: Actually
draw_circle()
doesn’t have afilled
argument yet. I think we should add one, so that it can be used to draw both circles (perimeter) and disks (filled circle).Did we consider renaming these:
shader_type canvas
=>shader_type 2d
shader_type spatial
=>shader_type 3d
CanvasItemMaterial
=>Material2D
SpatialMaterial
=>Material3D
Please, consider changing the name of
TreeItem.get_children()
method, as the name implies a collection of children to be the return value, when in practice it is the first child that is returned and then you have to iterate over the rest of them usingget_next()
(as described in #19796).loop, repeat, oneshot, of animationplayer, timer, and similar nodes, could be clarified.
looping, repeating, and not being a oneshot, are all the same things.
@akien-mga please consider adding the Tree navigation methods to your list. They are very confusingly named and not well documented.
When I first encountered them I thought get_child() and get_next(), get_prev() operated an iterator much like they way Directory works. I had to do my own testing to find out that all they do is produce the same pointer each time they are called.
get_children() should be renamed to get_first_child().
get_next() and get_prev() should be renamed to get_next_sibling() and get_prev_sibling()
TreeItem.get_children()
should be renamedget_first_child()
. The doc should also explain that it’s NOT returning the child items. Children are iterated usingget_next()
.Animation.track_remove_key_at_position should be Animation.track_remove_key_at_time
I wonder if we should move these to dedicated Reference-based types (similar to JSONParseResult) to provide type safety and autocompletion. Compile-time errors are better than run-time errors 🙂
In general, I think we should avoid returning Dictionaries unless the returned keys’ names is meant to change depending on the input.
I think this adds too much complexity and would also require more typing than necessary. Unlike most operations in programming, printing is one of these things people do so often it’s important to save every possible keystroke.
Well, if we have to pick one that works for everything, it’s
to
, not2
. This also works better for other class methods that convert it to something, likeObject.to_string()
, which you wouldn’t call2string()
. But do we actually need to make it the same for everything?This would also affect some of the names, because
str2var
should probably be expanded to at leaststring_to_var
, if notstring_to_variant
.@LightningAA What if we changed
count()
so that it had a single optional parameter. If you give it a null argument, then that is perceived as “empty” (the default) and it returns a value similar to size(). But if you give it a non-null argument, then it returns the number of elements matching that argument. If the argument is a Callable that accepts a single parameter, then you could even treat it like a predicate.On
Control
:focus_neighbour_*
->focus_neighbor_*
I disagree. A
Mesh
has multiple surfaces. These are not getters and setters of theMesh
’ssurface_material
. They are methods to set a specifiedsurface
’sMaterial
. This is a standard naming convention in Godot for this type of method; so they should not be changed.I disagree with both of you. I am a Godot user and I use Godot specifically because GDScript is terse and quick to write. If you gonna make them twice as long then the speed advantage is lost as I will be forced two write twice as much and would have to scroll twice as far to see the entire line of code horizontally.
When you are coding you don’t wanna be typing incredibly long function or variable names. Some of these proposed changes changes add extremely long function and variable names for the sake “clarity” at the expense of everything else. You can read the built in help if you have any doubts. Plus programming is about learning an API. You will always look up a function first time you use it regardless of the name. Take printf in C is terse and simple. In Godot you would name it
send_formatted_string_to_standard_out.
Not only you’re forcing everyone to re-learn the api but some of these changes don’t even have a unified vision. Seems to me like a whole bunch of people got together and each changed one parts of the engine.Take Array for example
remove
->remove_at
What does _at add here? You already have remove_value. What else can you remove?erase
->remove_value
Still not clear enough. From documents “Removes the first occurrence of a value from the array.” Also people might think you can remove one single value from the entire array. For clarity it should actually beremove_first_occurance_of_value
because that’s what the function actually does. So you made the function longer and equally confusing just longer.You have
remove
anderase
two different functions but you turn them both in the same variation onremove
with extra letters. But they function work differently. Remove removes from a specific index where as erase removes the first instance of a value found.These are okay just not really useful other than forcing people to update their code. invert -> reverse
duplicate -> clone empty -> is_empty
If is not broken don’t fix it.
Editor vs standalone is probably standard naming (at least one I see in many different engines) so no reason to reinvent the wheel imho.
I think find_node and/or get_node should be renamed because the names don’t indicate they differences between the 2 at all. Since find_node only looks at children, maybe find_child_node? I’m not sure what a good alternate name for get_node would be. My first thought was get_tree_node since it can get a node from anywhere in the tree, but it can also be used outside of the scene tree with relative paths.
@Zylann This method is actually really weird. There are properties called
canvas_transform
andglobal_canvas_transform
, and the description ofcanvas_transform
is this:So, you would expect
get_final_transform
to get both of these transforms combined. However, if you look at the code, that’s not actually what it does.The code for this is
return stretch_transform * global_canvas_transform;
. I’ve been trying to figure out whatstretch_transform
is for and I have no idea, it’s not exposed, and it’s certainly not set when changingcanvas_transform
. There is also a method called_stretch_transform
which does not usestretch_transform
. It’s called byset_size
, which takes the value generated by_stretch_transform
and assigns it tostretch_transform
. There’s alsocanvas_transform_override
which I haven’t even mentioned and isn’t exposed, but is used internally.In the end I think this whole class should be looked over. Viewport has 4 different Transform2D members, 4 Rect2(i) members, 9 Vector2(i) members, and 2 Transform (3D) members. That’s already 264 bytes (with single-precision floats) of just data structures storing the transformation information, if you add up all the pointers and such it’s probably closer to 1 KB. Maybe Viewport does need all this, but it seems overcomplicated, and at the very least there should be comments (there are far too few in this file).
I’m surprised that no one mentioned it yet buuut
Camera2D.zoom
the camera is zoomed out when the zoom is greater, which is not how “zoom” works and it’s counter-intuitive. Not sure what rename it to, maybeview_scale
or something similar.What about renaming
Engine.iterations_per_second
toEngine.physics_fps
for consistency with the Project Settings?VisualServer
’sinstance_create2()
should be changed to actually describe what it does differently thaninstance_create()
.I’ve also been wondering about
Tree
andGraphNode
being renamedTreeView
andGraphEditNode
. ForTree
, the reason is it’s way too broad of a name for a global GUI node IMO, all other GUI frameworks I know of useTreeView
. ForGraphNode
, it’s because I worked on a few prototypes recently involving actual graphs structures, and I could neither useNode
norGraphNode
which was quite annoying.Node.NOTIFICATION_TRANSLATION_CHANGED
should probably becomeNOTIFICATION_LOCALE_CHANGED
, as “translation” is used in Spatial nodes to mean “position” and not “language”.set_cell_item can be renamed to set_cell to unify GridMap with TileMap ?
WindowDialog.get_close_button() ConfirmationDialog.get_cancel() -> ConfirmationDialog.get_cancel_button() AcceptDialog.get_ok() -> AcceptDialog.get_ok_button()
So, String.right() returns n right characters from given position. Wouldn’t it be more intuitive if it returned just n characters counting from the right?
"abcdef".right(2)
Instead of “cdef” would return “ef”. IMO it’d be better.How about renaming Spatial to Node3D? I always found it weird.
TileMap.set_y_sort_mode(val) -> TileMap.set_y_sort_enabled(val) TileMap.is_y_sort_mode_enabled() -> TileMap.is_y_sort_enabled()
We should probably rename RID_Owner’s
getornull()
toget_or_null()
(also for RID_Alloc and RID_PtrOwner).Edit: Done in https://github.com/godotengine/godot/pull/53227.
In Godot 3.x, we have the following two nodes:
In the current master, we have the following two nodes:
I propose we rename
OmniLight3D
toPointLight3D
to match 2D (❤️). Alternatively, we could haveOmniLight2D
(🚀).See #47548
ScrollContainer’s and RichTextLabel’s get_scroll_v() methods should be either renamed or unified to return the same thing (in 3.3.2, ScrollContainer’s returns the scrollbar value while the RTL’s returns the scrollbar itself)
Obligatory mention of godotengine/godot-proposals#2816 which could possibly meet both needs in some future 4.x release.
@pycbouh I added that just now though, because of @MikeSchulze’s comment 😉
I think the
endian_swap
property ofFile
could be renamed tobig_endian
.endian_swap
without referring to the doc.endian_swap = true
on a big-endian-native platform (if any).StreamPeer
also usesbig_endian
for a similar property.Array:
count()
to something likecount_occurrences()
. “count” is confusing because in C# it means the length of a list.Change “pickable” of the CollisionObject2D to “enable_mouse_interaction” See: https://github.com/godotengine/godot/issues/38085 https://github.com/godotengine/godot-proposals/issues/2441
Should we rename the TextureProgress node to TextureProgressBar?
Advantages
Disadvantages
Edit: Done here: https://github.com/godotengine/godot/pull/44377
I just stumbled upon the counterparts
Camera::unproject_position()
andCamera::project_position()
. They do the exact opposite of what you would expect / what is commonly agreed upon.Camera::unproject_position()
does a normal camera projection, i.e. it takes a 3D position and projects it onto the screen.Camera::project_position()
does what people would call an unprojection, i.e. 2D->3D. (Technically everything is a projection, but we’re not speaking correct mathematical terms here but graphics slang.)If you’re not convinced see https://www.google.com/search?q=camera+unproject or https://dondi.lmu.build/share/cg/unproject-explained.pdf to convince yourself that practically every other library uses the terms project and unproject in the opposite directions to what they are currently implemented in Godot.
Edit: I think I now understand how this happened historically. There are also:
Camera::project_local_ray_normal()
Camera::project_ray_normal()
Camera::project_ray_origin()
which all take screen positions as inputs. I get that you can say “to project a ray onto an object in world space”, but also look at the documenation of
Camera::project_ray_normal()
andCamera::project_ray_origin()
(they are identical):… you mean by the inverse camera projection? One could suggest that it would be less confusing to just call them
cast_
instead ofproject_
. But since projecting rays is also not an uncommon term, a documentation update for the ray functions will probably suffice.Edit²: I fixed a typo and deleted most of my last edit. Now its restored.
The name should be descriptive though. Also length doesn’t matter if you take advantage of the autocompletion (which is built-in in the Godot editor).
I mentioned it once on IRC and didn’t get a reply, but TextureRect has a stretch mode called “Scale On Expand (Compat)”. This sounds like something that could be removed.
The
Tracking_status
enum inARVRInterface
should be renamed toTrackingStatus
, to be consistent with the styles of other enum names.EditorImportPlugin
andEditorExportPlugin
look like they are related, however one is about importing resources and the other is about exporting a project. I suggest we renameEditorImportPlugin
toEditorResourceImportPlugin
or something along those lines.Edit: And
EditorPlugin.add_import_plugin
must be renamed accordingly as well.@MuffinManKen I think that
get_node
is perfectly understandable since, as you stated, it can fetch any node, anywhere, so long as it is connected with the same tree as the given node, regardless of whether they are part of the SceneTree or not.@Coldragon I’m not sure I like renaming
find_node
tofind_child
either, as it then gives me the impression that it works only with direct children for some reason. The alternative would be to have it be calledfind_descendant
or something, but that is far too wordy/complex. Cutting it down to justfind()
also doesn’t make sense since it is then unclear what we are searching for. As such, I think that unless another alternative is sought,find_node
should stay as it is too. It should just have clear documented differences in behavior for the API docs.I have one that potentially fixes some bugs in
RichTextLabel
. Currently we havebbcode_text
andtext
, but both use internally the same data structure. Only the called methods are different, whileset_bbcode
falls back toset_text
whenuse_bbcode
is not enabled. So I went ahead and removed them in #39148. I added some other points there.@dalexeev I recently did a first pass at updating the first post, but I didn’t quite go through all comments yet.
I noticed some methods/properties use
normal_map
, whereas others usenormalmap
. We should settle on either of these, but probably not both. I’d prefernormal_map
, but I’m OK withnormalmap
too.Following methods
should be moved from the
OS
class to theInput
class for consistency with the methods:Yes, but thinking about it there is not much of a need to have both functions, as the difference would only be a matter of switching the “calling” object and the function argument. Maybe we could simply replace the function by something like
is_descendant_of()
and we’re done .@Xrayez,
I know, just referring to the fact that
empty
is a verb as well as an adjective, and addingis_
makes clear which one is meant.I’d be in favor of using
is_empty
across all built-ins for that bool.Please leave this issue for core contributors, it’s not a place to make suggestions of big renamings all over the place, the aim is to fix actual inconsistencies that make the API cumbersome.
The changes outlined in the OP are not a big compatibility breakage and most will be done for 4.0, what @reduz referred to is compatibility breakage of the “your project can’t be opened anymore” kind as between 2.1 and 3.0 (a lot more changed than just things getting renamed back then, such as the audio and particle systems being completely rewritten, the import system changing, etc.).
There will be some compatibility breakage in 4.0, otherwise it wouldn’t be called 4.0. It will be reasonable and porting will be easy, likely with a converter to ensure that renamed properties, methods and signals are properly converted if used in scenes and scripts. It’s not the place to discuss it in any case 😃
UX-wise, CheckBox and CheckButton are different things, despite having the same functionality. https://uxmovement.com/buttons/when-to-use-a-switch-or-checkbox/
CanvasItem
method:Should be renamed to
get_rid()
then.get_canvas_item()
is confusing because I’m already atCanvasItem
node. It also ensures consistency as other classes have similarget_rid()
method already:CollisionObject
,Resource
.set_as_toplevel()
could be renamed toset_as_top_level()
, but its behavior should be looked at.Remove
Resource::notify_change_to_owners
,Resource::{un}register_owner
. They are used only by GridMap and CollisionShape, the rest of the code uses the"changed"
signal.If I can have one added to the list, the LineEdit and TextEdit control nodes would really benefit from having consistent APIs with each other so they can be used (mostly) interchangeably. Right now it feels like a mess trying to work with them together, to the point where looking at one node gets me confused about the other.
Camera2D currently has two different properties both named
offset
(Regular offset and the one split up in V and H) that are two totally different things, this is really confusing.Another one might be renaming “margin” to “offset” for control nodes. Since margins are negative on the right side, this misleads people, especially when comparing with StyleBoxes
The
TileMap
class has a bunch of getter and setter methods that don’t agree with their respective properties. In fact I’d suggest renaming most getters and setters in that class, so they agree with their properties as well as the naming in other classes.Likewise, I prefer it when reading the reference or using the autocompletion of a scripting API keeps all related things together (which you get when they all have a
texture_
prefix).https://docs.godotengine.org/en/stable/classes/class_string.html#class-string-method-get-file It’s confusing because there’s already a File class but this method just returns the name of the file.
Just came across the Status enum of
StreamPeerTCP
. TheSTATUS_NONE
status is not very intuitive, it’s the initial status and also the status after disconnecting.I think
STATUS_DISCONNECTED
would be better. This is also howHTTPClient
names its corresponding status.Both #47548 and #47448 have a full list of what has been changed, these are listed there.
These are “out of order” on purpose, because they are not actually out of order. These methods are grouped around a virtual sub-entity, like caret or camera, which you cannot fetch individually and independently from the parent object, but methods are still unrelated to the parent itself.
Basically, if you could get a caret as a reference, it would be
get_caret().get_column()
. But as it stands in the scripting API you cannot extract a reference to the caret, so this is reflected in the name of the function. This also nicely groups the related methods in the class reference.In other words, these methods are named consistently with our practices, not against them.
Could
String.trim_suffix()
andString.trim_prefix()
be renamed toString.remove_suffix()
andString.remove_prefix()
?lstrip
,rstrip
, andstrip
. And Python usesremoveprefix
andremovesuffix
.rpc
should be renamed torpc_reliable
andrpc_unreliable
torpc
. Basically, RPCs should be unreliable by default. That is how Unreal Engine does it, and how you should do it in most circumstances.Image
is not always used as a source for textures to be further rendered. NeitherImageData
could fully describe the purpose of the class, since it contains data and a plethora of algorithms to process the data.In general, I haven’t used every single class in Godot, and I will be confused as well when trying to understand something which I may not necessarily need at the moment. I don’t get how a simple class rename could significantly improve the issue with misunderstanding/misconception. Confusion is a part of the learning process. 😕
I mean, it’s basically a dilemma between making this more clear either for beginner use cases, or advanced use cases. Seeing
ImageData
could be equally confusing to an experienced developer when they also want to find image processing methods to manipulate the data, or limiting the use cases to rendering textures withTextureData
name.I use
Image
class almost like on a daily basis so that’s why I’m so vocal about this, sorry! 😃I’m in favor of renaming
MeshInstance2D/3D
→Mesh2D/3D
in 4.0.By the way, the sprite conversion tool in 3.x also refers to it as
Mesh2D
:This can also be considered a bug in 3.x, in fact.
Sprite
is also renamed toSprite2D
in 4.0, so this should make it consistent.I don’t really like renaming
Mesh
toMeshData
because it would lead to renaming inherited classes as well. It’s similar to renamingImage
toImageData
just to avoid confusion with aTexture
. In cases like this, it’s certainly just documentation issue, but the above discussion does makes sense on some level.AnimationPlayer and Animation: replace all occurrences “remove” and change them to “delete”. Including the in-editor drop down “Remove” to delete an animation. -> “remove” is associated with something staying in memory ( like
Node.remove_child(child)
,Node.remove_from_group(group)
,Node.remove_and_skip()
) -> “delete” is what these methods are actually doing. If you currently “remove” a key, track, or animation, they are gone for good. -> “delete” is less ambiguous@Geequlim @Calinou How about
web
? Otherwise I would preferwebassembly
.Should we remove the
.append()
alias for.push_back()
in the various Array classes? See https://github.com/godotengine/godot/pull/44274/files/2f3386794b3fabefb9840be3f78c2d7195e16214#r541057877.Advantages
.push_front()
counterpart which doesn’t have an alias.Disadvantages
.append()
is used to append to arrays.Edit: Done here: https://github.com/godotengine/godot/pull/44382
@Feniks-Gaming I can tell you empty or is_empty are equally confusing just one is longer than the other.
Why not
ResourceImportPlugin
? Many (most?) editor classes don’t contain the word “Editor” already, likeSceneTreeDock
or all of the animation stuff.TreeItem’s get_children() only returns the first child and not all children like the name (or the description in the docs) suggest.
[Edit] Nvm the docs. The method description is updated in master, sorry.
Yeah, but consistency would be good, too. Get rid of boolean parameters in InputEvent then?
@dalexeev Boolean parameters are often less readable than having different methods, since
true
andfalse
have no context.We should consider changing
String
begins_with()
tostarts_with()
.It is that way in Java, C#, Python, JavaScript, etc.
Bugsquad edit: https://github.com/godotengine/godot/issues/16863#issuecomment-596069130
What about
RayCast.target
?IMO:
get_image
yes,get_bytes
no.texture.get_image().get_data()
I can’t find it here so I would propose,
find
andfindn
pair.image from 3.2 last stable build.
Maybe we can rename one to mention their nature of dealing with case sensitivity.
Say,
find_ignorecase
instead offindn
?#27509
AnimationPlayer
’sanimation_started
andanimation_finished
are a bit counter-intuitive. The first one is called for all animations in the queue while the latter is only called on reaching the very end of the queue. It isn’t clearly mentioned in the documentation.If we want to detect when any particular animation ends, we have to use both
animation_changed
andanimation_finished
which isn’t convenient.I really like @txigreman’s suggestion from that issue: We can use
animation_finished
whenever any single animation in the queue ends and add a new signalanimation_all_finished
(similar toTween
’stween_all_completed
) which fires only when we reach the queue’s end.Consider removing
new_text
argument fromLineEdit.text_changed
since it has the same behaviour asLineEdit.text
.Also consider adding
old_text
either in addition to or as a replacement ofnew_text
.Related to https://github.com/godotengine/godot/issues/16863#issuecomment-394745886
Vector2.tangent()
is described in the docs as:Returns a perpendicular vector.
, that’s the definition oforthogonal
ororthonormal
if the returned vector is normalized. SinceVector2.tangent()
returns a non-normalized vector I propose we should rename it toVector2.orthogonal()
or evenVector2.perpendicular()
if people have something against math nomenclature or perhaps evenVector2.normal()
, but people might get confused betweennormalized()
andnormal()
@Anutrix “filled” is a better word to use than “solid”, because “solid” can mean “not liquid or gas”.
@pycbouh It would also be a good idea for linking PRs for each issue if there is one. The OP already does this, but not for the comments below it.
Since it’s an optional argument (it has a default value), and also for consistency with
draw_rect
, it should go at the end.SoftBody
has anareaAngular_stiffness
which should bearea_angular_stiffness
(same for all related methods).As suggested in https://github.com/godotengine/godot-proposals/issues/252#issuecomment-557901552, it might make sense to rename everything related to
clear_color
tobackground_color
(including the project settings).No issue for this yet but after suggesting it to Akien today worth putting on the list.
Refactor the ARVRServer so it is called the XRServer. When we designed it the term XR to indicate both VR and AR wasn’t in wide use yet. And no, I’m not going for MRServer 😉
parse_input_event( InputEvent event ) Feeds an InputEvent to the game. Can be used to artificially trigger input events from code.
parse is misleading, parse would be receiving and processing but the description indicates sending or triggering an input
Rect2
:has_no_area
should be renamedhas_area
that would prevent double negation logic checking the opposite in conditionals. Same applies toAABB
:has_no_surface
.Change
CanvasItem.visibility_changed()
signal toCanvasItem.visibility_changed(visibility: bool)
, ie. send the visibility status with it. Since this will be enough then removeCanvasItem.hide()
signalConsider renaming
Array.invert
toArray.reverse
. Invert is more like upside down or the “reciprocal of” type of thing. See https://docs.godotengine.org/en/latest/classes/class_color.html#class-color-method-invertedRename
Tween.tween_completed
toTween.tween_finished
? Just likeAnimation.animation_finished
? Generally prefer_finished
over_completed
? It feels likestarted/finished
have a tighter relation thanstarted/completed
- biased from competition sports:start/finish
- maybe 😄Rect2
:clip
→intersection
AABB
hasintersection
method but notclip
, clipping generally means that we cut something out, which is not implemented in either class btw. Documentation describes it as:Might as well rename:
intersects
→overlaps
in order not to be confused with the actualintersection
operation:The
Position3D
andPosition2D
nodes are a bit ambiguous. Without reading the description, one might assume that they are like Spatial and Node2D but without rotation or scale. They should probably be renamed toPositionHint
andPositionHint2D
or perhaps some other word likeGizmo
since it’s a gizmo only in the editor orAxisMarker
because it looks like a little axis marker.If they were renamed to
Gizmo
then perhaps they could be given more general uses.I’ll add a 🎉 reaction to all suggestions which have been integrated into the OP, to have a better overview.
There are a lot of tween methods which return true every time, they should probably be made void.
@KoBeWi I agree. I don’t see the difference between the current implementation and substring.
Camera’s near and far properties have different names than its setters/getters ( eg set_znear/set_zfar). This should be changed ?
I noticed that Godot currenly has two different naming conventions in its method names.
Sometimes, it follows a common convention that can be found in APIs of such languages like C# or Java, like
[action][object]()
form: i.e.)Mesh.GetBlendShapeName()
AnimationPlayer.GetCurrentAnimation()
Button.GetButtonIcon()
However, in other places, it follows a different convention of
[prefix][action][object]()
, like:Mesh.SurfaceGetFormat()
AnimationTreePlayer.NodeGetInputCount()
CollisionObject.ShapeOwnerGetOwner()
They are just a few examples out of many.
If we can afford to do sweeping compatibility breaking changes sometime in future, I’d like to see they can be renamed to follow a single, well defined naming convention (hopefully the former, as it’s more often used both inside and outside Godot).
When working on converter from Godot 3.x to Godot 4 I found, that renaming this functions(first string) to another one(second string), works for some classes, but broke others. I think that this should be unified to help users to upgrade its projects
TextureButton
:This is a debatable issue. In one case, the advantage is that the name corresponds more to the English language, and in the other case, the advantage is that similar properties have one prefix and are located next to each other.
It’s necessary to bring all such cases to consistency.
I would also like the same name for index position everywhere, but would opt for
idx
in this case because it is used so much and so often and “sounds” exactly the same as “index”. Even though I’m usually for more explicit names, there is hardly any chance to misinterpretidx
as something other than “index”.The term “point light” is sometimes used to refer to both omni and spot lights. In contrast to DirectionalLight, the position of a point light matters. If we rename OmniLight3D to PointLight3D, we lose on this distinction.
PointLight2D has no concept of omni and spot lights because its appearance depends on the texture you use. While you could use a projector texture on an OmniLight3D to make it look like a spot light, doing so is inefficient, especially when shadows are enabled (which is a requirement for using projectors in Godot).
Due to this, I think neither of the suggested renames are appropriate. If improving search quality in the Create New Node dialog is a concern, we should add a keyword system with alternate names for nodes instead (e.g. using the class reference XML).
BTW there are a lot of
VisualServer
methods with such word order likecamera_create()
camera_set_cull_mask()
camera_set_environment()
camera_set_frustum()
IMO the decision should be made as to whether all engine functions should have “natural word order” (like in cases above) so that there is no need to come back to this question (maybe it’s worth a separate proposal?)
_Geometry2D::point_is_inside_triangle-> is_point_inside_triangle DirAccess::current_is_hidden -> is_current_hidden
There are probably many more.
Is it worth opening a separate proposal for a more OOP approach to print, having some Printer that allows for a reusable configuration of targeted output stream (out/err, editor, etc.), separator (tab/space), use of indents for pretty-printing, other
dump()
-like print settings for Objects/Dictionaries, etc.? That way you don’t have to have such a variety of global printing functions?I’m not sure I can agree with
printt()
andprints()
being removed. They are useful to me because then I don’t need to type in my own spaces/tabs to separate the many arguments I provide to them.I will say that I’m not opposed to them being renamed, but I don’t think the suggested names are that great. The need to type out two words, rather than one word and a optional letter that adds common separators, just doesn’t feel right to me. I’m not sure I can really explain why though.
(P.S. I don’t think merging them into the
print()
function could happen either, as unless I’m missing something about GDScript, I don’t think you can have function arguments after a varargs (and requiring them before the varargs would just be annoying).)Removing these would make sense only if there will be an alternative for printing with a custom separator. If renaming these at all I’d rather go with something like:
so the name would actually tell what the function does. But I’d rather leave them as is.
from https://github.com/godotengine/godot/issues/37019
User suggested to rename
CollisionObject._input_event
argumentsclick_position
tomouse_position
andclick_normal
tomouse_normal
:You are right, I have not considered
Image
containing and doing more than just being a container for raw data. That being said, I still find the naming conventions around textures and images extremely counter productive in terms of aiding the learning process. I would think what we are trying here is to find a balance that works for most people at least somewhat good. Not something that either only works for beginners or only works for pros.@akien-mga What you are saying makes a total sense, but what @nathanfranke is saying is not pointless either. All nodes in the scene tree are instances, but not all of them have the
...Instance
suffix. It seems like a pointless thing to add and has confused me in the beginning as well.MeshInstance
could just have been namedMesh
and the mesh resource could instead have been namedMeshData
orMeshResource
or whatever. This would have aligned more with my expectations before I got used toMeshInstance
out of shear frequency of using it. It would also seem to align more with the rest of the engine that does not explicitly call it’s Instances...Instance
. LikeSprite2D
orSprite3D
.same for
Tabs.remove_tab()
,ItemList/OptionButton/PopupMenu remove_item()
and many of other occurrences I guess?Either:
CapsuleMesh.mid_height
->height
or:CapsuleShape2D.height
->mid_height
CapsuleShape3D.height
->mid_height
Dictionary.empty()
->is_empty()
Aliases are actually common in some languages, e.g. Ruby. Also
append
is consistent withappend_array
.This is the implementation btw: https://github.com/godotengine/godot/blob/06314c1b0e8100546a53cf2786fa244c5d19af6f/core/variant/array.h#L70 It indeed calls
push_back
but it’s done inline, so I think it has zero overhead.I would appreciate the consistency. Ultimately I don’t care what it will be as long as there are no aliases. The world is a mess, don’t try to fix it with aliases; it just adds onto it. Just chose one like all other programming languages. When it comes to aliases I am fully autistic:
I know that all of this is nonsense but once I realise this I would have already spent 30 seconds of brain power on something else than my project. While googling
"godot array append"
will result inpush_back
(if that’s the one) in 10 seconds without any distracting thoughts.Rename
ButtonBase
Property fromtoggle_mode
to more clearcan_be_toggled
. Issue withtoggle_mode
is that it’s not clear enough what it does. When you seetoggle_mode = true
you need to stop and think what it means. When you seecan_be_toggled
you know for certain that this button can now be toggled when you click it.Object::is_class(name)
→Object::inherits_class(name)
.I understand now that this method is mostly equivalent to GDScript
is
(which wasextends
btw), but C++ requires more explicitness.The confusion I ran into is checking whether particular object is of existing type (without inheritance):
The underlying implementation uses “inherits” macros/templates all over the place, so it makes sense to me to rename this to
inherits_class
.See also https://github.com/godotengine/godot/issues/21461#issuecomment-416155187:
That is beyond renaming and should be discussed as a proposal.
@Calinou I find the current behaviour unusual. This scene setup yields “Click Child2, Click Scene”
(Note, all are set to Pass)
😄 Proposition A: Perhaps something like
Control.MOUSE_FILTER_PASSPARENTS
for the current behaviour, since input events only seem to be sent to the parents.🚀 Proposition B: Change the constants to these:
👀 Proposition C: Whether or not the node takes gui inputs isn’t really relevant, since one can just not connect any signals (or use a boolean flag). We can change the option to
Control.propagation_mode
and have these constants:That would look a lot cleaner in my opinion.
Discussed as part of godotengine/godot-proposals#1590.
I’d prefer these basic random methods to be at global scope for accessibility and prototyping purposes, at least some of them. But
seed()
andrand_seed()
can be surely removed.Rename
Image.load()
→Image.load_from_file()
.load("image.png")
files via code, see documentation changes at #42412.Image.load()
won’t be highlighted as a GDScript built-in name anymore: #28866.Image.load_*_from_buffer()
per image format. Buffer-related methods can be potentially unified into the same interface to prevent API bloat, but that’s another topic for discussion.@dalexeev What you propose is not correct. When we talk about echo events, we talk about repeated keyboards events while pressing a key, using the term here makes little sense, since the action system does not relies of event directly, it’s state being updated in a per-frame fashion. Also, actions can also work with other devices like controllers or mouse buttons, where “echo” event do not exist.
As mentioned above, raw bools should be avoided. A flag like INPUT_ALLOW_ECHO/INPUT_NO_ECHO would be much better than a bool.
@dalexeev There are many cases where you need to check if a key is pressed and not only if it was just pressed. For example, a script for moving a character needs to do this with WASD or the arrow keys. Pretty much every game is going to need to process input, so I don’t think it’s wasteful to just have two methods for these things.
Not if you are viewing diffs on GitHub. If the code requires an IDE to be readable, it is not good code.
The Mutable type column is wrong: only Object-derived, Dictionary, and Array are mutable. (
Vector2
might look mutable since you can dovec2.x = 0
, but it actually translates to something likevec2 = vec2.with_x(0)
)Here are the terms as I understand them:
Regardless, these names aren’t going to be changed. They are fine as-is, and familiar to programmers of various languages. We should end the discussion here to avoid filling this tracker with pointless discussion.
@razcore-art I recently answered a question related to ray casting, and I used
segment
word to describe it, so I think makes sense. But this could also be rewritten asdirection
andlength
, so it will actually become something closer to a ray rather than a segment, speaking geometrically (or just provide alternative properties which could co-exist). The problem is that there’s no way to set a normalized direction vector easily in the inspector. I wrote a proposal to make one, at least for 2D: godotengine/godot-proposals#397, but perhaps that’s too far fetched.EDIT: Upon further thinking,
segment
wouldn’t make sense much in terms ofRayCast
node, but makes sense in terms ofPhysics2DDirectSpaceState.intersect_ray()
.@nathanfranke I’d assume target to be a
Node
or aNodePath
. So at least this could beRayCast.target_position
. MaybeRayCast.cast_position
or. Don’t forget that ray casts can rotate, which can shift the actual cast position in global coordinates.RayCast.cast_offset
Mesh / MeshInstance
surface_get_material/surface_set_material
get_surface_material/set_surface_material
It should be the same naming I guess ?
@Flavelius I haven’t seen the term “alpha clip” used often. I’ve seen “alpha test” used much more often than both “alpha clip” and “alpha scissor” for sure, though.
@Zylann Should be
get_editor_main_screen()
sinceit’s not a Container, butit’s the main screen.EditorInterface.get_editor_viewport()
=>get_editor_main_container()
This function does not return a
Viewport
, only aControl
which happens to be the central one holding the 2D, 3D, Script editors and the Asset Library. For the record, the returned node is even aVBoxContainer*
, but is abstracted away (yet it’s important to know, because it affects your choices of settings). The doc is also wrong, as its description links to theViewport
class. https://docs.godotengine.org/en/stable/classes/class_editorinterface.html#class-editorinterface-method-get-editor-viewportViewport.get_final_transform()
=>Viewport.get_final_transform_2d()
?I think
autotile_coord
(properties and methods) ofTileMap
should be renamed fortile_coord
ortile_coordinate
because bothAUTO_TILE
andATLAS_TILE
use this and the name may be confusing for new users. Docs will also need an update. I can make this change if there is no problem.I think
Tree::ensure_cursor_is_visible
is misleading and should be renamed to something likeensure_focused_item_visible
orensure_selected_item_visible
.Even the class reference says:
GDScript
range_lerp() = map() seed = set_seed()
Just a question, why not
MouseButton
? if button == MouseButton.LEFT: reads nicer than if button == MouseButtonList.LEFT:Will the OP be updated with suggestions posted after the June of 2019? I understand that this is a lot of work, but this type of tracker is also perfect for contributors to tackle together. And I assume it’s already the time when we can get to work on renaming more stuff?
Updating the OP would also mean that the suggestion posted is accepted as worthwhile by the team.
@Goutte In English, “circle” can refer to either a hollow circle or a filled circle. I think the current name is more discoverable, so I wouldn’t change it.
We should probably swap
Node.add_child_below_node()
's first two arguments so the first argument is the same asNode.add_child()
. See #19642.Quoting @Zylann from #31404:
Edit (2020-01-03): On second thought,
call_remote()
andset_remote()
might make more sense as we already havecall_deferred()
andset_deferred()
.Edit: Don’t mind the closing/reopening below, I clicked too fast.
Not sure if this is the right place or not but since this could be a possible partial solution to renaming issues I will pitch it here. Why not make all the renaming changes that are required and then add a new language to translations called GodotOldNames/GodotPre4.0 which has all old names for all the changes. This way in case someone doesn’t find the old names present in old tutorials, they can just change language to GodotPre4.0 . This would also make switching to new names easier. This doesn’t solve the whole problem but might work along with #4397.
@Calinou This method does not actually print anything to the screen; it returns a string. The first thought is that
JSON.print
works just likeNode.print_tree
orOS.print_all_resources
or like all otherprint*
methods.What should it be renamed to? I’m not sure. JavaScript uses
JSON.stringify
for this. PHP has ajson_encode
function. Directly in GDScript there is a similar function -to_json
.UPD.
JSON.serialize
is also possible, but by the number of characters it is the same asJSON.stringify
. 😄@leonkrause Instead of
render_width
andrender_height
it would make more sense to call itviewport_width
andviewport_height
, or perhapscanvas_width
andcanvas_height
, since it’s the reference resolution used for the canvas viewport, and doesn’t actually affect rendering.@aaronfranke This issue is about renaming things in the API, not engine functionality or bugs.
InputMap
:get_action_list( String action )
the function name doesn’t tell much about what it does. Since it returns the EventInputs associated to a given action, it could beget_action_events(String action)
Also could help avoiding possible confusion as
InputMap
has another function calledget_actions( )
and both names could mean the same thing out of context.Should RayCast have a “Disabled” property instead of “Enabled”? CollisionShape has a “Disabled” which is
false
by default (which means they’re enabled by default). In contrast, RayCast’s “Enabled” property isfalse
by default, making them disabled by default.Or, to use the positive form (which I think is less confusing overall), should CollisionShape have an “Enabled” property (
true
by default) instead of “Disabled”?@Rodeo-McCabe I get the impression the node naming intention is to express things in human language and not mathematical. So the “area” isn’t meant to be a geometric one, but a place to be, like inside of a level or stage. IE - Area 1.
The node itself doesn’t directly have any geometry or is itself a geometry, so others like myself would wonder where is its area/volume when creating it, and then why would I have to attach areas and volumes (Shapes) to an area and volume?
If it were to undergo renaming to avoid confusing it with geometric area, synonyms would probably be the way to go.
They might be called:
While I understand the reasoning, I don’t know if there would be any particular benefit to changing the name, especially to new people. I do agree that “Volume” might be a more appropriate term for this, but I feel like having “Volume” for 3D and “Area” for 2D may be a little confusing. After all, a lot of nodes who have both 2D and 3D variants will be named “<node>2D” and “<node>3D”, or just “<node>” in some cases where the term being used would describe its 2D/3D nature on its own (i.e. “Sprite” instead of “Sprite2D”, since sprite is already a 2D term, so we don’t need the 2D suffix).
I think that the naming scheme being there in the first place (as much as there are some nodes who only mostly stick to the scheme) means that we shouldn’t have an exception to the rule, even where another term for one variant would make more sense, as otherwise it’s likely to confuse users.
If I may, however… perhaps rename
Area2D
toArea
, andArea
toArea3D
?Edit: Seems text surrounded by <> doesn’t appear unless you escape the <>
Vector2.angle_to_point()
should be renamed. It’s name is not consistent with current description. Not sure what a good name would be, and whether this function is worth keeping.Original issue: #16307
Shortening to vector.lerp() sounds good. At least as of https://github.com/godotengine/godot/pull/16106 usage of
lerp()
in script has a switch to support vectors and colors.@bojidar-bg maybe
replace_self
orswap_by
? but I think that the only way to avoid any kind of confusion is by documenting it properly.Would OS.has_feature() / Platform.has_feature() be more sensible in something like ProjectSettings, since they don’t exclusively convey anything about the OS?
Rename
CanvasItem.visible
toCanvasItem.is_visible
(and all other places where it’s used)? to be in line with all (or most, maybe I missed some)bool
properties?The Tree node has a
get_selected()
function with seems to return the focused TreeItem. It might be worth to rename it to something likeget_active()
, since focus and selection are different things.@aaronfranke Um, but default arguments do exist. 0 or -1 could count as no length specified.
So maybe change everything 3D to “Thing3D”? That crossed my mind too, but sounds like overkill. Anyways, I just suggested. Not like it’s very important. Also, Spatial2D is even worse than Spatial.
@aaronfranke Fair point. In that case, the list of required swaps is:
RigidBody.apply_impulse(position, impulse)
toapply_impulse(impulse, position)
RigidBody2D.add_force(position, force)
toadd_force(force, position)
RigidBody2D.apply_impulse(position, impulse)
toapply_impulse(impulse, position)
PhysicsDirectBodyState.apply_impulse(position, impulse)
toapply_impulse(impulse, position)
Physics2DDirectBodyState.add_force(position, force)
toadd_force(force, position)
Physics2DDirectBodyState.apply_impulse(position, impulse)
toapply_impulse(impulse, position)
PhysicsServer.body_apply_impulse(position, impulse)
tobody_apply_impulse(impulse, position)
Physics2DServer.body_add_force(position, force)
tobody_add_force(force, position)
Physics2DServer.body_apply_impulse(position, impulse)
tobody_apply_impulse(impulse, position)
@TGRCdev I would vastly prefer changing
apply_impulse
to (force, position) rather than changingadd_force
to (position, force). Position of the force is an optional parameter so it should go at the end. But all forces and impulses must have a force vector.The size of boxes/cubes is named inconsistently. BoxShape for collisions uses extents. CubeMesh has a size property with x, y and z, which are each half the extents. CSGBox has a Width, Height and Depth property which are defined like the x, y and z size in CubeMesh.
In addition to the size property, sometimes “Cube” is used and sometimes “Box” is used. It would make sense to use “Box” for everything since x, y and z for CubeMesh can be set independently and it is thus also a box.
Methods
OS
:execute
should be split in two different methods, one blocking and the other non-blocking. e.g. names:execute
/exec_process
(blocking) andspawn_process
(non-blocking) #19302@dalexeev That’s a good topic for a dedicated proposal on godot-proposals, it will be easier to follow than on an issue with 515 comments 😃
I’ll close this issue as superseded by #54161 as this one is too big to still be useful. Most proposed changes have been done, others might not be consensual yet or should be moved to #54161 or dedicated proposals if they are far reaching.
Yeah, makes sense. I don’t think PointLight3D would be confusing with SpotLight3D personally, but I see why OmniLight2D would not be an appropriate term.
Also, if PointLight2D is defined with a texture, is it even a point light? A point light comes from a single point, if PointLight2D has light coming from a texture then isn’t that light coming from the area of the texture?
That’s why I was saying GDScript would have to support named arguments first
It’s about terminal output redirection. The standard output and standard error streams can be redirected independently from each other (e.g. to a file).
For instance,
curl
will print the response’s contents to stdout and print progress reporting to stderr. This way, when you docurl https://example.com > out.html
,out.html
only contains the response, not the progress bar that was printed to the terminal. In contrast, if you usecurl https://example.com &> out.html
(in Bash),out.html
will contain both the progress bar and the response (and your terminal won’t print anything to screen).This is just an example among many others, but I want to make sure we don’t break user expectations when it comes to this kind of stuff. stdout/stderr handling is mostly a solved problem by now, so let’s not break it 🙂
@LightningAA
printerr()
andpush_error()
are not equivalent to each other.printerr()
does not cause a message to appear in the editor’s Errors tab, and it doesn’t use the same formatting as engine-provided error messages in the terminal output.printerr()
should just print a message to the standard error stream and do nothing special about it. It should behave exactly like standardprint()
, except that it prints to the standard error stream instead of the standard output stream. This means it will look the same to the end user, but standard streams aren’t distinguished from each other in a default configuration.As for renaming
push_error()
/push_warning()
toprint_error()
/print_warning()
, the issue is that we risk increasing the confusion betweenprinterr()
andpush_error()
.I don’t use
prints()
often, butprintt()
can be useful when performing print-statement debugging and printing many values on the same line. That said, I think we need some kind of PHP-styledump()
function eventually… 🙂@GDScript
:If GDScript ever supports named arguements:
otherwise:
AudioStreamSample
->AudioStreamWAV
. Documentation states it can be used for PCM audio data, so maybeAudioStreamWAV
should inheritAudioStreamPCM
?Edit: https://github.com/godotengine/godot-proposals/issues/4532
I would say rpc is usually reliable and rset is usually unreliable but those should remain the same for consistency. If there’s anything that should be done it would maybe be a warning against using rpc or rset reliable in _process
Yes, this issue. The opening post here lists every rename that was/is considered, has a checkmark if it has been done and a link to the appropriate PR.
It is.
Vector2.angle_to_point(Vector2 other)
could be removed as it same as(a-b).angle()
, see #16307.Personally,
(a-b).angle()
is more intuitive.@Error7Studios For
get_data
it was already proposed, PRed and merged: https://github.com/godotengine/godot/pull/47435 Andset_data
doesn’t exist onmaster
(the method was effectively replaced withupdate
in https://github.com/godotengine/godot/pull/36098).@nathanfranke Since there’s already
Mesh
,MultiMesh
, andNavigationMesh
, it’s pretty clear that theInstance
part is not redundant and has a special meaning.See the relevant documentation for these classes: https://docs.godotengine.org/en/stable/classes/class_meshinstance.html
(If that’s not clear enough, it’s a documentation issue.)
signal
Tabs close/tab_closed
should beclose_pressed
(orclose_button_pressed
) It is emitted when closeX
button pressed, not when a tab closed/removed@Calinou It is not a good name thought. All *Script classes are extended from Script but JavaScript not. I can’t find a better name for JavaScript language binding as there’s no namespace in godot script binding system.
I actually like these functions, they make code very easy to read. Considering that showing and hiding is very common, I see no reason to remove them.
I’ve noticed that
2
is used throughout GDScript language exclusively compared to other core/scene classes which useto
, so perhaps we already have consistency here… Those methods are technically part of the language, so GDScript aims to be less verbose.I welcomed
append
, and was confused by the lack ofprepend
.Another advantage of
push_back
is the nice symmetry withpop_back
. An advantage ofappend
is that it makes the code look a little more idiomatic.If performance is not impacted by aliases, it boils down to whether the expense of maintaining aliases is worth the comfort (sugar) and learning curve flattening provided to Godot users.
@nathanfranke the ultimate solution for this would likely be #26990.
I think the length of the naming name should be moderate. If people who have never learned programming read this code, it is like reading an English article again, which is great. Even if I haven’t learned programming, I know what the code is doing. The name should avoid ambiguity, avoid too many abbreviations, too many abbreviations, you need to check the document to know its function, but not intuitively see its function. But also avoid long names, because this will make the code too verbose and not concise enough. A balance must be found between the two! Disclaimer: English is not my native language. Too many abbreviations is really difficult to understand its function quickly, unless you remember the function of this function. The above is my personal opinion and my experience.
@opl- this is also discussed here: https://github.com/godotengine/godot-proposals/issues/287
This is covered by #41794
The whole existence of this megathread is the evidence that people are asking for it. These problems may be insignificant for you or someone else, but people do have problems with understanding the API because of bad naming. And this is the type of problems this issue collects. As for the significance of these changes overall, believe it or not, but tracking and implementing these is not taking away from the other development time.
And look at the example you gave and tried to explain:
You write that and see no reason to improve naming to be at least a bit more clear for all current and future Godot users?
@CarlGustavAlbertDwarfsteinYung
empty
can be an action if it’s read as a verb.is_empty
is definitely a quality. Of course that confusion may depend on your level of English.Also, even if a function was called
send_formatted_string_to_standard_out
in a modern code editor it can be typed assfstso
, or any other combination of the intermediate letters, if you so desire and the autocomplete will give you the only option.Why all these renames are so long? You’re changing something simple and short to something really long.
You’re changing
clip
tointersection
twice as long to type. You’re also changingControl.MOUSE_FILTER_PASS
toControl.MOUSE_FILTER_PASSTHROUGH
etcI would prefer simpler and shorter less verbose changes. It’s a function name not also a description of the function.
I discovered methods
property_can_revert
andproperty_get_revert
in the Inspector and reported them in #43078. However, I think they should be renamed with leading underscores to follow the conventions of_get
,_set
, and_get_property_list
.@aaronfranke For the other 207 cases, do you also suggest creating separate functions? If not, then this argument is not conclusive. In addition, I wrote above that if you can specify the parameter name, then it becomes readable without an IDE.
Often, but not more often than without echo.
The presence of three methods (
is_action_just_pressed
,is_action_just_released
,is_action_pressed
) is confusing because you expect there to be anis_action_released
method.ADD. Which option is easier, at least visually?
The following methods should be changedInconsistencies betweentofor consistency with and
need to be corrected.
P. S. I proceeded from the principle “The fewer similar methods, the better”.
@Xrayez I would avoid using the word “cast” at the start of a property, since I naturally read that as the verb “cast” and not the noun “cast”, and therefore
cast_position
likely wouldn’t have solved the original problem of not knowing this is a property and not a method (it would be easy to assumecast_position
is a method that casts to a position). I liketarget_position
.Get_node is not limited to getting descendents, so that would be a very misleading name. The 2 methods need very different names because what they do is very different.
On Sat., Jul. 25, 2020, 3:14 p.m. golddotasksquestions, < notifications@github.com> wrote:
@MuffinManKen
independent
doesn’t sound very self-explanatory to me either.What about
exported
orrelease
?In Godot 3.1, I added a
standalone
feature tag which evaluates totrue
when the project is running from an export template binary (debug or release). The opposite iseditor
, which evaluates totrue
when the project is running from an editor binary.However, with time, I think it would be better to rename
standalone
torunner
as it’s shorter and slightly more self-explanatory. What do you think?I find
get_node
good, butfind_node
could be renamedfind_child
String
we should dropfind_last()
in favor ofrfind()
. Not exactly a rename, but it’s still worth discussing which one to keep.It’s not “normal” set/get, they have an index for the target Surface (it’s a Vector inside Mesh class)
Texture (Texture2D) and Image
@MCrafterzz People open pull requests to rename things on a regular basis. This is done incrementally rather than all at once.
If the removal happens,
get_ticks_usec()
description should mention the separators.@aaronfranke which is also a
Container
node, actually^^ but yeah… main screen can work too@swarnimarun We have many other case-insensitive methods ending with
n
. If we rename one of them, we should rename all of them.Should Node2D/Node3D
to_global
andto_local
be removed? I gave feedback on #38902 which proposed adding methods that only work with the basis, but there are some issues with these.In the demo projects,
to_local
is not used anywhere, and$ grep -RIn "to_global"
returns only 5 results, all of which are in the GUI in 3D demo, and the performance of the demo could be improved by changing this so that it caches the global transform and usesxform
instead of usingto_global
.In a nutshell, the concern with these methods is both that they are uncommonly used, and also that they encourage writing code that performs bad since it recalculates the global transform several times.
@KoBeWi Instead of renaming
zoom
, shouldn’t we invert its scale so it zooms in when you increase the value?This will break compatibility just like renaming the property would. If we go the “invert scale” route, it can’t be fixed automatically, unfortunately. Still, it might result in less confusion in the long term.
Just because someone else does something doesn’t mean it still doesn’t cause confusion and is a good idea.
On Mon, May 18, 2020, 1:09 PM Jummit notifications@github.com wrote:
Note that they are called layers in Maya, Lumberyard and Unity.
Script::get_instance_base_type()
specifically returns the native type underneath. Now that we have named script classes, it makes more sense for this to be renamed toget_native_type()
since the “base” of the script could potentially be the name of a script. It’s misleading.Suggestion: make
find_node()
'sowner
parameter false by default.@Zireael07 I would prefer if we only had one way to get the tangent. I don’t think it’s a very common operation after all.
You could also keep
.tangent()
as just an alias for.perpendicular()
, no?map() is probably reserved for the new functional programming features.
There are cases in which Control nodes are refereed to as Modal nodes. https://github.com/godotengine/godot/search?q=modal&unscoped_q=modal Mostly in relation to the Viewport.get_modal_stack_top() function
InputEvent.is_action_pressed to InputEvent.is_action_just_pressed
InputEvent.is_action_released to InputEvent.is_action_just_released
https://github.com/godotengine/godot-proposals/issues/316#issuecomment-589426014
I like the “real” name personally, since the term “float” is often used for 32-bit floats specifically, but Variant’s real is a double, and most of the engine uses
real_t
which can be either.P.S. We already did that rename the other way around in 2017.
another inconsistency with physics body signals
Area:
area_shape_entered( int area_id, Area area, int area_shape, int self_shape ) area_shape_exited( int area_id, Area area, int area_shape, int self_shape ) body_shape_entered( int body_id, Node body, int body_shape, int area_shape ) body_shape_exited( int body_id, Node body, int body_shape, int area_shape )
i suppose area_shape in these last two should be self_shape ? or …
Rigid Body:
body_shape_entered( int body_id, Node body, int body_shape, int local_shape ) body_shape_exited( int body_id, Node body, int body_shape, int local_shape )
here it is called local_shape …
I think that
is_a_parent_of()
should be renamed tois_parent_of()
AnimationPlayer method .stop(false) should be called .pause(true) because that’s what it does.
Physics “Layer”, Render “Layer”
I remember going exactly through the same confusion as this poor person during my first few weeks of learning Godot.
Physics “Layer”, Render “Layer” might make sense to someone coming from Object Orientated Design, but to anyone else it’s very confusing. The term “layers” is used when describing multiple coatings of paint, or coats of cloth in a fabric, coats of skin on an onion (or ogre), coats of sediment on the planet surface, … Layers (plural, opposed to a single layer), seem to imply being stacked on top of each other in all those cases.
For many people “layering” especially when working with 2D graphics or animation implies working with a foreground, background, and layers in between or on top. Many people think of layers as celloloid films on top of a background, when in fact Godot like many other game engine uses various ways of “sorting” to accomplish this task.
The fact that we call the abstract commonality that Physics objects or Render objects might share “Layers”, while at the same time these abstract commonalities have nothing to do with the stacking appearance of visual layers (that’s “sorting”), is what makes this so confusing to some.
When I understood the real use and meaning of Physics “Layer”, Render “Layer”, I immediately wondered why these are not Physics Group, Render Group, and to be honest, I still don’t know. They certainly don’t have any ontop-stackable relation to each other, meaning their order or relation to each other is completely irrelevant. Naming them layer, imho archives nothing but confusing people, “group”, “affected by group” for masks, or maybe some other term I can’t think of right now would be more accurate.
I would expect 4.0 to bring a big wave of new people into Godot due to better performance, GI probes improvements, general hype and so on.
If those changes are made with 4.0, these people will get a hard landing as almost no existing tutorial (besides the official “first game” tutorial) will work for them anymore.
If those changes are made after 4.0, lets say in 4.1, it will be an extremely bumpy ride for those people. They just learned the basics a new engine, which they now have to relearn again. Beginnings are hard, having to go through this twice too shortly after another can be frustrating enough to give up altogether.
Would it therefore not make sense to have a “3.3” or “3.9” version before the 4.0 release that has all those compatibility breaking API changes, however gives the tutorial creators enough time to create some tutorials before expected massive influx of new users?
@Xrayez Change
print
towrite
? From Python to Pascal? 😆is_processing
:can_process
:It might be better to rename
can_process
to something likecan_process_paused
to avoid confusion withis_processing
method.When working on #31976, I noticed the shadow atlas value must be a power of 2 (both for directional shadows and omni/spot shadows). However, the methods accept any integer value; if it’s not a power of 2, the method will round it up to the nearest power of 2. We should probably make it accept an enum of values, so it can be presented in an user-friendly manner in the Project Settings.
Likewise, the anisotropic filtering level should be an enum (2×, 4×, 8×, 16×), since only power-of-two values make sense here.
Geometry
’spoint_is_inside_triangle()
tois_point_in_triangle()
, to match the other methods that also returnbool
s in the same class.The
speed
property of the mouse and touch input events should be renamed tovelocity
since it’s a Vector2.PopupMenu’s has
index_pressed(index)
andid_pressed(id)
which work fine, but alsoid_focused
which is actually emitted with the index instead of the id of the item.@razcore-art Already a PR open about this https://github.com/godotengine/godot/pull/20371, and it keeps backwards compatibility (I think).
EDIT: Doesn’t anymore.
Should we have
lerpf
/lerpv
/lerpc
instead ofColor/Vector2/3.linear_interpolate
? At least renameColor/Vector2/3.linear_interpolate
toColor/Vector2/3.lerp
@aaronfranke
get_native_class()
perhaps? Then you could get the script name fromget_script().global_name
if it has one.Come to think of it, maybe there should be a
set_cellv
for GridMaps as well?Building on what @lupoDharkael said, Godot has several places where double negatives are used. Errors like “Condition !Math::is_nan(x) is false” are confusing.
As pointed out by @clayjohn,
WORLD_MATRIX
in CanvasItem shaders is actually a modelview matrix. @reduz agrees that in 4.0 we should replace it by actual model and view matrices, e.g.CANVAS_MATRIX
andITEM_MATRIX
.Morning! @akien-mga couldn’t we rename
instance
tonew
instead ofinstantiate
? Having the same interface onPackedScene
asObject
s do for example removes some boilerplate for plugin creation especially, but probably more generally. @willnationsdev what do you think? I know you encountered this before.Sorry if there is already a talk about this somewhere, I couldn’t find it by skimming through.
or just:
grabber_area
->progress
slider
->background
?Well, on the other hand I like that global-related function (like global_position, global_scale and global_transform) are next to each other in the auto-completion suggestions. Both solutions make sense IMHO.
load_from_globals()
in InputMap should beload_from_project_settings()
.Need to remove
get_selected_id()
fromOptionButton
currently it always returns -1. After https://github.com/godotengine/godot/pull/21837 was mergedget_selected_id()
will return the same asget_selected()
.@aaronfranke I agree that using
Get-
orSet-
prefixes is sort of a ‘Javaism’ which is better be avoided in C#.My main concern was the usage of ‘domain prefix’ as in the case like
shape_owner_get_shape
ornode_get_input_count
, so if we can reimplement them as proper C# properties withoutGet-
orSet-
prefixes, it’d be even better.On a side note, I always thought it rather odd that properties in Godot’s C# API have matching set of getters and setters, like for example,
Node.Name
andNode.GetName()
/Node.SetName()
.It feels rather redundant to me, but if there’s any reason why we keep such a convention, I suppose we would get both
NodeInputCount
andGetNodeInputCount()
/SetNodeInputCount()
anyway, if we are to renamenode_get_input_count
as suggested.There’s a few methods for
RigidBody
and it’s related classes whose parameters should be swapped for consistency:RigidBody.add_force(force, position)
toadd_force(position, force)
PhysicsDirectBodyState.add_force(force, position)
toadd_force(position, force)
PhysicsServer.body_add_force(force, position)
toadd_force(position, force)
Class and method list
@aaronfranke GDScript does have properties in a way, since engine classes can define a getter and setter (or only a getter) and expose to GDScript as properties. That said, I’m against removing
get
andset
from the methods because 1) it makes the name clearer and 2) it makes a distinction between getter and setter. For instanceMesh.SurfaceFormat()
sounds like a method that “formats the surface”, and not one that “gets the format”. Also, most (if not all) of those can be ignored and used as properties instead anyway.Now, I don’t care that much about
gdscript_function.cpp
andgdscript_functions.cpp
. One has the GDScriptFunction class, the other contains the definition of the GDScript functions, it’s always clear to me which is which (though I’m used to it). It’s also not a breaking change, so it doesn’t need to be here.We should get rid (or seriously discuss) binary resource extensions… (RES_BASE_EXTENSION)
From this #20228,
Label.clip_text
is not necessary anymore. I believe it’s the same for Button.clip_text.The methods changed in #16757. The argument order makes more sense, but it breaks API compatibility between 3.0 and 3.1 (#19648).
rect_min_size Control.set_custom_minimum_size(value) -> Control.set_rect_min_size(value) Control.get_custom_minimum_size() -> Control.get_rect_min_size
There are many more in Control class the get/set should all have the same name as the variable it is annoying to check the docks every time when you know the variable name.
Sprite.set_region(val) -> Sprite.set_region_enabled(val) Sprite.is_region() -> Sprite.is_region_enabled()
#16116