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

Doubles: gdnative type hint break stubs #649

Open
Scrawach opened this issue Jul 27, 2024 · 3 comments
Open

Doubles: gdnative type hint break stubs #649

Scrawach opened this issue Jul 27, 2024 · 3 comments

Comments

@Scrawach
Copy link

Scrawach commented Jul 27, 2024

Versions

  • Godot: 4.2.2
  • GUT: 9.3.0
  • OS: Linux

The Bug

Using gdscript type hints for gdnative object breaks stubs for doubles.

Honestly, I have no idea why. Probably some engine optimizations. Everything works fine for custom types, problems only with gdnative types. During debugging, with a typed instance, there is no further work with the GUT framework - the value is returned immediately (without steps inside GUT or stubs or something else). So I think it's an engine issue.

This may be a problem similar to #490 and #482, but I'm not sure.

Steps To Reproduce

  1. Create test:
extends GutTest

func test_doubles_native_types_hint() -> void:
	var doubled_peer = double(StreamPeerTCP).new()
	stub(doubled_peer.connect_to_host).to_return(42)
	
	var untyped_result: int = untyped_invoke(doubled_peer)
	var typed_result: int = typed_invoke(doubled_peer)
	
	assert_eq(untyped_result, 42)
	assert_eq(typed_result, 42)

func untyped_invoke(peer) -> int:
	return peer.connect_to_host("", 1)

func typed_invoke(peer: StreamPeerTCP) -> int:
	return peer.connect_to_host("", 1)
  1. Run tests.
  2. Test failed, because typed_result is not equal 42.
@bitwes
Copy link
Owner

bitwes commented Jul 27, 2024

I believe this is related to #633. I'm pretty sure, that since Godot knows what peer is in typed_invoke, the engine optimizes things and calls all methods on it directly, bypassing the overrides created in the double.

The documentation should be updated to explicitly list doubling natives (which always use the double strategy INCLUDE_NATIVE). A couple examples of what you shouldn't do might be useful too.

@Scrawach
Copy link
Author

Yes, I agree, #633 is related to this.

It would be great to cover this problem in the documentation. Right now it seems that the only workaround to this problem is to use custom type wrappers. In the example,

class_name MyStreamPeerTCP

var peer: StreamPeerTCP

func _init() -> void:
	self.peer = StreamPeerTCP.new()

func connect_to_host(host: String, ip: int) -> int:
	return peer.connect_to_host(host, ip)

Or don't use type hints in the project...

@bitwes
Copy link
Owner

bitwes commented Jul 28, 2024

I'm not sure when Godot started punching through to underlying implementation on typed variables but it feels like a recent change based on related issues being opened in GUT. Maybe it's just that more people are doing it. As Godot gets better at doing this kind of optimization, doubling native objects and methods is not working as expected more often. A new approach might be needed.

Instead of fixing doubles of native things, maybe we could remove typed variables and parameters in a special kind of double. When creating a double GUT dynamically generates a script that inherits from the object being doubled and generates wrappers for all the methods. The wrapper methods do not have typed parameters, but all the class variables retain their typing. The super methods still have their typed parameters though, so you eventually get to a spot where things are typed.

If the doubler completely rewrote the source, removing typed variables/parameters then you could create a double that avoids the punch through. There could also a be a ton of issues doing this that I'm just not thinking of right now.

Maybe the Godot devs would be open to some way of forcing a Native object to use local versions of methods. This seems highly unlikely, but it would solve the problem. Maybe an annotation that didn't work with release builds. Maybe a lot of things.

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