Add option to use Editor icons from the path to icon in GDExtension

[sylbeth]

Describe the project you are working on

A GDExtension file autogenerator for Rust GDExtension. It handles the pathfinding to the libraries, associating classes to icons, dependencies and the configuration of the .gdextension file. https://github.com/sylbeth/gdext-generation

Describe the problem or limitation you are having in your project

While working on my project, I went back and forth checking on ways to handle linking classes to icons if all you want is to use an already existing Godot Editor node. The only ways I could think of were either the user manually copy pasting the files or running a git sparse-checkout to get all the icons in bulk, which just means bloating your project with icons you may never use that are already in the executable to add insult to injury. So I thought the solution would be, in the meantime, as there are no default base class finding implementations yet, to just define it in the .gdextension file.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

It would be a change of the [icons] table in the .gdextension file. There, the key value pairs are ClassName - "path/to/icon". There, I propose the following, ClassName - "ClassNameWithIconDefined", or, at least, built-in classes, since it makes little sense for the rest. That way, one can either link to an svg file, and thus, ending with .svg, or starting with "res://", or whatever the limitations at hand are, or put directly the name of the class to use the icon from.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

In editor/editor_node.cpp, in the function _get_class_or_script_icon, it's where, to my knowledge, the magic of icon finding and linking happens. There we can find this snippet of code:

// Check if the class name is an extension-defined type.
Ref<Texture2D> ext_icon = ed.extension_class_get_icon(p_class);
if (ext_icon.is_valid()) {
	return ext_icon;
}

This is where the gdextension icon is found. If we follow the function that's being called, we find the following:

Ref<Texture2D> EditorData::extension_class_get_icon(const String &p_class) const {
	if (GDExtensionManager::get_singleton()->class_has_icon_path(p_class)) {
		String icon_path = GDExtensionManager::get_singleton()->class_get_icon_path(p_class);
		Ref<Texture2D> icon = _load_script_icon(icon_path);
		if (icon.is_valid()) {
			return icon;
		}
	}
	return nullptr;
}

Here we can clearly see that as soon as it finds the path, it tries to find the icon associated. There could be two ways to implement this, either in the "extension_class_get_icon", where the following code (taken from the _get_class_or_script_icon) could probably be used (after the check for icon validity):

else if (theme.is_valid()) {
		if (theme->has_icon(&icon_path, EditorStringName(EditorIcons))) {
			return theme->get_icon(&icon_path, EditorStringName(EditorIcons));
		}
}

Or it could be also done in the original function, by applying roughly the same code.

If this enhancement will not be used often, can it be worked around with a few lines of script?

No, it cannot. You would have to copy the files, bloating your directory of addons, then manually linking it anyway.

Is there a reason why this should be core and not an add-on in the asset library?

It is part of the editor icon linking for gdextension, which is one of the quite asked requests for it.

[sylbeth]

Okay, I pulled up my custom godot and implemented it myself. It works, and it can act as a first fallback before fallbacking on Node. This way, people can try to add their own icons, if they don't work, or they aren't there, and their name is the same as one of the themes' icons, then it pulls it from there. The code changed is the code between comments (part of _get_class_or_script_icon):

if (theme.is_valid()) {
		if (theme->has_icon(p_class, EditorStringName(EditorIcons))) {
			return theme->get_icon(p_class, EditorStringName(EditorIcons));
		}
		
		//If there is a path associated with the node but there was no icon there or it was invalid.
		//Then it takes the basename of the file and, if it's the name of an icon in the theme, it uses that instead as a fallback.
		//This enables using "Node2D" as a valid path to get the icon without having to copy it and have it inside the project folder.
		/*
		if (GDExtensionManager::get_singleton()->class_has_icon_path(p_class)) {
			String icon_name = GDExtensionManager::get_singleton()
			->class_get_icon_path(p_class).get_file().get_basename();
			if (theme->has_icon(icon_path, EditorStringName(EditorIcons))) {
				return theme->get_icon(icon_path, EditorStringName(EditorIcons));
			}
		}
		*/

		if (!p_fallback.is_empty() && theme->has_icon(p_fallback, EditorStringName(EditorIcons))) {
			return theme->get_icon(p_fallback, EditorStringName(EditorIcons));
		}

		// If the fallback is empty or wasn't found, use the default fallback.
		if (ClassDB::class_exists(p_class)) {
			bool instantiable = !ClassDB::is_virtual(p_class) && ClassDB::can_instantiate(p_class);
			if (ClassDB::is_parent_class(p_class, SNAME("Node"))) {
				return theme->get_icon(instantiable ? "Node" : "NodeDisabled", EditorStringName(EditorIcons));
			} else {
				return theme->get_icon(instantiable ? "Object" : "ObjectDisabled", EditorStringName(EditorIcons));
			}
		}
	}

This way, worst case scenario the GDExtension::get_singleton()->class_get_icon_path(p_class) gets called twice, but this can easily be changed if that's what's wanted here. In any case, this does exactly what I asked.

[RadiantUwU]

I honestly thought Godot already did this (or how I remember with my objects). Looks like a good enhancement that really would be a shame if we didnt have.