GDExt: Allow specifying inclusion/exclusion tags

[Faless]

What problem(s) does this PR solve?

• See Document need for exclusion of webrtc.gdextension when exporting on Web webrtc-native#182 .

Additional information

This is useful for extensions that can be skipped on specific platforms like the WebRTC extension (which on Web is part of core), so users don't have to manually add the exclusion for specific export presets..

See also: godotengine/webrtc-native#194

[dsnopek]

Thanks!

This is something we've talked about a number of times, and I think we should do it, but I'm unsure about some of the details in this PR:

• The name skip_flags takes feature tags, so should it actually be skip_features or skip_tags?
• Also, does it make sense to frame this negatively as "skipping" where we list the platforms the extension shouldn't be loaded on? Or, should we frame it positively as which platforms it should be loaded on? Or both?
• In past discussions, we considered the list of libraries to already be a positive list of which feature tags this should be loaded on, and simply adding a new a boolean which would indicate that it's OK to skip this extension if there is no library that matches. I forget what name we discussed, but it'd be something like skip_if_library_missing = true

So, I think there's things to discuss around how we want to expose this to users

[Faless]

The name skip_flags takes feature tags, so should it actually be skip_features or skip_tags?

Ah, indeed, I messed up, I originally called it skip_tags then changed it to skip_flags because I misremembered the name to be "feature flags" :/. I don't mind changing it back.

• Also, does it make sense to frame this negatively as "skipping" where we list the platforms the extension shouldn't be loaded on? Or, should we frame it positively as which platforms it should be loaded on? Or both?

Well, the problem is that right now the GDExtension assume you always need to export. One might argue that we should implement both filters (include/exclude) like in the export presets, but is there a use case for it?

• In past discussions, we considered the list of libraries to already be a positive list of which feature tags this should be loaded on, and simply adding a new a boolean which would indicate that it's OK to skip this extension if there is no library that matches. I forget what name we discussed, but it'd be something like skip_if_library_missing = true

The problem I see is that, you in fact, want to be warned by default if the library is missing most of the times.

That is almost always the case so you don't create a broken export by mistake (e.g. you are exporting for an arch you forgot to compile the extension for).

On the other end, the extension creator is expected to know which tags should always be excluded (e.g. web, or template, or editor, ecc). And should be able to provide custom feature tags for exclusion (e.g. no-webrtc).

I know this has been discussed multiple times, it's just been quite a few years and we still don't have a solution.

I think at this point we should just be implementing something that works for the known cases (notably webrtc and git plugin) instead of trying to guess what could be needed in the future 🤷

[Faless]

• Or, should we frame it positively as which platforms it should be loaded on? Or both?

Okay, I've been thinking about this a bit more. If we want to make it more flexible, covering more potential cases, we could, implement this as include_only_tags and exclude_only_tags (can't be both set).

If the include_only_tags is not empty, only export matching those tags are included (all others are skipped).

If the exclude_only_tags is not empty, only exports that match the tags are excluded.

We also probably want to make full match of the tags, e.g.:

include_only_tags = "linux.x86_64, linux.x86_32, windows.x86_64, windows.x86_32, macos.universal"

WDYT?

[dsnopek]

One might argue that we should implement both filters (include/exclude) like in the export presets, but is there a use case for it?

The main use-case for the include filters that has been discussed is GDExtensions that only work on iOS or Android (because they depend on platform features that only exist on those platforms).

But, yeah, I don't know of any use case for using both at once, only using one at a time, so your proposal makes sense to me.

If the include_only_tags is not empty, only export matching those tags are included (all others are skipped).

If the exclude_only_tags is not empty, only exports that match the tags are excluded.

We also probably want to make full match of the tags, e.g.:

include_only_tags = "linux.x86_64, linux.x86_32, windows.x86_64, windows.x86_32, macos.universal"

That sounds goods to me!

However, I'm wondering if we want to do this as an array, rather than a string? Like:

include_only_tags = ["linux.x86_64", "linux.x86_32", "windows.x86_64", "windows.x86_32", "macos.universal"]

I think I'd slightly favor the array version over the string version

[Faless]

@dsnopek I ended up calling the options include_tags/exclude_tags (both as array as you suggested), and made them not mutually exclusive.

In practice:

• If include_tags is empty, everything is included by default, otherwise only exports/platforms that match are included.
• If exclude_tags is empty, nothing is excluded, otherwise exports/platforms that match are excluded.

So you can for example do:

include_tags = ["editor.windows", "editor.linux", "editor.macos"]
exclude_tags = ["windows.arm64"]

This is hopefully flexible enough.

[Ivorforce]

This was brought up in the GDExtension meeting.

I haven't followed the whole backstory, but in my opinion this feature seems useful (if niché) and should be added.

I would personally choose a different syntax. There's nothing wrong with explicit inclusion/exclusion tags, so I'd be OK with those as-is.

The one I'd prefer though would re-use / extend our current binary matching system.
Something like this (blacklist):

[libraries]
macos.single.debug = "./macos/libEXTENSION-NAME.macos.template_debug.dylib"
ios = none

The files are read top to bottom, so you could make a whitelist like this:

[libraries]
macos.single.debug = "./macos/libEXTENSION-NAME.macos.template_debug.dylib"
; Exclude all remaining combinations
; Would be easier if we added a wildcard, but this works:
debug = none
release = none

But yea, like I said. Wouldn't die on that hill.

[Mariano89]

Tested on macOS

I'm using the godot-firebase-ios plugin in a game I'm working on, and its maintainer got the review request and asked if I could take a look.

I pulled the macOS editor from this PR's CI artifacts (version string 4.8.dev.gh-121575.80a506d5a).

I created two copies of the plugin's demo project from upstream main, identical except that I added include_tags = ["ios"] to one of them in the .gdextension file. In both, I ran a GDScript that reports what GDExtensionManager has loaded.

demo-a (no tags):

PROBE is_extension_loaded: true
PROBE loaded_extensions: ["res://addons/GodotFirebaseiOS/GodotFirebaseiOS.gdextension"]

demo-b (include_tags = ["ios"]):

PROBE is_extension_loaded: false
PROBE loaded_extensions: []

One required step was to open each project in the editor first, or both runs return a false negative.

I've tested on Mac so far, still need Windows and Linux, but let me know if this was the right approach and I'll replicate it across the other platforms. Also, happy to test again if the syntax changes.

[Faless]

@Ivorforce the gdextension is a ConfigFile, so we can't add none, but we can probably use an empty string in place of that.

I had a branch doing something similar, but it was getting a bit messy by the way that section is parsed.

This is because that section is parsed trying find "best matching tags" on a common minimum set of tags (notably platform + arch(s)). I think it's doable, just a bit complex for what we need.

[dsnopek]

Thanks! This looks good to me :-)

[Repiteo]

Thanks!