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

Behavior change with handling _notification in class hierarchy #1498

Open
Naros opened this issue Jun 19, 2024 · 4 comments
Open

Behavior change with handling _notification in class hierarchy #1498

Naros opened this issue Jun 19, 2024 · 4 comments

Comments

@Naros
Copy link
Contributor

Naros commented Jun 19, 2024

Godot version

4.3-beta1

godot-cpp version

4.3 - sha 45be6d0

System information

Windows 11

Issue description

In Godot 4.2, with a GDExtension class hierarchy of Node3D > CustomNode3D > SuperCustomNode3D, if both of the custom classes implemented in GDExtension had a _notification method, the SuperCustomNode3D had to call the CustomNode3D implementation as it would not be dispatched by Godot automatically.

In Godot 4.3, this behavior has been fixed and now works as it should, where Godot handles the dispatch of the _notification functions regardless of how the hierarchy is constructed.

This should at the very least be documented in a migration guide as a behavior change.

Steps to reproduce

  1. Create a CustomNode3D that extends Node3D, implementing _notification.
  2. Create a SuperCustomNode3D that extends CustomNode3D, implementing _notification.

In Godot 4.2, the _notification in CustomNode3D was never called.
In Godot 4.3, both _notification functions are called.

Minimal reproduction project

N/A

@dsnopek
Copy link
Collaborator

dsnopek commented Jul 11, 2024

Perhaps we could document it on this page after it's been merged?

@Naros
Copy link
Contributor Author

Naros commented Jul 11, 2024

I discussed this with @YuriSizov on discord recently, and we concluded this came down to a fix already merged on Godot 4.2 around _notification changes (don't have the PR/commit handy atm), and in my use case I was still based on Godot 4.2.1 which was why I saw this difference.

Given that, it may be worth to at least call this out for users who may have been building on an older 4.2.x build and not necessarily the latest 4.2 branch of godot-cpp

@dsnopek
Copy link
Collaborator

dsnopek commented Jul 11, 2024

Ah, probably PR #1381

I think we probably need a general way to record behavior changes for godot-cpp. We're generally OK with making these sort of bug fixes and even cherry-picking them, because you have to update your godot-cpp for the change to happen. But since it is cherry-picked, we don't have an easy way to point to them, since it would affect the 4.1, 4.2 and master branches, but only from a certain commit. We only make godot-cpp releases that are synchronized with Godot releases, so we don't have a version number to refer to.

@Naros
Copy link
Contributor Author

Naros commented Jul 11, 2024

Agreed. It could be something small, such as a markdown file in the godot-cpp repository that tracks these changes, providing a quick, at-a-glance view of these bits. As a maintainer of several Red Hat projects, I know many users in our communities find knowing breaking changes are more important than bug fixes.

But I would say the next step should be much larger focused. For example, there have been a few instances going from 4.2 to 4.3 of the engine, new virtual methods were added and require GDExtension users to implement those contracts to avoid the engine throwing errors. This typically isn't well documented, and leads to plug-in author communities dealing with lots of user questions until a new build is pushed that implements a virtual method stub. I understand why this does not come up in the engine; these PRs typically implement these where needed by engine code but aren't necessarily flagged well in the change notes.

So yes, a good generalized way universally across the Godot ecosystem would be ideal long-term.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants