godot-rust / godot-rust/gdext

Low-hanging consistency fruits

Open
#756 0 comments 3 reactions 0 assignees View on GitHub
c: engine quality-of-life
Dominant language
Rust
Stars
5.2k
Forks
312
Avg merge
12h 59m
Merged PRs (30d)
9

Description

This is a very cosmetic issue, and not urgent at all :slightly_smiling_face:

The engine often uses different representations for the same thing. In some cases that's OK or would be hard to change, but in others it's a special case in codegen that may be simple to add. Just to brainstorm, doesn't mean we change all those:

## Instance IDs

We have `InstanceId`, `ObjectID` (native structs, wraps `u64`), raw `u64`, [raw `i64`](https://godot-rust.github.io/docs/gdext/master/godot/global/fn.is_instance_id_valid.html), [`EncodedObjectAsID`](https://docs.godotengine.org/en/stable/classes/class_encodedobjectasid.html)...
- [ ] [Occurrences of `instance_id`](https://godot-rust.github.io/docs/gdext/master/godot/?search=instance_id)
- [ ] Ideally we can replace everything with `InstanceId`
- [ ] Heuristic: methods ending in `_instance_id` that return `u64` or have an `id: u64` parameter.

## Intly typed enums

- [x] [`Object::connect_ex()`](https://godot-rust.github.io/docs/gdext/master/godot/classes/object/struct.ExConnect.html) takes `u32`, despite [`ConnectFlags`](https://godot-rust.github.io/docs/gdext/master/godot/classes/object/struct.ConnectFlags.html) existing.
- [x] [`Signal::connect()`](https://godot-rust.github.io/docs/gdext/master/godot/builtin/struct.Signal.html#method.connect) takes `i64`.
- [x] https://github.com/godot-rust/gdext/issues/185

## Duplicated types in manual and generated code

We already handled some (`Side`, `Corner`, `Vector2Axis`, `Vector3Axis`), there are a few remaining:
- [ ] [`GDExtension.InitializationLevel`](https://godot-rust.github.io/docs/gdext/master/godot/classes/gdextension/struct.InitializationLevel.html) vs. [`InitLevel`](https://godot-rust.github.io/docs/gdext/master/godot/init/type.InitLevel.html)
- [ ] [`ObjectId`](https://godot-rust.github.io/docs/gdext/master/godot/classes/native/struct.ObjectId.html) vs [`InstanceId`](https://godot-rust.github.io/docs/gdext/master/godot/obj/struct.InstanceId.html)

They don't necessarily need to be de-duplicated, but we could at least provide conversions.

## Integer type proliferation

- [ ] `_count` methods (get/set) mostly use `i32`
- unclear if `usize` is an option, although it's currently unsupported. Might be better to stick to `i32`, as it's not really inconsistent, just a bit harder to integrate with Rust.
- What to do with all the other "index-like" APIs, see https://github.com/godot-rust/gdext/pull/982#discussion_r1893732978
- [x] `Array::subarray_{deep|shallow}` and `PackedArray::subarray` use `usize` but underlying Godot has `i32`, allowing wraparounds.
- `NodePath::subpath` follows the same schema.
- Considerations of making `usize` ranges, but allow utility to convert Godot negative-wrapped-range to convert into [`RangeBounds`](https://doc.rust-lang.org/std/ops/trait.RangeBounds.html)
- [ ] Signals use `i32`/`i64`
- [`GraphEdit::connect_node()`](https://godot-rust.github.io/docs/gdext/master/godot/classes/struct.GraphEdit.html#method.connect_node) uses `i32` ports
- `GraphEdit` signal `connection_request` takes `i64` ports

## Getters are unnecessarily mutable

- [x] #1274
- [ ] Many more; should be heuristic

## Unnecessarily weak typing

- [x] `Object::set_script`/`get_script` works on `Variant` rather than `Option>`
- [ ] `WeakRef` as well, although that class is pretty useless in GDExtension, not really worth fixing

Use cases for structs
- [ ] `Node::rpc_config`/`get_rpc_config` -- currently `Variant`
- [ ] Return values of [`PhysicsDirectSpaceState3D`](https://docs.godotengine.org/en/stable/classes/class_physicsdirectspacestate3d.html) and related APIs
- `get_rest_info` -- `Dictionary`
- `intersect_ray` -- `Dictionary`
- `intersect_point` -- `Array[Dictionary]`
- `intersect_shape` -- `Array[Dictionary]`

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.