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

[4.1-v0.0.17] Signal arguments makes it crash #175

Closed
why-try313 opened this issue Oct 11, 2023 · 5 comments · Fixed by #205
Closed

[4.1-v0.0.17] Signal arguments makes it crash #175

why-try313 opened this issue Oct 11, 2023 · 5 comments · Fixed by #205
Labels
bug Something isn't working

Comments

@why-try313
Copy link
Contributor

why-try313 commented Oct 11, 2023

Concerned version

4.1-v0.0.17-alpha-20231003

Problem

It seems that any function passing arguments makes the application crash, both on editor connection and using the .connect(str, fn) method

What works:

  • Connect a signal via editor as long the signal doesn't return arguments (0 args)
  • Connect a signal via code as long the signal doesn't return arguments (0 args)
  • Connect a signal via code and pass an anonymous function to trigger the method with an argument
    this.connect("sig_name", () => { this.myMethod(myValueHere) })

What doesn't work:

  • Connect a signal via editor that passes at least one argument (full freeze)
  • Connect a signal via code that returns arguments (full freeze)
    this.connect("input_event", this.myMethod) // returns 5 arguments
  • Connect a signal via code that passes at least one argument as third parameter (argument ignored)
    this.connect("sig_name", this.myMethod, [myValue])
  • Connect a signal via code that passes at least one argument with bind (argument ignored)
    this.connect("sig_name", this.myMethod.bind(myValue))

Observations

  1. It seems that after re-establishing binding_script.js with some modifications the connect method in code passes the arguments array
// From
return godot_object_connect.apply(this, [signal, target, method_name, params, flags]);
// To
return godot_object_connect.apply(this, [signal, target[method_name], params, flags]);
/// Apply this to all Object.defineProperty return statements
  1. Connecting a method on the editor doesn't match the number of arguments since MethodInfo doesn't set the number of arguments in /quickjs/quickjs_binder.cpp:L1871
    connect issued

Note : The parameters seem to be received by the functions since they are logged on the terminal that runs godot but freezes the program completly after printing them

@why-try313 why-try313 changed the title [4.1-v0.0.17] Signal arguments makes it crash crash [4.1-v0.0.17] Signal arguments makes it crash Oct 11, 2023
@fire
Copy link
Collaborator

fire commented Oct 11, 2023

Maybe we are not passing the arguments correctly with 0 count.

@why-try313
Copy link
Contributor Author

why-try313 commented Oct 11, 2023

Arguments are returned, but it looks like the signal callback fails.

Another problem that might be Signal related, TextEdit completly ignores keyboard inputs or clicks

After further testing: It turns out the crash only occurs on one of these nodes input_event(camera, event, position, normal, shape_idx),

that being said, child_entered_tree(Node) still doesn't return the node

To illustrate, xkill was made after freezing:
xkill

@fire
Copy link
Collaborator

fire commented Oct 11, 2023

I wonder if it's Camera3D access that's crashing? The rest are primitive types.

@nmerget
Copy link
Collaborator

nmerget commented Oct 15, 2023

Another problem that might be Signal related, TextEdit completly ignores keyboard inputs or clicks

If you add a script on a TextEdit it stops working...
If you place the script on a Control above and trigger the signal from TextEdit to the Control it isn't ignoring the clicks

--> This is a bug, we should check why we lose any input events if we attach a .mjs :(

@nmerget nmerget added the bug Something isn't working label Sep 15, 2024
@nmerget nmerget linked a pull request Sep 17, 2024 that will close this issue
1 task
@nmerget nmerget removed a link to a pull request Sep 17, 2024
1 task
@nmerget
Copy link
Collaborator

nmerget commented Sep 20, 2024

TextEdit

Moved this to #206

@nmerget nmerget linked a pull request Sep 20, 2024 that will close this issue
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants