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

Improve certain error messages #407

Open
Bromeon opened this issue Sep 11, 2023 · 10 comments
Open

Improve certain error messages #407

Bromeon opened this issue Sep 11, 2023 · 10 comments
Labels
quality-of-life No new functionality, but improves ergonomics/internals

Comments

@Bromeon
Copy link
Member

Bromeon commented Sep 11, 2023

Users gave feedback that certain error messages are unhelpful. This thread collects concrete examples.

While some of them are due to the nature of how we interact with Godot and cannot always be improved, in other cases there might be low-hanging fruits.

@Bromeon Bromeon added the quality-of-life No new functionality, but improves ergonomics/internals label Sep 11, 2023
@lilizoey
Copy link
Member

removing the blanket impl of To/FromVariant and VariantMetadata for EngineEnum would mean that if one of those trait impls are missing then people will get a more clear error message.

@lilizoey
Copy link
Member

after #467 a lot of conversion errors are nicer, but some still could be improved. for instance

when trying to pass an Array to a function expecting Array<f32/f64> we give the error:

Panic msg:
  in method `update` at parameter [0] of type godot_core::builtin::array_inner::Array<f32>: expected array of type Float, got array of type Nil

this could be improved in this case to say how you can fix it, since the fix is usually to change the variable's type to Array[float]

@jrockett6
Copy link
Contributor

Not a huge deal, but when a #[func] called from godot panics, Godot throws Method not found, along with the rust panic messages. Once you learn that this is the case it's easy to ignore the Method not found error. But it can be troublesome for new users thinking that they are doing something wrong when e.g. connecting signals to rust functions.

ERROR: Error calling from signal 'pressed' to callable: 'MyNode::on_pressed': Method not found.
   at: emit_signalp (core/object/object.cpp:1140)

@Bromeon
Copy link
Member Author

Bromeon commented Jan 5, 2024

@jrockett6 related: #254 and to some extent #422.

@fpdotmonkey
Copy link
Contributor

In cases where the godot versions between gdext and godot bin are incompatible, it'd be good to read out the version numbers explicitly (for example, if you select the wrong version branch in gdext)

@noah427
Copy link

noah427 commented Jan 6, 2024

Edit bromeon: context in Discord thread.

Registering a class twice will result in an error like this.

/home/noah/.cargo/git/checkouts/gdext-76630c89719e160c/0b93fe1/godot-core/src/lib.rs:161 - Rust function panicked in file /home/noah/.cargo/git/checkouts/gdext-76630c89719e160c/0b93fe1/godot-core/src/registry.rs at line 384. Context: failed to initialize GDExtension level `Scene` /home/noah/.cargo/git/checkouts/gdext-76630c89719e160c/0b93fe1/godot-core/src/lib.rs:109 - Panic msg: called `Result::unwrap()` on an `Err` value: () /home/noah/.cargo/git/checkouts/gdext-76630c89719e160c/0b93fe1/godot-core/src/lib.rs:161 - Rust function panicked in file /home/noah/.cargo/git/checkouts/gdext-76630c89719e160c/0b93fe1/godot-core/src/lib.rs at line 65. Context: failed to initialize GDExtension level `Editor` /home/noah/.cargo/git/checkouts/gdext-76630c89719e160c/0b93fe1/godot-core/src/lib.rs:109 - Panic msg: called `Result::unwrap()` on an `Err` value: PoisonError { .. }

This error doesn't tell us the actual issue. It should probably say something like "Class x registered more than once".

@fpdotmonkey
Copy link
Contributor

When changing the base class of an object and hot reloading, gdext spews a bunch of errors, but the first one that godot itself provides is all that's required for the developer to understand what went wrong and how to fix it.

ERROR: GDExtension class 'Junction' attempt to change parent type from 'Node2D' to 'Sprite2D' on hot reload. Restart Godot for this change to take effect.
   at: _register_extension_class_internal (core/extension/gdextension.cpp:408)
ERROR: Rust function panicked in file /home/fp/.cargo/git/checkouts/gdext-76630c89719e160c/e3644a0/godot-core/src/registry.rs at line 453. Context: failed to initialize GDExtension level `Scene`
   at: <function unset> (/home/fp/.cargo/git/checkouts/gdext-76630c89719e160c/e3644a0/godot-core/src/lib.rs:161)
ERROR: Panic msg:  failed to register class `Junction`; check preceding Godot stderr messages
   at: <function unset> (/home/fp/.cargo/git/checkouts/gdext-76630c89719e160c/e3644a0/godot-core/src/lib.rs:109)
ERROR: Rust function panicked in file /home/fp/.cargo/git/checkouts/gdext-76630c89719e160c/e3644a0/godot-core/src/registry.rs at line 307. Context: failed to initialize GDExtension level `Editor`
   at: <function unset> (/home/fp/.cargo/git/checkouts/gdext-76630c89719e160c/e3644a0/godot-core/src/lib.rs:161)
ERROR: Panic msg:  global lock for loaded classes poisoned; class registration or deregistration may have panicked
   at: <function unset> (/home/fp/.cargo/git/checkouts/gdext-76630c89719e160c/e3644a0/godot-core/src/lib.rs:109)

@Bromeon
Copy link
Member Author

Bromeon commented Jan 8, 2024

This message (posted in the wrong issue):

Notes for some stuff you asked me to post here from the discord:

  • Names of callables don't actually show up in errors (just says '')

  • Callable argument error will say you're converting to type "" (empty string, no quotes in the output) instead of an actual type name.
    image

@Lamby777
Copy link
Contributor

Lamby777 commented Jan 8, 2024

This message (posted in the wrong issue):

Notes for some stuff you asked me to post here from the discord:

  • Names of callables don't actually show up in errors (just says '')
  • Callable argument error will say you're converting to type "" (empty string, no quotes in the output) instead of an actual type name.
    image

The image didn't get posted... or maybe it's just gone now after I deleted the original message. Here it is, for context.
image

@QueenOfSquiggles
Copy link

An error I spent way too long debugging is this one:

ERROR: Rust function panicked in file /home/squiggles/.cargo/git/checkouts/gdext-76630c89719e160c/8d93420/godot-core/src/engine/mod.rs at line 181. Context: load_track_file
   at: <function unset> (/home/squiggles/.cargo/git/checkouts/gdext-76630c89719e160c/8d93420/godot-core/src/lib.rs:161)
ERROR: Panic msg:  clone: access to instance with ID 203004315173 after it has been freed
   at: <function unset> (/home/squiggles/.cargo/git/checkouts/gdext-76630c89719e160c/8d93420/godot-core/src/lib.rs:109)

GDscript entry point

CoreDialog.load_track_file(file_path)

The actual code I wrote that was triggering this error was:

if let Some(gui) = &mut self.gui.clone() {
    if gui.is_instance_valid() {
        gui.queue_free();
    }
}

Specifically I believe it was on the call to clone() given the error. If I had been given a stack trace of sorts (or whatever system rust uses to track down the call stack?) then I very likely could have solved it in minutes.

On first pass, gui was None because this is part of gui's initialization function. But on the second pass it crashes with the error above.

Even something as simple as declaring the name of the object that is being cloned would be nice (though I imagine that wouldn't be accessible at this point, just sharing things that would be helpful)

Also of note: my problem is solved, no need for suggested solutions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quality-of-life No new functionality, but improves ergonomics/internals
Projects
None yet
Development

No branches or pull requests

7 participants