godotengine
Add support for resource conversion plugins in filesystem dock
Based on a past discussion for a long overdue UX feature. Makes ResourceConversionPlugins avaliable to the filesystem dock, including allowing multiple files to be batch converted in one go and updating the usage of said resources in active scenes. Keeping in draft for a bit longer since I want to look at revising some of the API naming around detecting viable conversion plugins based purely on class name to avoid having to load the resource beforehand, and possibly look into introducing a warning dialogue explaining how conversion of types is a one-way operation.
That being said, I think the majority of the implementation is fine.
Decided to revisit this and add the remaining things I wanted to add because, to be frank, the current workflow for converting standard materials to shader materials is kind of atrocious. It now gives you warning when you attempt to convert files from the filesystem dock, and although there are currently no instances where multiple conversion types are avaliable, it will now collapse the conversion choices into a submenu if more than one is present in the future.
I'm not happy about the fact that I had to add a new (and fairly reduntant) function to the API. I would have much rathered change 'handles' to a string type rather than a resource type, but I realize that doing so would break the API and subsequently break GDExtensions. As such, another function is added to ResourceConverterPlugins to check handling by type name rather than resource, meaning the resources won't have to be loaded when simply right-clicking on them
I think handles() takes a whole object to allow filtering objects by more than only a type (although it's not really used in practice).
You can use this trick to achieve the same functionality with a low cost:
Ref<Resource> temp = ClassDB::instantiate(p_type);
for (int i = 0; i < resource_conversion_plugins.size(); i++) {
if (resource_conversion_plugins[i].is_valid() && resource_conversion_plugins[i]->handles(temp)) {
ret.push_back(resource_conversion_plugins[i]);
}
}
Instantiating an empty resource is much cheaper than loading it.
I see some faulty logic in _get_valid_conversions_for_file_paths(). It returns empty Vector when it encounters any problem, but otherwise it will happily dump every plugin into one vector. The plugins might come from different resource types, so the list may contain conversion entries that only apply for some of the files. If that's the case, why abort when a file has no available conversions? Such files could be simply skipped.
Maybe I'm misunderstanding something.
Sorry I neglected this PR for so long, but okay, I wanted to take another shot of rebasing and attempting to mainline this. I still think its a very handy convinence feature which addresses a couple of needless frictions for the user when it comes to converting standard materials into ShaderMaterials (currently you can only really do it if its already assigned to an inspector property). I've addressed your feedback @KoBeWi and included things like unifying the replace function in the inspector dock, making use of HashSets and made use of your suggestion of using the ClassDB::instantiate method instead.
One thing I do want to question is the issue regarding the behaviour of the _get_valid_conversions_for_file_paths(), where it will return empty if any error is encountered. I think this might be intended behaviour; the operation is intended to exclusive rather than inclusive, and the part of the function which checks if there are any converters which are not valid across another resource:
// Check existing conversion targets and remove any which are not in the current list.
for (const String &S : all_valid_conversion_to_targets) {
if (!current_valid_conversion_to_targets.has(S)) {
all_valid_conversion_to_targets.erase(S);
}
}
I might also be missing something, but I believe this behaviour is intended since if the user has multiple resource types selected, it should only exclusively show the conversion targets which are valid for every resource in the selection.
Found a bug: if there are multiple conversion options, the first one in the list will always be used regardless of what you select. I wrote some dumb custom plugin for testing:
@tool
extends EditorPlugin
func _enter_tree() -> void:
add_resource_conversion_plugin(Conver.new())
class Conver extends EditorResourceConversionPlugin:
func _handles(resource: Resource) -> bool:
return resource is Material
func _converts_to() -> String:
return "GDScript"
func _convert(resource: Resource) -> Resource:
var code := """extends Node
func _ready():
print(%s.new())
""" % resource.get_class()
var gd := GDScript.new()
gd.source_code = code
return gd
Okay @KoBeWi and @AThousandShips, I think that should address most of the issues raised. One I'm not sure about is @KoBeWi's point about setting conversion dialog text in the constructor.
Updated again! @KoBeWi I wasn't sure if that was what you meant by an 'early return', but can you confirm that the change I made was what you were suggesting?
