gdnative
gdnative copied to clipboard
Naming inconsistencies
This issue collects godot-rust symbols, whose current names could be made more expressive in a future version. This includes anything: types, functions, macros, variables. It excludes generated names from the GDNative bindings, as they are not chosen by us*.
All of these are up to discussion, of course. Migration could happen step-wise: v0.10 deprecates the names and adds type aliases for the new names (or for old ones), and v0.11 removes old names.
-
[x]
RefInstance
->TInstance
Analogous toRef
andTRef
, the latter is the temporary, lifetime-bound variant of the former.RefInstance
suggests that this is a mix betweenRef
andInstance
. -
[x]
TypedArray
->PoolArray
Would immediately reflect that it's the Rust counterpart of GDScript'sPool*Array
s. We need to decide if it makes sense to keepInt32Array
andFloat32Array
around. -
[ ]
VariantArray
->Array
(?) This one is a bit less clear: while it would match GDScript's typeArray
, the name "Array" is very general and doesn't clearly describe the purpose. On the other hand, the same problem occurs withDictionary
. However, "dictionary" is quite specific to GDScript terminology; in Rust such types are typically called "maps". -
[ ] Conversions between
Ref
,TRef
,Instance
,Script
etc. Some terms are used interchangeably (base/owner), while some are not related to the conversion involved (claim). This is OK to some extent, but we need to be careful that there are not too many names that need to be "learnt by heart", as that makes APIs less accessible. -
[x] Type states; e.g.
Access
->Ownership
See this comment.
Related, but planned for v0.10: #712
* Note regarding GDNative types
A lot of them have multiple capital letters, e.g. ARVRCamera
, AudioStreamOGGVorbis
, PCKPacker
, UPNP
, WebRTCPeerConnection
and more. Would likely cause more confusion than benefit to "correct" them, add lots of manual special cases, and sometimes become obsolete in Godot 4. What needs to be checked though is why this was done for some types like G6dofJointAxisParam
. Godot seems to use G6DOFJointAxisParam
, and GDNative apparently too. But where does the de-capitalization in godot-rust originate?
@Bromeon I like the idea of simplifying these.
General thoughts
-
TInstance
is a lot easier to immeditately comprehend than the currentRefInstance
- Making the array types (across the board) match their GDScript counterparts is a good idea. Sometimes it can be really tricky to figure how which one you actually need.
- I think that the only exception to the above is
VariantArray
. Since it must hold onlyVariant
typed objects, it could be worth keeping that clearly stated in the name. Fortunately if we make the change toArray
since Rust arrays are declared[a, b, c; 3]
there's little risk of confusion or name collision. - Access should probably be
Ownership
since that's the real semantics being used. (Unless Ownership is a reserved keyword or type or something).
Regarding the conversions between types, is the goal to create some extension methods to help with the conversions?
Thanks!
I think that the only exception to the above is
VariantArray
. Since it must hold onlyVariant
typed objects, it could be worth keeping that clearly stated in the name.
The same thinking applies to Dictionary
though, and we don't call it VariantDictionary
. On the other hand, there are also no Pool*Dictionary
variants.
One practical aspect: there is no standard Array
type with which it could collide, is there a very popular crate with it?
Regarding the conversions between types, is the goal to create some extension methods to help with the conversions?
Not necessarily, the conversions can be part of the type. Two things are important to me:
- if one wants to do a certain conversion (let's say
TRef
->Ref
orRef<T, Shared>
->Ref<T, Unique>
), they know where to look - same operations/concepts are named the same in different places
TypedArray
->PoolArray
Would immediately reflect that it's the Rust counterpart of GDScript'sPool*Array
s. We need to decide if it makes sense to keepInt32Array
andFloat32Array
around.
The aliases were mostly for backwards compatibility, and can be confusing to new users. It should be nice to remove them in favor of a singular PoolArray<T>
interface.
The remaining renames can be implemented during 0.10 (with deprecation) and/or for 0.11 (with removal). Will need more discussion anyway.