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

Make GraphEdit's cpp virtuals equal to gdscript. #78426

Conversation

Daylily-Zeleen
Copy link
Contributor

@Daylily-Zeleen Daylily-Zeleen commented Jun 19, 2023

Some GraphEdit's methods can be overrided in GDScript but can't be overrided in C++, I don't know it is Intentional or not, but it blocking the development of godot module.

@Daylily-Zeleen Daylily-Zeleen requested a review from a team as a code owner June 19, 2023 04:00
Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

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

I agree that exposing virtuals is important to allow extending via C++ modules. Did not look deeply technically.

Note we are in feature freeze.

@akien-mga
Copy link
Member

Needs rebase after #79308. CC @Geometror

@Daylily-Zeleen Daylily-Zeleen force-pushed the daylily-zeleen/graph_edit_virtual branch from 7987b15 to 8275806 Compare August 7, 2023 14:58
@Daylily-Zeleen
Copy link
Contributor Author

Needs rebase after #79308. CC @Geometror

Done.

@akien-mga akien-mga modified the milestones: 4.x, 3.x, 4.2 Aug 8, 2023
scene/gui/graph_edit.h Outdated Show resolved Hide resolved
scene/gui/graph_edit.h Outdated Show resolved Hide resolved
@Daylily-Zeleen Daylily-Zeleen force-pushed the daylily-zeleen/graph_edit_virtual branch from 8275806 to 2f0639e Compare August 8, 2023 16:22
@akien-mga akien-mga merged commit 3a0086e into godotengine:master Aug 9, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@Daylily-Zeleen Daylily-Zeleen deleted the daylily-zeleen/graph_edit_virtual branch August 24, 2023 07:48
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.

4 participants