Godot: Improvements on class, function, property naming for 3.0

Created on 17 Sep 2016  Â·  69Comments  Â·  Source: godotengine/godot

Godot API has been barely unchanged for 8 years. For version 3.0 we'll take the chance to break some compatibility to make the API less confusing in many aspects and improving it in others.
Is there any method, functions, class, etc. that you have found confusing in the past, that you would wish it had a different name?
If so, this is the right time to start pointing it out! Feedback will be very welcome!

image

discussion enhancement

Most helpful comment

functions like file_exists() could be directly accessible without being forced to create File or Directory object.

Current shortcut is File.new().file_exists().

All 69 comments

There are already API discussions here https://github.com/godotengine/godot/issues/5696

I'd propose to close as duplicate of #5696 so that the discussion stays there.

btw I think "Click on Press" means the button will consider itself clicked when it is pressed rather than pressed+released. Which makes sense in the end.

AnimatedSprite node to SpriteSequence node?

set_enable_monitoring( bool enable ) to set_monitoring( bool enable )

In Range there is set_unit_value() which probably could be renamed to set_percentage(), my first thought on set_unit_value() is that it sets something rounded to integer. get_unit_value() also could be renamed to something with percentage.

Also in Range there is a pair set_rounded_values() and is_rounded_values() (in this case better name would be has_rounded_values). I'm not sure what it does, doc is empty, it may round values to integers or to step value? From name I would guess that set_rounded_values() will take some collection as param. Still if it rounding to integers or steps I think it's better to rename it to is_rounding() and set_rounding() or is_rounding_to_step()...

Next one in Range is get_val() and get_value() and setters. Do they do the same or different thing?

In String there are methods with is_valid prefix. It suggest that for example floats can be divided in two groups: valid ones and invalid. Probably these could be simplified to is_float(), is_identifier() and so on...

Isn't related to GDScript, but what about having all of the things in the Inspector/Project Settings/Editor Settings, etc. finally in alphabetical order? :) The sections in alphabetical order and the properties / options within those sections also in alphabetical order... it's really difficult to find stuff as it is

The problem with alphabetical order would be that related stuff could end up at the wrong place. If the problem is about finding a property, I would prefer be able to hover the inspector and Ctrl+F to highlight properties with a given name.

@Zylann I was afraid I would be misunderstood so let me do it visually:

alphabetical_order

I agree that "list of properties" like the project settings and editor settings could be in alphabetical order, however that's really the inspector I was talking about. Inspector properties should be ordered by topic, not name. For example, in your screenshot, Vframes would end up far away from Hframes and Frame, but it's better to have them together with Frame as last.

Properties are ordered by importance, so I generally think this is OK as is
(maybe alphabetical could be an option). For the editor and project
settings I know this is a mess, will fix with proper setting subsections
for 3.0

On Sep 17, 2016 22:16, "Răzvan Cosmin Rădulescu" [email protected]
wrote:

@Zylann https://github.com/Zylann I was afraid I would be misunderstood
so let me do it visually:

[image: alphabetical_order]
https://cloud.githubusercontent.com/assets/1177508/18612035/edf4c860-7d4d-11e6-8dbc-2c087c53e907.png

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/6523#issuecomment-247817776,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AF-Z25zHR1SwVQsL6p6EZZd2A3WRIoysks5qrJDigaJpZM4J_lvQ
.

I was also about to point out the Particles inspector. There are tons of properties there, maybe more in the future. I wonder if a raw list of properties is a right way to customize particles? But it may not be related to this topic :p

I think the engine could use some consistency with it's naming schemes and organization.
Like we have Node2D but then Spacial instead of Node3D.
Then we have Camera2D and... Camera? no 3D.
I mean, to me it's all clear and it is color coded, but for new users and for possibly faster coding in some cases, I think we could standardize and everything that is for 2D will have that suffix, same with everything that is for 3D.
Also, "control" doesn't scream GUI/UI to most people, probably even calling it "CanvasItem" or simply "Canvas" would make more sense to many, and then maybe renaming CanvasItem to Base2D. Personally I'd call it NodeUI, again for the sake of standardization.

Maybe there are better ideas then what i suggested, but the bottom line is these things could use some work. Right now they are not very immediate for the first timer. A lot of the need for tutorials is gone with names that clearly communicate everything, and to me Godot still has the potential of being the easiest engine to pick up and use, by anyone.

Click on press could be "Click When Pressed"?

@Zylann @reduz yes, I agree on Inspector too, it's just something I didn't think of :))... it's 4 AM here, my mistake. But the rest stands.

@KioriSun This is in because Godot was originally a 3D engine and 2D was added later on. That said I kind of don't mind, except it might make sense to rename Spatial to Node3D, but not sure how much.

For GUI, the only standard namings for these kind of things are "Control" and "Widget". Canvas and CanvasItem are also standard namings for what they actually are in other software packages, so not trying to change much of that.

@reduz
About control and widget, yes absolutely, they are the norm.
But, i didn't mean to say that Godot should follow any one's standards but rather that it needs 'a' standard, for clarity of meaning.
In that context, my thought was that 'control' while normal it's not exactly immediately identifiable for first timers. And that maybe it could be tweaked for the sake of non tech users.

The entire focus of what I'm saying is user friendliness.

Just to clarify my own statement.

Thanks for taking the time to reply, it's fantastic that you guys are even considering these types of changes.

About control and widget, yes absolutely, they are the norm.
But, i didn't mean to say that Godot should follow any one's standards but rather that it needs 'a' standard, for clarity of meaning.
In that context, my thought was that 'control' while normal it's not exactly immediately identifiable for first timers. And that maybe it could be tweaked for the sake of non tech users.

Naming scheme that is easy and consistent even for high school students without reinventing the standards which has a clear distinction of 2D, 3D, Control/UI, etc will be good. I am using Godot as a game coding teaching tool and this will help me most. Also #4951 (edited: It is not about API). Younger ones demand faster responses. :)

functions like file_exists() could be directly accessible without being forced to create File or Directory object.

Current shortcut is File.new().file_exists().

6235 - yet another area of naming inconsistency, coupled with a lack of a matching function.

make Array.pop_front() or .pop_back() returns the popped value instead of void ( discussion here #3763 ) or rename them remove_back() and remove_front().

give the same API to all type of arrays.

for instance, sorting StringArray() requires to convert it to Array(), and then back to StringArray().

Add Spatial.get/set_global_translation() as a shortcut and speed optimization for when we don't need/want to get/set_global_transform() the full matrix.

This would also make the Spatial API more coherent with the Node2D and Control API which offers get/set_global_pos().

We have a Image.get_data() which returns a RawArray, but it does not allow to update the actual content of the image because the returned array is a copy, not a reference.

Also, there is no Image.set_data(), and no Image constructor that accept a RawArray to initilize.

There is no fast/optimized way to initialize/modify the content of an Image from memory other than the slow put_pixel().

related discussion : #6315

edit : ImageTexture does not have a create_from_rawarray() neither.

Also, if we load a "PNG" or "JPG" from network, there is no way to create a ImageTexture directly from memory. We are forced to save the "PNG" or "JPG" to disk to load it back to gdscript.

On machines that use a Flash/SSD it is a waste.

Just had a talk about calling super class methods on IRC. So if you have this:

class A:
    func do_something()
        print('A')

class B extends A:
    func do_something()
        print('B')
        .do_something()

then the .do_something() in B calls the A class do_something (this isn't documented in the docs by the way). I think this is confusing, considering all of the ways we can call methods in classes:

self.get_node()
get_node()
.get_node()

the first two meaning the same thing, while the third meaning a call to get_node() from the class we're inheriting from. I think we should at least have something different for the call to the class we're inheriting from, something like super().get_node() or super.get_node() (to be similar to self.get_node()) or something...

I'm more used to base.foo(), but that's taste and colours :)
Also, FYI self.foo has a subtle difference than foo, because it works as if retrieved from the outside. For variables with setget it's different, though there is not much difference for functions yet.

@Zylann I'm not talking about differences in implementation, but the differences for the end user (the game devs.). Yah, I knew with programmers there was no way this wouldn't be pointed out :P

@razvanc-r:

this isn't documented in the docs by the way

It is documented: http://docs.godotengine.org/en/latest/reference/gdscript.html#referencing-functions. There's also an extra point about constructors: http://docs.godotengine.org/en/latest/reference/gdscript.html#class-constructor

@vnen my bad, I saw this https://github.com/godotengine/godot-docs/issues/249 when it wasn't answered and skimming through the docs I couldn't find that information because it is in a rather unusual place, I would have imagined it would be under "inheritance", not under "functions"

Oh yah, those fill and expand properties for the horizontal and vertical size flags for the Control nodes need to be reviewed, those are confusing and not documented well. Also I think they only apply tor Containers? That is if we have this hierarchy:

  > Button

Then if setting size flags on the Button node does nothing so I think this needs some review...

The Tween-class has the method interpolate_property(...), which has a parameter times_in_sec. I find the name so very strange... I mean, "times-in-sec" tells to me "how many times something is called per (one) second" - yet the meaning is actually "duration in seconds". I would understand if it were time_in_secs. Am I missing something?

In the completion for _process(delta) and _fixed_process(delta) modify delta to dt. delta is more generic and in physics it is attached to all variables that represent a difference, by definition: deltaA = A_f - A_i where A_f = A final and A_i = A initial, so it doesn't make sense to have only delta.

Also going back to my discussion on names (#6599), I'd suggest _process_fix instead of _fixed_process, this would convert process to a verb which is how functions should be named.

To keep it consistent this is what I'd suggest:

  • _process(dt) with set_process
  • _process_fix(dt) with set_process_fix
  • _process_input(event) (instead of _input(event)) with set_process_input
  • _process_unhandled_input(event) (instead of _unhandled_input(event)) with set_process_unhandled_input
  • _process_unhandled_key_input(event) (instead of _unhandled_key_input(key_event)) with set_process_unhandled_key_input

This will make it clear and consistent what these functions are supposed to do.

_Or maybe you guys have a better name for _process_fix?!_

@razvanc-r I guess that in this case _process (_what?) would be ugly, and would have to be changed as well :smile:.

So maybe _process -> _process_frame and _fixed_process -> _process_physics or something like this...

Also, I'm highly against dt as an auto-completed parameter name, since it excessively unintuitive to people without the said physics background (but ΔT is even worse), plus it looks like a shortened variation of [D]el[t]a.
I would propose step as in PhysicsBody[2D]DirectState::get_step, or maybe just leave it as-is.

@bojidar-bg yes, very nice idea! I was too close to the forest and couldn't see the trees :P, I second _process_frame and _process_physics.

About the delta, I'm against using only delta, maybe delta_t or even delta_time then would be better. I'm also against step because this would imply a fixed step and for _process_frame that's not true. I didn't know about that get_step function in PhysicsBody[2D]DirectState, I suppose that should then be renamed to PhysicsBody[2D]DirectState.get_delta_time or whatever it is we choose for the autocomplete of _process_physics.

_edit_: When I first looked at Godot I was super confused about this _process and _fixed_process and I'm always for explicit vs implicit and concise.

_fixed_process() is also used to process logic at regular "dt".

so, renaming it _process_physics would become counter intuitive too.

btw, _fixed_interval_process() would be longer, but more obvious

@SuperUserNameMan Naming it _fixed_interval_process would imply that the parameter is called interval BTW. Sounds good by me.

I settled, I suggest _process_render and _process_physics.

I think these are clear enough in giving the information that one is called when the frames are drawn and the other when the physics is calculated. I don't like _fixed_interval_process or the other as this doesn't imply that it's called for/by the physics server. Also it doesn't adhere to the "function names should be verbs" convention. In the docs it can be explained that _process_physics is called at regular intervals and I think it's enough.

See #6599

Also, for 3.0 how about we make these buttons toggle buttons instead? (and any other that I don't know of):

godot

And I think having the Collision Layer and Mask for RigidBody[2D], Area2D etc (ligts and everything else). with tags (like Unity) is a way better thing for the game developer, you can easily forget on what layer you've placed a certain object but with tags it's a lot more intuitive, you know that the enemy needs to interact with the player so maybe you put it on layer 'enemy' & layer 'player', instead of ticking 'second' and 'third' boxes... With tags then we could also have search of some sorts, list all the nodes that are in layer 'enemy' for example

MaterialShader and ShaderMaterial are confusing too.

I was digging in the Tween and found that start(), reset(), stop(), resume() and remove() return always true. Probably it was meant to show to the user that tween with such property was found but it's not implemented. So probably these functions can return void.

Some nodes have set_rotation & set_rotationd and others have set_rot, set_rotd, let's stick with one.

Also for 3.0 I think it would be time to have a rotate_around function to rotate around a given point/axis and also apply rot/loc/scale like blender has. It would be super useful.

@razvanc-r I think the -d versions are there because they expect degrees instead of radians. So that actually means we must settle for one of the two units.

Speaking of rotations, in 3D the scene editor and inspector seems to work in euler (and the poor piece of software gets a bit crazy if you work with local transform), is there a way to make all work with quaternions while showing euler to the user?

This includes set_rotation methods.

@razvanc-r in addition to rotate around pivot, I'll like to see more relative transforms and relative move() for KinematicBody (2D and 3D), could be useful for visual script.

@Zylann, I'm not talking about set_rotation vs set_rotationd, I'm talking about set_rotation vs set_rot also for the other.

Also... WTF is Node2D.edit_set_pivot?! edit_set_state, edit_set_rect?!?! Also, some stuff is placed in Node2D like get_transform and some other stuff is on CanvasLayer like get_global_transform. etc. Shouldn't this be unified?

For 3.0 I'd also like to have more standardized names for the maths functions. Example, there's lerp that works on floats only, but then there's Vector2.linear_interpolate, I think it makes sense to have lerp work on both float and vectors, can't think of any other right now but yah...

@razvanc-r Quaternions? Colors? (everything AnimationPlayer can interpolate, actually)

@Zylann yes, exactly, let's keep it uniform 🙃. All of them should have the same method for interpolation and math related computations in general.

OK, I'm not sure if there's a reason why Node2D has some transforms and CanvasItem has some other, like get_transform, set_transform, translate etc. I think all of these should be part of Node2D because those are the properties related to positioning which should be at the most basic object. Also there's get_transform in CanvasItem returns Matrix32 and get_transform in Spatial returns Transform, shouldn't these be returning the same thing? For example with Spatial you can get the transform then do transform.origin += something, but you can't with CanvasItem

RayCast2D should update its state immediately after moving its parent if it has one or after calling cast_to (I don't know if RayCast works different). As it is at the moment if for example we have

func _input(event):
    ray_cast.cast_to(Vector2(9, 0))
    ray_cast.is_colliding() # <- here the old collision is returned, not the new

@razvanc-r that's because the physics engine is only synced in _fixed_process(), currently there is no chance to get such result outside of it. Some explanations here https://godotengine.org/article/why-does-godot-use-servers-and-rids

Range has set_val and set_value, I think both do the same.
edit:
Also, it can have a get/set percent, and some methods to go forward/backwards one step, that could be handy.

@Zylann I know but that doesn't mean it wouldn't be a very helpful feature. For example why is it possible to rotate the RayCast2D through a parent, say Node2D and get the correct result in _input but you can't use cast_to and get correct result in _input? Makes me wonder, it seems that rotations are synced with physics, but cast_to is not so there must be a way to get cast_to to update the RayCast2D to get correct results in _input as well.

_edit_ I might have spoken prematurely about that rotation :). The way I sync it with the fixed process is calling two times yield(get_tree(), 'fixed_frame')... why it needs to skip two frames I don't know... but that's the way it works it seems. I suppose this is ok :).

By this 3.0 version godot should probably get proper standard radio buttons which are separate from the checkboxes that are used everywhere...

checkbox and radio buttons have to be grouped by linking to a variable (radio) or array (checkbox) which is used to group them. Sort of "bound to variable" property may be useful.

could someone define what is API? Is that interface of any software or interface programming for our suite, needs?!

@linuxtopia I think there is no specific definition as we have seen in the "googacle" trial, but most people refers to something like the interface exposed to the user (here, the Object tree I suppose)

I find get_global_mouse_pos() not intuitive.

Maybe should it be renamed get_mouse_global_pos() since we are looking for something belonging to the mouse ?

Viewport has signal size_changed() and Control has signal resized(). Probably size_changed() for both would be better.

Control's set_custom_minimum_size() triggers minimum_size_changed signal also when minimum size was not changed.

AnimatedSprite calls signal frame_changed() on explicit set_frame() call ie. in set_animation() but not on frame++ when processing.

TreeItem.get_children() seems to return TreeItem.get_first_child(), not children.

String.hex_to_int() requires the string to begin with _"0x"_, otherwise it returns _0_. There are two problems with this, the first being that most users may expect the contrary behaviour or both to work. The second problem is that it silently returns a value that cannot be differentiated from a successful parsed hexadecimal string.

My proposal is to not force this requirement or to print an error and specify the requirement in the documentation.

Another thing that must be specified in the documentation is that this method returns a negative integer if the string begins with _"-"_, while it works with two's complement.

Tree::get_single_select_cell_editing_only_when_already_selected(), surrealist...

I think the 0x problem was fixed in the ipv6 patch

On 16 November 2016 at 16:19, BBric [email protected] wrote:

Tree::get_single_select_cell_editing_only_when_already_selected(),
surrealist...

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/6523#issuecomment-261043729,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGVmPcZ4fXqGuQBtnHUzNtyltY0CvYC6ks5q-1cqgaJpZM4J_lvQ
.

@punto you are right, I am on a different computer so I didn't notice my local repo was this out of date.

@BBric very Java-ish :laughing:

How about having mouse events that mimic the (mobile) screen events? For example, there is SCREEN_DRAG but no MOUSE_DRAG. Also with this in mind, the ScrollContainer should have scroll with dragging, not just using the scroll bars. :)

most of these were taken care of, so closing

There were actually quite a few proposals not taken care of yet, I've added some to #7516. Many proposals here were more feature requests for API changes than stuff that just needs renaming - more complex API changes should likely be discussed in their own issues as they can be easily overlooked here (as they were during @reduz's first pass, and I'll admit I skipped some big suggestions when looking for items to add to the todo list in #7516).

Was this page helpful?
0 / 5 - 0 ratings

Related issues

RebelliousX picture RebelliousX  Â·  3Comments

Zylann picture Zylann  Â·  3Comments

EdwardAngeles picture EdwardAngeles  Â·  3Comments

bojidar-bg picture bojidar-bg  Â·  3Comments

mefihl picture mefihl  Â·  3Comments