Allow to set custom feature tags for testing

[KoBeWi]

Closes godotengine/godot-proposals#373

Added a dialog under "Project -> Tools -> Custom Feature Tags..." that allows to specify custom feature tags when running the project. The are read during Project Settings initialization (passed via __GODOT_EDITOR_CUSTOM_FEATURES__ environment variable), so they also affect project setting feature overrides.

godot.windows.tools.64_cInpHtDNT0.mp4

EDIT:
New look:

Production edit: closes godotengine/godot-roadmap#18

[akien-mga]

I think this kind of feature would be nice to have exposed together with the dialog I propose here to configure run options and number of instances: godotengine/godot-proposals#522 (comment)

There could be an additional text field for each instance to configure custom feature tags (so you can also test e.g. how demo and full clients could play together in a multiplayer game, or test steam and eos variants at once, etc.).

[rsubtil]

Would this also work when exporting projects through the command line for CI environments? Something like this?
__EDITOR_CUSTOM_FEATURES__=feature1,feature2 godot --export ...

[Calinou]

Would this also work when exporting projects through the command line for CI environments? Something like this? __EDITOR_CUSTOM_FEATURES__=feature1,feature2 godot --export ...

It should work, but that reminds me…

The environment variable's naming should be changed to match Godot conventions. Specifically, all environment variables should be prefixed with GODOT_ to avoid conflicts with other programs: GODOT_EDITOR_CUSTOM_FEATURES

[KoBeWi]

Would this also work when exporting projects through the command line for CI environments?

No, it only works when running the project. It does not affect the export.

EDIT:

I think this kind of feature would be nice to have exposed together with the dialog I propose here to configure run options and number of instances

I can change this after #65753 is merged

[Calinou]

Tested locally (rebased against master da81ca6), it works as expected.

I suggest moving this to the Debug menu indeed, as this functionality is not persisted to project.godot.

[Calinou]

Is there a reason the environment variable is surrounded by 2 underscores? I think the EDITOR makes it clear that it'll only work in editor builds. This feature should be able to work when running a project from the command line when using an editor build after all.

For comparison, other environment variables used by Godot aren't surrounded by underscores.

[KoBeWi]

This is to signify that this variable is internal. Another example:

godot/editor/editor_run.cpp

Line 306 in da81ca6

OS::get_singleton()->set_environment("__GODOT_EDITOR_STOP_SHORTCUT__", shortcut);

Although maybe using it from command line makes sense.

[adamscott]

I would put this dialog inside the editor settings, as a simple entry. I don't feel it needs to have a new menu entry.

[Calinou]

I would put this dialog inside the editor settings, as a simple entry. I don't feel it needs to have a new menu entry.

Setting feature tags for testing is specific to a project, rather than something you enable for all projects in your editor instance.

Maybe we should merge this dialog together with the one that lets you customize command line arguments for each instance?

[akien-mga]

I think this kind of feature would be nice to have exposed together with the dialog I propose here to configure run options and number of instances

I can change this after #65753 is merged

This can be worked on now. Feel free to involve other folks in the UI concept if you're unsure about how to feature everything in that dialog.

[KoBeWi]

I'm reworking it right now. Here's my idea for the unified instance list:

[akien-mga]

I'm wondering if this wouldn't work better with a table like we have for Autoloads, etc.

Instance	Launch Arguments	Override Main Run Args	Feature Tags	Override Main Tags
1	--yolo	[x]	blue, red, green	[ ]
2		[ ]	demo	[x]
3	"res://my_test_scene.tscn" --verbose	[x]		[ ]

The main challenge may be for the "Override ..." columns not to take too much space so there's still enough for the actually args and feature tags. Maybe the column names can be wrapped on two lines.

Edit: Seems possible to wrap long column names for columns meant to be small, we do it for Autoloads "Global Variable":

Edit 2: And possibly it could use the same kind of widget we have in the inspector to add/remove elements to arrays/dictionaries... but I guess those are completely different types of controls so it might requirement significant rework which is maybe not worth it.

[KoBeWi]

Ok pushed the design I showed above, I'll try to change it to table.

EDIT:
Still not changed, I was just fixing CI.

[KoBeWi]

Here's the dialog with table:

It's more compact (at least vertically), but it lost tooltips for overrides.

Should I push this version?

[Calinou]

It's more compact (at least vertically), but it lost tooltips for overrides.

It looks pretty good 🙂

I'm wondering if this table could have alternating row colors, by the way. This would help improve readability of long rows, but I don't remember if we have support for that in Tree.

Should I push this version?

Yes – if in doubt, push it as a separate commit and squash it later.

[akien-mga]

That looks pretty good to me!

[KoBeWi]

Pushed the new design. Old code is in 5dd6941

[akien-mga]

Tested briefly, it seems to work well!

The original minsize seems to be too small to fit the table initially, it would need some more space:

---

Bunch of ideas for future improvements, but let's maybe wait and see what users request:

• Some users might want to preserve their config for multiple instances while reducing the amount of instances they're testing now. Currently reducing the count will lose the config for the last instances in the list. There could be a checkbox to enable/disable a specific instance while keeping its config saved.
• We still don't have a simple way to query all enabled custom feature tags aside from checking each manually with OS.has_feature()?
• Multiple instances could use a way to be identified visually. We could change the window title (as suggested in Identify debug instances with window title #68054, but this should be redone based on this new dialog, and ideally not affect the single-instance use case), or add another kind of identifier that could be queried from code. We could also envision an editor hotkey (or checkbox in this dialog) that toggles displaying the instance number on each instance. Or many other options, some which may best be explored via plugins until we figure out what makes most sense.

[akien-mga]

For discussion's sake - any particular reason to pass these as an environment variable instead of a new command line argument?

[KoBeWi]

Not really. Environment variable is easier to access and parse, but it could be a command line argument I guess. I used the same approach as #47744 (though in that PR a variable made more sense, because it was passing event data).

[KoBeWi]

We still don't have a simple way to query all enabled custom feature tags aside from checking each manually with OS.has_feature()?

Some features are defined using #ifdef, so they can be only queried.

Multiple instances could use a way to be identified visually.

#34528 would be one idea, but not sure how it would work.

The original minsize seems to be too small to fit the table initially, it would need some more space:

Should I change it now?

[akien-mga]

The original minsize seems to be too small to fit the table initially, it would need some more space:

Should I change it now?

Yeah that would be good if you know what to change. For the record I'm on a 2K screen with 150% system scaling (and the same configured for Godot scaling).

[KoBeWi]

It's this line:

set_min_size(Size2i(1200, 500) * EDSCALE);

Though not sure what value I should use. I didn't observe this problem on my side.

[KoBeWi]

Increased height to 600.
I also changed how dialog size is determined (it's set during popup instead of minimum size, so you can shrink it now).

[akien-mga]

Thanks!