Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inspector doesn't appear for resources extending AnimationLibrary #91516

Closed
ydeltastar opened this issue May 3, 2024 · 11 comments · Fixed by #91522
Closed

Inspector doesn't appear for resources extending AnimationLibrary #91516

ydeltastar opened this issue May 3, 2024 · 11 comments · Fixed by #91522

Comments

@ydeltastar
Copy link
Contributor

Tested versions

v4.3.dev.custom_build [89850d5]

System information

Windows 11

Issue description

The inspector doesn't show up when selecting AnimationLibrary resources. While it may be disabled by design so we edit it on the AnimationPlayer's manager, it means any resource based on it can't use the inspector as well.

I have animations exported as separated glTF files. AnimationLibrary only accepts Animation resources so I have to export them all as separate files ending up with duplication (a .glTF and an .res for the same animation) plus manual library management. I'm trying to extend AnimationLibrary so I can set an array of resources and the library will build automatically if they have animations, but the inspector doesn't appear when I select it.

Steps to reproduce

Create a script that extends AnimationLibrary and a resource file from it.
The inspector doesn't appear if you select the file.

class_name AnimationCollection extends AnimationLibrary

@export var hey:Array[Resource]

Minimal reproduction project (MRP)

N/A

@AThousandShips
Copy link
Member

This is more of a new feature as AnimationLibrary isn't really designed to be edited outside of the AnimationMixer I'd say, there's little reason to do so generally, there should be other ways to workaround storing these without duplication though

@ydeltastar
Copy link
Contributor Author

ydeltastar commented May 3, 2024

there should be other ways to workaround storing these without duplication though

Only resources extending an AnimationLibrary can be assigned in the AnimationPlayer so a workaround is to do a factory resource to create a library file and use it instead. But this goes back to the duplication issue.

@AThousandShips
Copy link
Member

AThousandShips commented May 3, 2024

My bad thought you needed to inspect them in some other resource, you are talking about opening them in the file system, not as a property in the inspector, yes?

However I think AnimationLibrary is designed to be used internally to the animation player, but might be wrong, so it's meant to be edited not via the inspector but via the dedicated editor, much like SceneReplicationConfig, TileSet, etc.

@ydeltastar
Copy link
Contributor Author

ydeltastar commented May 3, 2024

you are talking about opening them in the file system, not as a property in the inspector, yes?

Yes. Double-click from the file system and edit exported variables. Then, if it extends AnimationLibrary, I can also assign it in the AnimationPlayer and I can use all editor features such as preview and animation tree configuration.

Wouldn't it be enough to just hide the proprieties the user is not supposed to edit so it shows an empty inspector? This is the case if we inspect a library from an export variable. Why disable the inspector itself?

@AThousandShips

This comment was marked as outdated.

@AThousandShips
Copy link
Member

AThousandShips commented May 3, 2024

So are you looking at a library that was imported? That's opened in the import view by design, a non-imported one is fully editable in the inspector, the imported case is handled by settings

So then the question is if the editing of the imported cases should be done that way, I can't import my own animations at the moment because I don't have an appropriate resource, but it works fine otherwise

Edit: It seems it only some times works to open it like this, so if this is a matter of the case with an animation library stored as a res or tres file then this would fix the issue, please update:

Edit 2: It does indeed fail to open when double clicked, it only worked for me when creating the resource, so this is a bug, thank you for reporting, and a fix is open

@ydeltastar
Copy link
Contributor Author

Here is a concrete example:
custom-animation-library.zip

This script will load two glTF (each with a single animation) and an actual Animation .res. It will automatically look for the Animation reference and add it to the AnimationLibrary.

@tool
class_name AnimationCollection extends AnimationLibrary

@export var resources:Array:
	set(value):
		resources = value
		update_library()

func _init() -> void:
	resources = [
		preload("res://animations/x_sword-attack.glb"),
		preload("res://animations/x_sword_attack.res"),
		preload("res://animations/x_sword_idle.glb")
	]

func update_library():
	for resource in resources:
		if resource is PackedScene:
			var scene = resource.instantiate() as Node
			var animation_player = scene.find_child("AnimationPlayer") as AnimationPlayer
			
			if animation_player:
				for name in animation_player.get_animation_list():
					var animation = animation_player.get_animation(name)
					add_animation(resource.resource_path.get_file(), animation)
		elif resource is Animation:
			add_animation(resource.resource_path.get_file(), resource)

If you go main.tscn -> AnimationPlayer -> Animation -> Manage Animations you see the collection worked. It auto-added 3 animation references each from each file.
The only problem is that there is no way to change the resources variable because the inspector doesn't appear when I select animation_collection.tres.

@AThousandShips
Copy link
Member

Please try the linked PR, it should fix that, I missed the specific details when first testing this and this is very much a bug, thought the exact decision on how to inspect the library is a future question, but the specific code is broken

@ydeltastar
Copy link
Contributor Author

That's it, thank you! It fixed the issue.

It's fine if all priorities are hidden to be edited with a specialized editor but it's great to leave the option to extend it with tool scripts.

@AThousandShips
Copy link
Member

The properties on it (which are none for now but default, but available for extended versions, i.e. your case) are shown, please test if things like saving works correctly for your cases, I don't have an animation library to test on at the moment

@ydeltastar
Copy link
Contributor Author

It is working fine. I'm testing with a good number of glTF and .res mixed I can add/remove, playback in the player, clear cache and reload the editor just fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants