Skip to content

Godot should pass count when freeing the property list of GDExtension classes  #9462

Closed
godotengine/godot
#91179
@lilizoey

Description

@lilizoey

Describe the project you are working on

gdext rust bindings for Godot.

Describe the problem or limitation you are having in your project

When registering the get_property_list callback you return a pointer to an array of GDExtensionPropertyInfo, as well as the count of how many items are in the array. Later Godot will then call a registered free_property_list with that same pointer with the expectation that you will free this array (or perform any other cleanup as needed).

However since we only get the pointer in free_property_list, it isn't very easy to determine the length of this array and so you need to find some way to encode the length given just a pointer to the array. Which makes it difficult to properly cleanup and free the list.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

By having godot pass both a pointer and the count to free_property_list it will make it very simple to turn this pointer into an array.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

This has already been implemented for script extensions in godotengine/godot#89055. I imagine the implementation would be very similar for GDExtension classes.

If this enhancement will not be used often, can it be worked around with a few lines of script?

Any solution we can come up with in gdext will require some amount of overhead, or will require users of the library to write unusual code using unsafe rust. We can for instance:

  1. Pass a null terminated list. This makes free_property_list always have linear time overhead.
  2. Store the length in a hashmap of array pointers to array lengths. This requires extra allocation.
  3. Require the user of the library to uphold the safety contract. This is highly discouraged in rust unless absolutely necessary, which this really shouldn't be.
  4. Make some kind of header struct which stores the length, and then stores the array afterwards. However rust does not have a way of safely doing this at the moment except in very specific cases.

Is there a reason why this should be core and not an add-on in the asset library?

This cannot be an add-on, as it changes how GDExtensions are registered with godot.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions