bitwes / Gut

Godot Unit Test. Unit testing tool for Godot Game Engine.
1.81k stars 99 forks source link

Doubles: gdnative type hint break stubs #649

Open Scrawach opened 1 month ago

Scrawach commented 1 month ago

Versions

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)


2. Run tests.
3. Test failed, because `typed_result` is not equal `42`.
bitwes commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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.