MatejSloboda / Dijkstra_map_for_Godot

MIT License
77 stars 13 forks source link

Editor crash on Godot 3.5.2 #123

Open goblinJoel opened 1 year ago

goblinJoel commented 1 year ago

On Godot 3.5.2, I get this error. I was running via the Godot_v3.5.2-stable_win64_console.cmd version to try to see crash logs, since I've noticed the editor being unstable and not logging anything when started normally.

I can't tell what version of the addon I'm on, but I think 1.3. I installed it late October.

thread '' panicked at 'dijkstra_map_gd::Interface has already been registered as Interface', /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/gdnative-core-0.11.0/src/init/init_handle.rs:79:13 note: run with RUST_BACKTRACE=1 environment variable to display a backtrace

I've found at least two cases where this happened:

  1. On normal project load from Godot's startup menu
  2. Project > Reload Current Project

This is intermittent.

I'll add more if I see more editor crashes that mention rust or Dijkstra map. Usually when the editor crashes for me in general, it's either during or seconds after project load.

astrale-sharp commented 1 year ago

Hmm that's strange, thanks for reporting, do you think you could give me the output of tree . at the root of your directory? I wonder what's trying to register the class a second time.

I assume you have only one version of the addon at the time? (not two?)

goblinJoel commented 1 year ago

Some notes about the tree and project: This has been happening for awhile, since before I added Dialogic, although it feels like it's gotten worse recently. (Yesterday, it took 3 or 4 tries opening the project before it loaded without crashing. Usually it's more like 1/3 of the time.) Possible clues:

astrale-sharp commented 1 year ago

Maybe I have it! we built for 3.5.1, if you use 3.5.2 you may have to build yourself! https://github.com/MatejSloboda/Dijkstra_map_for_Godot#method-2-from-github

goblinJoel commented 1 year ago

Interesting. I could try that next time I'm working on it, although I may need more guidance on how to install the GDNative dependencies (step 3) and where to run the cargo command from in step 4 (the cloned repo's root?). The instructions do assume you know how to work with the Rust ecosystem a bit, but I'm using this for its functionality rather than the language it's made with. (I've never really worked with Rust outside of past troubleshooting for this addon.)

For what it's worth, the addon functions fine while the game is running.

astrale-sharp commented 1 year ago

I found the culprit in https://github.com/godot-rust/gdnative README

Compatibility list:

Godot 3.5.1 (works with gdnative 0.11)
Godot 3.4 (works with gdnative 0.10, custom build for 0.11)
Godot 3.3 (custom build)
Godot 3.2 (custom build)

building yourself will not solve the issue (i would have provided guidance tho, don't worry)

Although this might be an working option https://godot-rust.github.io/book/gdnative/advanced/custom-godot.html and i can guide for it as well.

otherwise maybe getting 3.5.1 if it's acceptable for you !

goblinJoel commented 1 year ago

I can see if i can reproduce the issue in 3.5.1. The crashes have been going on long enough, I'm skeptical that's the issue. But I only checked the log recently and saw the "dijkstra_map_gd::Interface has already been registered as Interface" error, so it's possible older crashes were unrelated. Does it make sense that building for a different maintenance version of Godot could cause this?

Thanks for your assistance, by the way!

astrale-sharp commented 1 year ago

To me it doesn't make much sense, I would expect errors of crashing because wrong types or number of argument were passed (cause that's mostly whats changing) But maybe building with a more recent gdnative version will help either way (bug fix upstream), I would totally try it!

Your welcome! I love to help ^^

astrale-sharp commented 1 year ago

What's your OS btw?

goblinJoel commented 1 year ago

I'm on Windows 10

goblinJoel commented 1 year ago

I had to try several times, but I managed to reproduce it on 3.5.1. This was on opening the project:

Godot Engine v3.5.1.stable.official.6fed1ffa3 - https://godotengine.org OpenGL ES 3.0 Renderer: NVIDIA GeForce RTX 2060/PCIe/SSE2 Async. shader compilation: OFF

Editing project: C:/Users/redacted/Documents/Godot/Tegwyn Tactics (C:::Users::redacted::Documents::Godot::Tegwyn Tactics)

Godot Engine v3.5.1.stable.official.6fed1ffa3 - https://godotengine.org OpenGL ES 3.0 Renderer: NVIDIA GeForce RTX 2060/PCIe/SSE2 Async. shader compilation: OFF

thread '' panicked at 'dijkstra_map_gd::Interface has already been registered as Interface', /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/gdnative-core-0.11.0/src/init/init_handle.rs:79:13 note: run with RUST_BACKTRACE=1 environment variable to display a backtrace

So it's not just on 3.5.2, although it might crash more on the newer version -- it's hard to tell, since it's pretty random.

astrale-sharp commented 1 year ago

I'm very unsure what's happening :/

goblinJoel commented 1 year ago

I'm running an experiment now on 3.5.2 where I've changed the Run > Main Scene to a random test scene that doesn't reference anything, by script or by instance, that references the DijkstraMap addon. That means that when I load the project from the splash menu, it won't be loading a game level scene that references the addon like it had been. If I can't get it to crash this way, maybe it has something to do with opening the project to a scene that uses the plugin. Perhaps there's a race condition involving load order or static type class names that causes it to sometimes register twice.

Edit: Godot 3 regularly spits out errors about an "type (Foo) not matching argument type (Foo)", or something like that, because it gets confused on static typing and thinks a class script is busted. (But then it works fine when I run the game.) So while I'm not seeing that error when I get the rust crashes (because the rust crash happens early), basically, I don't trust it to be loading a scene with a bunch of scripts that reference each other's types without doing something weird.

Update: Well, there goes that workaround. I left it open for awhile and clicked back into the Godot window to try another reload, and it immediately crashed with the same error. This one wasn't even on project load. I guess the next experiment will be to remove the static typing references to DijkstraMap and see if it somehow stops crashing. So weird.

goblinJoel commented 1 year ago

https://github.com/godot-rust/gdnative/blob/3731a64f441fcd87ac5010f296fd9fc03763c490/gdnative-core/src/init/init_handle.rs#L79 appears to be the line in 0.11.0

It looks like the latest, 0.11.3, changes that function quite a bit: https://github.com/godot-rust/gdnative/compare/0.11.0...0.11.3#diff-862a5680ed5c09514682c79a5aa2cfda611f6cbbf7a28f47d0cf5ec695b3584cR122

Notably, the panic line has been replaced with what looks to me as a non-rust-user like a more permissive check. Before:


        if let Some(class_info) = class_registry::register_class_as::<C>(name) {
            panic!(
                "`{type_name}` has already been registered as `{old_name}`",
                type_name = std::any::type_name::<C>(),
                old_name = class_info.name,
            );
        }

After:

        match class_registry::register_class_as::<C>(name, self.init_level) {
            Ok(true) => {}
            Ok(false) => return,
            Err(e) => {
                godot_error!("gdnative-core: ignoring new registration: {e}");
                return;
            }
        };

So your suggestion of building it myself might actually do the trick after all! If you can walk me through installing the GDNative dependencies (step 3) and where to run the cargo command from in step 4, then I'll give that a try. If it works, maybe you'll be able to update the prebuilt binaries for 3.5.1 (or maybe even 3.5.2) the same way.

astrale-sharp commented 1 year ago

Yea, maybe godot tried to register more agressively and gdnative allowed it, anyway ~~~ For 3.5.2, you should replace this line : https://github.com/SimonasLetukas/Dijkstra_map_for_Godot/blob/e7c4a0b12630f1b36ac97a85a0a6baf7b7b4213f/dijkstra-map-gd/Cargo.toml#L12 with gdnative = { git = "https://github.com/godot-rust/godot-rust.git", features = ["custom-godot"] } with godot in your PATH. then, in dijkstra-map-gd, simply run cargo build and see if it works.

if it does build in release and proceed with the guide ;)

was this detailed enough?

goblinJoel commented 1 year ago

I still can't get over how much space Visual Studio and Clang tools take, which is what stopped me from building myself back in October. (It does not allow you to change the install location for everything, so several gigs still end up in my tiny, almost full system drive!) Googling doesn't really find any other ways of installing Clang either, so I have it (slowly) installing now. It turns out I already had rust from last time, and I found how to add tag 0.11.3 to the cargo file line you mentioned, so hopefully it will succeed at building once this finishes installing.

goblinJoel commented 1 year ago

It worked! (So far.) Thanks for the guidance! The addon still seems functional, and I haven't seen crashes on load yet, although I've only tried loading/reloading the project a few times. The new dll is only 0.5mb for some reason instead of 2mb. For reference for the next person new to rust who has to do this on Windows 10:

With all that, going to the root of the cloned repo and running cargo build --release worked and put the new dll in the folder as described in [the instructions](cargo build --release).

astrale-sharp commented 1 year ago

Amazing! :D we should update the binaries then , I'll close this once we do

goblinJoel commented 1 year ago

Before you did that, I wanted to try a few more times and see if the error happened again. I got lucky the first try today with what I think is the expected error from the updated gdnative code:

Godot Engine v3.5.2.stable.official.170ba337a - https://godotengine.org
OpenGL ES 3.0 Renderer: NVIDIA GeForce RTX 2060/PCIe/SSE2
Async. shader compilation: OFF

Editing project: C:/Users/Joel/Documents/Godot/Tegwyn Tactics (C:::Users::Joel::Documents::Godot::Tegwyn Tactics)

Godot Engine v3.5.2.stable.official.170ba337a - https://godotengine.org
OpenGL ES 3.0 Renderer: NVIDIA GeForce RTX 2060/PCIe/SSE2
Async. shader compilation: OFF

ERROR: gdnative-core: ignoring new registration: `dijkstra_map_gd::Interface` has already been registered
   at: <unset> (C:\Users\Joel\.cargo\git\checkouts\godot-rust-1005fa777d499fc2\91e1b77\gdnative-core\src\init\init_handle.rs:136)

However, the project continued to load in its default scene, and when I run the game from the editor, pathfinding using this addon seems to be working! I have not tested exporting the game.

It's not clear why this error is/was happening several times in a row sometimes, once in awhile other times, and not happening many times in a row at other times. Probably some kind of race condition with the order things finish loading, I'd guess.

astrale-sharp commented 1 year ago

At least it doesn't crash anymore!

goblinJoel commented 1 year ago

Indeed! Seems to have made the issue harmless, which makes me pretty happy. Thanks for your help :)

astrale-sharp commented 1 year ago

You're welcome :)