godot-rust / gdext

Rust bindings for Godot 4
https://mastodon.gamedev.place/@GodotRust
Mozilla Public License 2.0
2.79k stars 167 forks source link

SceneTree::call_group_flags expects i64 as flag, not GroupCallFlags #185

Open kzerot opened 1 year ago

kzerot commented 1 year ago

SceneTree's method call_group_flags expects i64 as a flag, not the GroupCallFlags struct. Also, "ord" field is i32, not i64. It's working, but syntax is not very obvious:

tree.call_group_flags(GroupCallFlags::GROUP_CALL_DEFERRED.ord().into() ,"group".into(), "method".into(), &[]) 
Bromeon commented 7 months ago

The problem is that these bitfield types are sometimes exposed as enums in the GDExtension JSON, and their parameter/return types use int.

Examples where the bitfield type is used (search bitfield:: in JSON):

There are also interesting cases that look like bitfields, but are not:

There may also be situations where a type is correctly marked as bitfield, but still used as int in APIs. Please comment if you know such cases.


I don't know how to easily detect that an enum is a bitfield, or that an int parameter/return type represents a bitfield type. There are of course some options:

  1. Manually annotating known cases.
  2. Heuristic based on method and parameter names.
    • This is going to be tough, given the above inconsistencies. Maybe also taking into account "meta": "int32" or so...
  3. Combine (1) and (2): heuristic with exceptions/additions.
  4. Implement a solution upstream, that is, fix the types in Godot's API. Might be problematic with backwards compatibility.
Lemiczek commented 7 months ago

To be honest, this does feel like an upstream issue that we can do very little about, without facing increasing headaches.

For the ConnectFlags example specifically, this was already called out upstream here: https://github.com/godotengine/godot/issues/74829, which seems to have hit a bit of a standstill, with no further comment on breaking compatibility by making it a Bitfield<ConnectFlags>. Maybe we come back to this if/once this is fixed for Godot 5.x? Since this is a breaking change for GDExtensions, and thus more suited for major releases?

If we were to manually annotate, I think we'd have to mark it behind a compatibility flag of sorts, since the extension API can change, and cause issues for these specific edge-cases way down the line. 🙁

We could maybe do the heuristics approach by looking at each value and seeing if it's a power of two? (There also weird edge-cases to this however, since certain flags can have duplicate values or have the value 0)

Actually we can't sinceArrayFormat (for example) despite being set as a flag/bitfield has the values 3, 7, 13, 19, etc...

I don't think we can even look for enums that end with Flags, since there are legitimate enums with that name, that go sequentially (0,1,2,3).

I really don't see a good way to solve this from our side.

Also, why are these things so inconsistent in the first place?! 😭