Not able to call C++ _process(), _ready() reliably

[TokisanGames]

Godot beta 16

@outobugi and I are building a GDExtension terrain plugin. I have a C++ class with _ready() and _process() that I want to run. However some cases work, some cases don't. I can't tell what the right path without testing every possibility because it's so inconsistent, and there's no clear way to have it work properly in all cases.

Legend:
T - @tool mode
Ed - Editor mode
Gm - Running in Game
C - C++
GD - GDScript
rd - _ready() in C++ or GDScript
pr - _process() in C++ or GDScript
Symbols: Blank = N/A | + = Yes | X = No and it's likely a problem | E = Error, and maybe a problem

Script	T	Ed_C_rd	Ed_C_pr	Ed_GD_rd	Ed_GD_pr	Gm_C_rd	Gm_C_pr	Gm_GD_rd	Gm_GD_pr
1. None		+	+			+	+
2. extend		+	X			+	X
3. extend	Y	+	X			+	X
4. funcs		+	+			X	X	+	+
5. funcs	Y	X	X	+	+	X	X	+	+
6. super		+	+			X	X	E	E
7. super	Y	X	X	+E	+E	X	X	E	E

1. No script attached to the node. C++ functions work fine.
2. Attached to the node is a script with only extends MyNode, my C++ plugin. Issue: C++ runs one function but not the other!!
3. w/ Tools mode. Same results.
4. Above plus simple _ready() and _process() functions that print a message. Issue: This calls the C++ functions in the editor and the GDScript functions in the game!!
5. w/ Tools. Issue: Different results from w/o tools mode. C++ won't run at all.
6. Above, plus adding super._ready(), super._process(delta). Issue: In game, the debugger broke in and complained that these functions do not exist.
7. w/ Tools. Issue: Only gdscript runs in editor, but dumps errors in the console about missing functions. In game won't run at all w/o debugger coming up.

Adding C++ Bindings
By binding _ready() and _process() in C++ w/ bind_method, we get console errors: ERROR: Method 'Terrain3D::_process()' already registered as non-virtual., and also for ready. This applies to all. Initially, it seems these tests are worthless, but that turns out to not be true.

Script	T	Ed_C_rd	Ed_C_pr	Ed_GD_rd	Ed_GD_pr	Gm_C_rd	Gm_C_pr	Gm_GD_rd	Gm_GD_pr
1. None		X	X			X	X
2. extend		X	X			X	X
3. extend	Y	X	X			X	X
4. funcs		X	X			X	X	+	+
5. funcs	Y	X	X	+	+	X	X	+	+
6. super		X	X			+	+	+	+
7. super	Y	+	+	+	+	+	+	+	+

1-6. C++ doesn't work at all, and notably does not work if a script is not attached!
7. Here we see 7 works across the board.

Summary

In summary, if we want a plugin that runs C++ _ready() and _process() we either have to:

1. Compile w/o bindings, AND require that the user never attaches a script to the Terrain node, or it will break.
2. Compile w/ bindings, AND require the user to ignore the errors, AND always have a script attached to the Terrain node that implements _ready() and _process() AND calls the super functions.

Neither of these are good options as it stands.

Presumably there are similar inconsistencies with _enter_tree() and others.

Is there a better way to bind _ready() and _process? I'm using this:

	ClassDB::bind_method(D_METHOD("_ready"), &Terrain3D::_ready);
	ClassDB::bind_method(D_METHOD("_process", "delta"), &Terrain3D::_process);

Similar to #562, but for Godot 4

[TokisanGames]

@ozzr Suggested I test _notification to capture NOTIFICATION_READY and NOTIFICATION_PROCESS. This works. I can run proxy functions off of these in C++ and skip _ready/_process altogether. They work reliably whether the node has a script attached or not, tool mode, editor, game mode, and we don't need to call super, or require the user to follow weird rules. It's a sufficient workaround for the bugs above for now. set_process is required, otherwise only _ready is called.

void Terrain3D::_notification(int p_what) {
	switch (p_what) {
		case NOTIFICATION_READY: 
			ready();
			set_process(true);
			break;

		case NOTIFICATION_PROCESS: 
			process(get_process_delta_time());
			break;		
	}
}

[VanessB]

It seems to me that this and issue#72034 are the same issues.

[hakuhan]

I have meet simular problem when I call a C++ function at gdscript _ready/_process func. And that C++ function has a parameter which type is not Variant (Object* or Ref). Crash will happen.

[Klaim]

Has this issue been fixed in 4.0 and 4.1 (beta 2 right now)?

[gg-yb]

Has this issue been fixed in 4.0 and 4.1 (beta 2 right now)?

I can still reproduce the issue in 4.1.1

[anrp]

I think that godotengine/godot#83583 may also resolve this too if you'd be able to test, as I suspect its the same issue (I ran in to it in the form of "when I have a node from GDExtension that has _process method, the C++ one is not called at all after I attach GDScript", which that linked pr resolved)

[anrp]

[Copying misplaced comment from https://github.com/godot-rust/gdext/issues/111 ...]
FYI I did dig a bit because I was curious, and I'm quite sure this is not supposed to work, however....

If you apply the following patch:

diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp
index 2439e5760c..bc303d9cc9 100644
--- a/modules/gdscript/gdscript_analyzer.cpp
+++ b/modules/gdscript/gdscript_analyzer.cpp
@@ -1743,7 +1743,7 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
                        }
 #ifdef DEBUG_ENABLED
                        if (native_base != StringName()) {
-                               parser->push_warning(p_function, GDScriptWarning::NATIVE_METHOD_OVERRIDE, function_name, native_base);
+                               // parser->push_warning(p_function, GDScriptWarning::NATIVE_METHOD_OVERRIDE, function_name, native_base);
                        }
 #endif
                }
@@ -3299,7 +3299,7 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
                // If the method is implemented in the class hierarchy, the virtual flag will not be set for that MethodInfo and the search stops there.
                // Virtual check only possible for super() calls because class hierarchy is known. Node/Objects may have scripts attached we don't know of at compile-time.
                if (p_call->is_super && method_flags.has_flag(METHOD_FLAG_VIRTUAL)) {
-                       push_error(vformat(R"*(Cannot call the parent class' virtual function "%s()" because it hasn't been defined.)*", p_call->function_name), p_call);
+                       // push_error(vformat(R"*(Cannot call the parent class' virtual function "%s()" because it hasn't been defined.)*", p_call->function_name), p_call);
                }

                // If the function requires typed arrays we must make literals be typed.

(which just prevents it from stopping processing on errors - a serious hack)

and you register _process again...

godot::ClassDB::bind_method(godot::D_METHOD("_process"), &GDExample::_process);

which causes the engine to complain

ERROR: Method 'GDExample::_process()' already registered as non-virtual.
   at: bind_virtual_method (external/godot-cpp/src/core/class_db.cpp:334)

and then you override in GDScript...

# Called every frame. 'delta' is the elapsed time since the previous frame.
func _process(delta):
  print(\"godot process\")
  super(delta)

then it works as you have expected: engine calls script which calls the native class function.

[anrp]

I'm beginning to think that this might be a documentation kind of error... it seems like what godot considers to be a virtual function is not meant to be implemented in the class declaring that it is virtual means that it is not supposed to be callable if it is overridden.

The error from the engine complaining (above^) is actually coming from the built-in/generated node.hpp register_virtuals() function (where it tries to bind _process as virtual if the implementation of the most-derived class is not equivalent to the (dummy) implementation in Node.) By binding it myself in _bind_methods, I prevent godot-cpp from binding it as virtual (I didn't realize but those error messages were also causing early abort (return) in the ClassDB functions).

Falling out of that, it seems like you're not supposed to be able to call parent class virtual functions; probably why it couldn't be "found" by godot as its expectation is that if it is "virtual" it doesn't exist in that class, i.e. "hasn't been defined" is more like "is not supposed to be defined" (hence why it worked when I disabled that error and did not let it get registered as virtual)

[AwesomeJuniorMiss]

Has this issue been fixed in 4.0 and 4.1 (beta 2 right now)?

No.

[dsnopek]

Some part(s) of this issue may be fixed by PR godotengine/godot#83583 which will be included in Godot 4.2