godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.24k stars 21.22k forks source link

Adding extra args in the editor "connect a signal to a method" popup when the method is located on a superclass fails #89787

Open albinaask opened 8 months ago

albinaask commented 8 months ago

Tested versions

4.2.1 Stable

System information

Godot v4.2.1.stable - Windows 10.0.22631 - Vulkan (Forward+) - integrated Intel(R) UHD Graphics 620 (Intel Corporation; 30.0.101.1338) - Intel(R) Core(TM) i5-8265U CPU @ 1.60GHz (8 Threads)

Issue description

Just like adding an extra value to a script that is put on a node works, instead directing it onto a method that is in a superclass that is written in GDScript should work.

bild

The picture above shows the setup:

Steps to reproduce

  1. Open MRP
  2. Open Scene
  3. Open the 'connect signal' dialog through right clicking the signal 'text_changed' under Node in the inspector
  4. Pick the superclass method through the 'pick' button.
  5. Add an extra bool parameter
  6. Press 'Connect'
  7. Observe error

Minimal reproduction project (MRP)

error project.zip

azuloo commented 8 months ago

So when you press 'Connect' the following chain of events is happening:

  1. Object::connect() checks if the supplied callable is valid.
  2. The object checks if it has this method (line_is_edited()). In this case the check is against Object::script_instance.
  3. Because you're calling it from the editor and it is not considered the runtime - Object::script_instance is instantiated with PlaceHolderScriptInstance class instance.
  4. PlaceHolderScriptInstance::has_method() calls has_method() on attached script (GDScript in this case).
  5. GDScript::has_method() only checks if it has the supplied method in its own method list and doesn't check its parent methods. And this causes the error.

Marking a script with @tools helps to execute it in editor and thus - fixes the error. But the bound method is not being called. Whether it is intended - requires further investigation.