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

Map Import Plugin #56

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Map Import Plugin #56

wants to merge 4 commits into from

Conversation

rwilliaise
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Owner

codecat commented Jan 3, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

@ashelleyPurdue
Copy link

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
Copy link
Contributor Author

rwilliaise commented Mar 14, 2024

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 godotengine/godot#69076 (comment), but I do not have the time to build and test something like that currently. Sorry.

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

Successfully merging this pull request may close these issues.

3 participants