Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GDExtension: Fix setting base class properties on a runtime class #94089

Merged

Conversation

dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Jul 8, 2024

Fixes #93676

This PR makes it so that placeholders for runtime classes from GDExtension will only try to store properties that are specifically from the runtime class (or any of its parent classes that are also runtime classes).

This fixes #93676 in my testing, but also fixes other issues that I hadn't noticed previously with setting properties on parent classes. For example, before this PR, if you make a runtime class in a GDExtension that extends Sprite, then set the texture property in the editor, it won't actually display the texture in the editor (but will when loading the saved scene in the running game).

Also, while working on this, I realized that we have a difference of behavior between runtime classes defined in modules and those defined in GDExtensions: in a GDExtension, if you make a runtime class that has a non-runtime parent class, it won't run any of the code from that parent class, but only use its native parent; whereas in a module, the parent is always native, and it would always run the parent's code. So, if someone used runtime classes that extended non-runtime classes from their GDExtension, and then moved those classes into a module, they would act differently.

I think it should be possible to make GDExtensions work the same as modules in this regard, and I started hacking on it, but it would be a big patch. So, instead, I've added a new limitation preventing runtime classes defined in GDExtensions from extending from non-runtime extension classes. This way, when we eventually make the changes to correct this behavior difference, we can also remove this limitation, preventing a future change in behavior.

@dsnopek dsnopek added this to the 4.3 milestone Jul 8, 2024
@dsnopek dsnopek requested a review from a team July 8, 2024 17:35
@dsnopek dsnopek requested a review from a team as a code owner July 8, 2024 17:35
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested and confirmed this fixes the Rust MRP.

@akien-mga akien-mga merged commit 8ed9bfd into godotengine:master Jul 8, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

@dsnopek dsnopek deleted the gdext-valid-runtime-properties branch July 22, 2024 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resources extended from GDExtension classes don't work right in 4.3
2 participants