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

Calling isActionPressed on an InputEvent instance crashes Godot #157

Closed
PadraigK opened this issue Oct 6, 2023 · 5 comments
Closed

Calling isActionPressed on an InputEvent instance crashes Godot #157

PadraigK opened this issue Oct 6, 2023 · 5 comments

Comments

@PadraigK
Copy link
Contributor

PadraigK commented Oct 6, 2023

Thread 0 Crashed::  Dispatch queue: com.apple.main-thread
0   Godot                         	       0x108e13be4 0x104cb8000 + 68533220
1   Godot                         	       0x108e13b78 0x104cb8000 + 68533112
2   Godot                         	       0x108a2c01c 0x104cb8000 + 64438300
3   Godot                         	       0x108a4f064 0x104cb8000 + 64581732
4   libSwiftGodot.dylib           	       0x1295a299c InputEvent.isActionPressed(action:allowEcho:exactMatch:) + 1184 (InputEvent.swift:100)
5   libSimpleRunnerDriver.dylib   	       0x1131be654 PlayerController._input(event:) + 140 (PlayerController.swift:27)
6   libSwiftGodot.dylib           	       0x129951be4 _Node_proxy_input(instance:args:retPtr:) + 1092 (Node.swift:2115)
7   libSwiftGodot.dylib           	       0x129951798 @objc _Node_proxy_input(instance:args:retPtr:) + 12
8   Godot                         	       0x106ecbc9c 0x104cb8000 + 35732636
9   Godot                         	       0x106ef8438 0x104cb8000 + 35914808
override func _input(event: InputEvent) {
        event.isActionPressed(action: "hi")
    }
@PadraigK
Copy link
Contributor Author

PadraigK commented Oct 6, 2023

Note that calling Input.isActionPressed(action: "hi") works fine as a work around

@tishin
Copy link
Contributor

tishin commented Oct 9, 2023

There's an issue with reference handling in generated Node.swift:

let resolved_0 = gi.ref_get_object (args [0]!.load (as: UnsafeRawPointer.self))!
swiftObject._input (event: lookupLiveObject (handleAddress: resolved_0) as? InputEvent ?? InputEvent (nativeHandle: resolved_0))

As far as I can see, it expects the argument to be a pointer to Ref<InputEvent>, but actually gets a pointer to InputEvent itself, so I managed to fix it by simplifying argument handling:

let resolved_0 = args [0]!.load (as: UnsafeRawPointer.self)
swiftObject._input (event: lookupLiveObject (handleAddress: resolved_0) as? InputEvent ?? InputEvent (nativeHandle: resolved_0))

For some reason all refcounted objects get this wrapper in Generator:

if cmap.isRefcounted {
    argPrep += "let resolved_\(i) = gi.ref_get_object (args [\(i)]!.load (as: UnsafeRawPointer.self))!\n"
} else {
    argPrep += "let resolved_\(i) = args [\(i)]!.load (as: UnsafeRawPointer.self)\n"
}

I'm not sure if it's a general bug, or InputEvent handler is some sort of edge case scenario here.

@alicerunsonfedora
Copy link
Contributor

This also appears to happen for touch events, too:

    public override func _input(event: InputEvent) {
        super._input(event: event)

        if event.isClass("InputEventScreenTouch") {
            LibAnthrobase.logger.debug("Received touch event: \(event)")
        }
    }

@migueldeicaza
Copy link
Owner

Sadly the commit history does not point out the reason for that change (be78878), and I do not recall the reason for that. It was too early on the binding development, and might have been an artifact of the way things worked in 4.0

I added a bad workaround as well when I was working on the editor in a similar area (651645e), and even filed a bug to get back to try to figure out the proper fix (#109).

I suspect the patch from @tishin is correct, and it should be much better than what we have now.

The Godot-CPP bindings use this ref_get_object, but I struggle to find out where it is being used, due to the convoluted usage pattern (include/godot_cpp/classes/ref.hpp

@migueldeicaza
Copy link
Owner

Ok, I have now verified that the C++ code does nothing with that binding code.

So I think that this can go in, and I will also revert the other incorrect workaround.

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

4 participants