search

godotengine / godot-cpp

C++ bindings for the Godot script API
MIT License
1.7k stars 528 forks source link

[SCons] Add option to build without threads #1451

Closed Faless closed 4 months ago

Faless commented 5 months ago

This is relevant for the Web platform, where builds with and without threads are incompatible.

Faless commented 5 months ago

@adamscott in https://github.com/godotengine/godot/pull/85939 we added the threads feature flag.

I think for max compatibility we should also add a nothreads feature flag when threads are not available. To make it less error prone when defining the tags in the .gdextension files (right now the order matters I think). WDYT?

adamscott commented 5 months ago

I think for max compatibility we should also add a nothreads feature flag when threads are not available.

In what cases would not OS.has_feature("threads") != OS.has_feature("nothreads")?

Faless commented 5 months ago

In what cases would not OS.has_feature("threads") != OS.has_feature("nothreads")?

If you look at the changes in the .gdextension file:

https://github.com/godotengine/godot-cpp/pull/1451/files#diff-d24941d2910274e6d843d090ee0af8f2f32fbcc4c37112c21d394a36d183d201R32-R35

I can filter the "thread" version, but not the "nothread" version (and I'm relying on the fact that tags are matched in order).

So if I provide only a "nothread" version, I don't think I can't let godot know to not try to load that extension in threaded builds via the .gdextension file.

DmitriySalnikov commented 2 months ago

@dsnopek can these changes be cherry picked to 4.1 and 4.2?

I understand that there is no support for threads=yes/no in Godot 4.1 and Godot 4.2, but this is necessary for compatibility with Godot 4.3. As I pointed out here, I managed to build my library compatible with Godot 4.2+ (actually 4.1.4, but so far there is a problem with shaders). To do this, I needed a patch with this PR.

dsnopek commented 2 months ago

I don't think it'd do any harm to cherry-pick it!

dsnopek commented 3 weeks ago

Cherry-picked for 4.2 in PR https://github.com/godotengine/godot-cpp/pull/1570

dsnopek commented 3 weeks ago

Cherry-picked for 4.1 in PR https://github.com/godotengine/godot-cpp/pull/1572