codecat / godot-tbloader

TrenchBroom Loader for Godot 4. (Alternative to Qodot)
MIT License
232 stars 29 forks source link

Map Import Plugin #56

Open rwilliaise opened 1 year ago

rwilliaise commented 1 year ago

MapImportDemo

Allows users to drag and drop maps into the filesystem, similar to the 3D asset import process.

I made this mostly to gain familiarity with the code base, excuse me if there are any style regressions or other issues. I added compilation database generation to the buildsystem as I use Neovim with coc.nvim, please tell me if this should be omitted.

rwilliaise commented 1 year ago

Ooh okay - found a pretty large issue. If a user has more than a couple maps, a race condition occurs and hard freezes Godot. This can occur even with one map if you have an auto-save folder left over from Trenchbroom.

image

The crash arises from loading resources (e.g. textures, entities, materials) from multiple threads. I did not realize that _import isn't always ran from one thread. I'll try and fix this today.

codecat commented 1 year ago

Sorry I wasn't able to respond earlier. This looks super cool! Didn't even realize you could do this with addons/extensions 😱 I'll have to dive into the code a bit later for a proper review.

As for the compilation database, it might be best to leave that out for now (and do that for a separate PR?), because I'm not at all familiar with that so it would be good to have a separate discussion about it.

rwilliaise commented 1 year ago

Thank you! I will remove the compilation database gen from the branch and create a separate PR some other time - it's not super urgent.

rwilliaise commented 1 year ago

The crash arises from loading resources (e.g. textures, entities, materials) from multiple threads. I did not realize that _import isn't always ran from one thread. I'll try and fix this today.

After looking at it again, the freeze arises from a call to create_trimesh_shape at src/builder.cpp at line 384. A similar issue is tracked at https://github.com/godotengine/godot/issues/69076, and unless a viable workaround is found, this PR is blocked.

ashelleyPurdue commented 3 months ago

The crash arises from loading resources (e.g. textures, entities, materials) from multiple threads. I did not realize that _import isn't always ran from one thread. I'll try and fix this today.

After looking at it again, the freeze arises from a call to create_trimesh_shape at src/builder.cpp at line 384. A similar issue is tracked at godotengine/godot#69076, and unless a viable workaround is found, this PR is blocked.

I've been able to make this work by disabling multi-threaded importing in Godot's project settings. Is that a viable workaround in your opinion?

rwilliaise commented 3 months ago

Unfortunately, I do not think it is a viable workaround for actual production builds, as requiring multi-threading importing to be off to use this plugin would be, in my opinion, odd behavior. Although, if it makes my branch usable, feel free to keep it off.

A viable workaround could be putting the trimesh generation in flight and polling the generation, as referenced in https://github.com/godotengine/godot/issues/69076#issuecomment-1611097801, but I do not have the time to build and test something like that currently. Sorry.