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

Control virtual methods are not dispatched properly in a multi-tiered custom class hierarchy #1333

Open
Naros opened this issue Dec 13, 2023 · 2 comments

Comments

@Naros
Copy link
Contributor

Naros commented Dec 13, 2023

Godot version

4.2.0-stable

godot-cpp version

4.2.0-stable

System information

Windows 11, MSVC 2022

Issue description

When creating a custom implementation of GraphNode and adding custom logic inside _has_point and _get_tooltip, the behavior of these methods differs between DEBUG and RELEASE builds.

On debug builds, these methods are called whereas for RELEASE builds they are not.

It's important to note that the class hierarchy is GraphNode --> MyGraphNode --> MyGraphNodeComment. These virtual methods are implemented on the comment node only.

A workaround appears to be that to have consistent behavior across builds, the overridden methods must also be overridden in the MyGraphNode class with default implementations, i.e.:

    bool _has_point(const Vector2& p_point) const override { return GraphNode::_has_point(p_point); }
    String _get_tooltip(const Vector2& p_point) const override { return GraphNode::_get_tooltip(p_point); }

Steps to reproduce

  1. Create a custom class MyGraphNode that is derived from GraphNode.
  2. Create another custom class MyGraphNodeComment that derives from MyGraphNode.
  3. Implement _has_point and _get_tooltip with custom behavior inside MyGraphNodeComment, i.e.
bool MyGraphNodeComment::_has_point(const Vector2& p_point) const
{
    Ref<Texture2D> resizer = get_theme_icon("resizer");
    if (Rect2(get_size() - resizer->get_size(), resizer->get_size()).has_point(p_point))
        return true;

    if (Rect2(Point2(), Vector2(get_size().x, 28)).has_point(p_point))
        return true;

    return false;
}

String MyGraphNodeComment::_get_tooltip(const godot::Vector2& p_point) const
{
    if (Rect2(Point2(), Vector2(get_size().x, 28)).has_point(p_point))
        return get_tooltip_text();
    return "";
}
  1. Under debug builds, these methods are fired just fine but aren't fired under release builds.

Editor - 4.2 stable official
Godot CPP - Checkout hash 4.2.0-stable 0f78fc45bd9208793736afda6c56ff7e85d4d285

Minimal reproduction project

N/A

@dsnopek
Copy link
Contributor

dsnopek commented Dec 30, 2023

Thanks!

When you talk about debug vs release builds, are you referring to debug/release builds of Godot or debug/release builds of your GDExtension? Or, always paired (debug Godot build with debug GDExtension build, and vice versa for release)?

@dsnopek
Copy link
Contributor

dsnopek commented Dec 30, 2023

Unfortunately, I've been unable to reproduce this so far. I tried adding a test case which makes a sub-sub-class with _get_tooltip() which is in this patch.

I tried running the tests with Godot 4.2.1-stable with the editor (debug) running a debug build of the test GDExtension, the editor (debug) with a release build of the test GDExtension, and exporting the project (so, release template of Godot 4.2.1-stable) with a release build of the test GDExtension -- the tests passed in every combination.

Can you provide an MRP or more instructions on how to reproduce? Thanks!

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