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

Non-static methods can't be referenced as Callables from static methods #91403

Closed
KoBeWi opened this issue May 1, 2024 · 8 comments · Fixed by #91412
Closed

Non-static methods can't be referenced as Callables from static methods #91403

KoBeWi opened this issue May 1, 2024 · 8 comments · Fixed by #91412

Comments

@KoBeWi
Copy link
Member

KoBeWi commented May 1, 2024

Tested versions

4.3 dev6 broken
4.3 dev5 works

System information

Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1060 (NVIDIA; 31.0.15.4633) - Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz (8 Threads)

Issue description

extends Node

func _ready() -> void:
	static_method()

static func static_method():
	print(not_static)

func not_static():
	pass

Running this script in dev5 prints Node::not_static. Running in dev6 results in Invalid access to property or key 'not_static' on a base object of type 'Nil'.. The callable is not accessible even if you name the script and do ClassName.not_static.

Steps to reproduce

  1. Run the code above in 4.3 dev5
  2. Run the code above in 4.3 dev6

Minimal reproduction project (MRP)

See above.

@KoBeWi KoBeWi added this to the 4.3 milestone May 1, 2024
@akien-mga akien-mga moved this from Unassessed to Release Blocker in 4.x Release Blockers May 1, 2024
@akien-mga
Copy link
Member

CC @dalexeev @vnen

@dalexeev
Copy link
Member

dalexeev commented May 1, 2024

I think the old behavior is also incorrect, non-static members should not be accessible in a static context, there should be an analyzer error. If you try to call not_static() you will get the error Cannot call non-static function "not_static()" from static function "static_method()"..

@KoBeWi
Copy link
Member Author

KoBeWi commented May 1, 2024

I was using it to obtain method name, i.e. Engine.get_main_loop().call_group(GROUP_NAME, not_static.get_name()). The alternative is hard-coding &"non_static", which is less safe.

btw I forgot to mention that the new error is runtime-only. It doesn't appear in the editor.

@akien-mga
Copy link
Member

Looking at the changelog, I suppose this might have been caused by #90223 ?

@dalexeev
Copy link
Member

dalexeev commented May 1, 2024

Looking at the changelog, I suppose this might have been caused by #90223 ?

Yes, most likely. But I think this is a valid assumption for the compiler, we should not allow static access to non-static members at the analyzer level.

@vnen
Copy link
Member

vnen commented May 1, 2024

Yeah, the old behavior was wrong. For a Callable you need an object and a method name, but in a static constant there's no object.

Maybe we can consider adding a nameof or something for this case if it's needed (see godotengine/godot-proposals#837). But I don't think reverting to old behavior is a good idea.

Not showing the error in-editor is another issue and should be fixed too.

@KoBeWi
Copy link
Member Author

KoBeWi commented Oct 25, 2024

I didn't test after the fix, because I assumed it was working, but seems like instead of allowing it, the fix just made the error appear in the editor too. Was that intended?

@dalexeev
Copy link
Member

dalexeev commented Oct 28, 2024

@KoBeWi Yes, that's how it was intended. GDScript functions and Callables are not the same as function/method pointers. You can't access a non-static member of a class in a static context. #91412 only fixed an analyzer bug. The previous behavior was unintentional and was removed with the optimization for static methods in #90223.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Release Blocker
Development

Successfully merging a pull request may close this issue.

4 participants