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

Add support for Resource inheritance (prototypal inheritance) #4089

Open
perrauo opened this issue Feb 23, 2022 · 21 comments · May be fixed by godotengine/godot#86779
Open

Add support for Resource inheritance (prototypal inheritance) #4089

perrauo opened this issue Feb 23, 2022 · 21 comments · May be fixed by godotengine/godot#86779

Comments

@perrauo
Copy link

perrauo commented Feb 23, 2022

Describe the project you are working on

Examining Godot as a potential engine for a project.

Describe the problem or limitation you are having in your project

I have a number of skills, abilities, and would like to base one resource on another. As it stands common fields must be re-entered in each similar resource.

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

This is often referred to as prototypical inheritance.

One Godot developer mentioned this feature on our forum.
https://discord.com/channels/212250894228652034/610347507222052867/944622534819733545

Highly sought-after feature in Unity and would give Godot a competitive advantage.
https://forum.unity.com/threads/suggestion-prefab-variants-for-any-object-scriptableobject-variants.585268/
https://github.com/GieziJo/ScriptableObjectVariant
https://www.reddit.com/r/Unity3D/comments/fn5db2/is_there_some_sort_of_scriptableobjects_variants/
https://blog.gemserk.com/2020/05/26/using-unity-prefabs-and-gameobjects-only-for-data/

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

This should be easier than the current scene inheritance Godot has.

We could keep a list of dirty property names (strings), a reference from instance to the parent, and a reference from parent to instance. In the editor when the parent is modified, the property could be propagated to the children. The links could be forgotten at runtime. This would prevent having any cost at runtime.

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

No.

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

This is because this feature could be used in more than custom resource types. For example Material Inheritances.

Keywords

Templates, Resource Inheritances, Prototypal Inheritance, ScriptableObject Variants

@YuriSizov
Copy link
Contributor

YuriSizov commented Feb 23, 2022

I think this proposal is missing an explanation of what you want to have in Godot and how it is supposed to work. If you don’t have a concrete actionable proposal, please open a discussion instead. Or clarify what and how do you propose to implement.

Furthermore, you need to explain in more detail what limitation there is that you cannot overcome. What you describe is solved by extending classes normally in Godot, be it nodes or resources.

@Zireael07
Copy link

He wants not to enter common fields in resources. AFAICT you have to enter data by hand every time you create a new resource, extending doesn't help. Something like having default values for export variables.

Alternatively, the use case could be served by the spreadsheet proposal?

@YuriSizov
Copy link
Contributor

YuriSizov commented Feb 23, 2022

Default values should be taken from the property declaration, the get method implementation, and the property_get_revert method implementation. There may be some limitations or bugs, which needs addressing and fixing, but I still don’t understand what exactly is the problem and what exactly is the proposal.

AFAICT you have to enter data by hand every time you create a new resource, extending doesn't help. Something like having default values for export variables.

Also, possibly related to this #2280?

@perrauo
Copy link
Author

perrauo commented Feb 23, 2022

No this is different than default property definition. I would like to benefit from the editor to define the default values.
https://forum.unity.com/threads/asset-inheritance-no-boilerplate-material-and-scriptableobject-variants.1191088/

@YuriSizov
Copy link
Contributor

Unless you change the value of a property in the child scene, this is already how it works in Godot.

So, again, please provide a more concrete description of the issue, and a description of a solution that you want to implement in Godot. If this is a general idea without any of those, then please open a discussion instead. Proposal must define a problem and a solution, relative to Godot. Linking private forums and examples from other engines is not enough, I'm afraid.

@perrauo
Copy link
Author

perrauo commented Feb 24, 2022

I'm not talking about scenes here, talking about resources.

@YuriSizov
Copy link
Contributor

I'm not talking about scenes here, talking about resources.

Scene is a type of resource. All resources work the same. Unless you change the value, it's not saved. If it's not saved, whatever is the default for the parent class is the default for the child class.

@Zireael07
Copy link

@pycbouh: Whoa, that's very poorly documented then. I always thought resources and scenes were completely separate concepts...

@AnidemDex
Copy link

Scenes are PackedScene type, and those extends from Resource, so they are resources.

Looking at https://forum.unity.com/threads/asset-inheritance-no-boilerplate-material-and-scriptableobject-variants.1191088/ I think is something possible to replicate that behaviour playing with property list and property_get_revert function for custom resource class (maybe that class can have a kind of hook to modify other objects? Not sure), but no idea if is that what you want to do or is something else

@TheDuriel
Copy link

TheDuriel commented Feb 25, 2022

Looking that the OP posts examples.

What OP appears to be looking for is: Resource inheritance.

Like we have inherited scenes, where we load a tree of nodes with properties, and can then change those properties and save it as yet another scene.

https://gameprogrammingpatterns.com/prototype.html#prototypes-for-data-modeling This is a more comprehensive example.

Inheritance, for Data, not Classes.

This does have real world applications. For example if I define a Resource called "EnemyData". And create and save an instance of this Resource to disk as "GoblinData", but now want to make several more Goblins that share common properties. My only way to do so is to manually update all the values I want to keep the same.

With Resource Inheritance, I could define a new instance of EnemyData, save it as GoblinKingData, and have it refer to GoblinData for any property I do not manually set.

This could be achieved via code by creating a GoblinData resource class that extends EnemyData, and sets these defaults. But now we are asking a game designer to open up an IDE just to tweak those base values.

@YuriSizov
Copy link
Contributor

YuriSizov commented Feb 25, 2022

This does have real world applications. For example if I define a Resource called "EnemyData". And create and save an instance of this Resource to disk as "GoblinData", but now want to make several more Goblins that share common properties. My only way to do so is to manually update all the values I want to keep the same.

I guess I see what you're saying. So instead of inheriting in the OOP sense, this idea is about inheriting data itself? Basically, use one instance as fallback for another instance?

If so, I think this pattern can be implemented in userland where the script for your resource would have a field to reference another instance of the same data script. I'm not entirely sure this needs to be implemented in the core somehow, though I guess it is similar to how materials have the next pass member. We don't have a designer-only way to create a data resource, it requires coding anyway, so at this moment it doesn't seem unreasonable to expect that the programmer would be able to set it up for the designer.


Either way, without an implementation detail, this a discussion material, not a proposal material.

@TheDuriel
Copy link

TheDuriel commented Feb 25, 2022

If so, I think this pattern can be implemented in userland where the script for your resource would have a field to reference another instance of the same data script.

It's doable. But extremely ugly in the inspector to the point I'd rather not give that to a designer.

Every property now needs an "override" boolean that your getters need to check for. Checking that the assigned base resource is of the same type as the new one also requires use of get_script() since you can't use your own class name due to the cyclic reference problem.

This kind of editor tooling actually already exists in godot as well, for control nodes that implement custom constants. (At least visually that is what property fields in an inherited resource would need to look like.)

If necessary I can hack together a functional example in GDScript.

@Zireael07
Copy link

@TheDuriel: I said literally the same thing a couple comments above yours, that the data has to be entered by hand every time... thanks for explaining it to @pycbouh more clearly

@YuriSizov
Copy link
Contributor

I said literally the same thing a couple comments above yours, that the data has to be entered by hand every time... thanks for explaining it to @pycbouh more clearly

You also said that it's like default values for export variables, which is a thing that exists and which side-tracked me 🙃


This kind of editor tooling actually already exists in godot as well, for control nodes that implement custom constants. (At least visually that is what property fields in an inherited resource would need to look like.)

Oh, I think exposing something like this is doable and can be useful indeed. The only problem is that currently it works from get_property_list and validate_property, so it still would require some code to set up. Given this and the rest of your message I think this is more about accommodating such patterns and improving some syntax and power features, rather than implementing the "resource inheritance" itself.

I.e. we need to allow self references (if it's still not a thing in 4), we need to expose validate_property to scripting (I think it may be in 4, or possible at least), and we need to make it easier to decorate exported properties using annotations for cases like this "overridable property". But we don't necessarily need to make every resource out of the box support fallback behavior on other resources.

This is where I'd draw the line between what we can do in core, and what we should leave to users who need it this way.

@TheDuriel
Copy link

TheDuriel commented Feb 25, 2022

tool
extends Resource
class_name CheckboxResource

var health: float = 64.0


func _get_property_list() -> Array:
	var p: Array = []
	
	p.append({
			"name" : "health",
			"type" : TYPE_REAL,
			"usage" : PROPERTY_USAGE_EDITOR | PROPERTY_USAGE_CHECKABLE,
			})
	
	return p

The exact details of how this works are very obtuse. But my attempt at reproducing what control nodes do proves that this is technically achievable for developers. But needs documentation.

image

For example I am uncertain how to retain the checked status after saving the resource. Changing the value of 64 to something else does automatically check the box. Furthermore loading the resource does correctly load the new value, but the box does not remain checked.

TL:DR: If _get_property_list() gets thorough documentation, then what the initial proposal desires can be relatively easily implemented by developers.

Also, you need to override _get_property_list(), instead of get_property_list()

A full implementation would look something like this:

tool
extends Resource
class_name CheckboxResource

var health: float setget ,get_health

export(Resource) var prototype: Resource setget set_prototype


func get_health() -> float:
	if not health:
		if prototype:
			return prototype.health
	return 0.0


func set_prototype(new_value: Resource) -> void:
	if get_script() == new_value.get_script():
		prototype = new_value


func _get_property_list() -> Array:
	var p: Array = []
	
	p.append({
			"name" : "health",
			"type" : TYPE_REAL,
			"usage" : PROPERTY_USAGE_EDITOR | PROPERTY_USAGE_CHECKABLE,
			})
	
	return p

@AnidemDex
Copy link

AnidemDex commented Feb 26, 2022

I am uncertain how to retain the checked status after saving the resource

You need to update it manually, from PROPERTY_USAGE_CHECKABLE to PROPERTY_USAGE_CHECKED, in a setter, in a method, where you need it, is just a visual hint for editor @TheDuriel .

... is something possible to replicate that behaviour playing with property list and property_get_revert function for custom resource class ...

What I meant here was doing something similar to what Duriel did, but taking care about the revert value. Now, looking at this issue again (and #4089 (comment)) godotengine/godot#52943 is probably the necessary tool for this, where pinned properties will ignore changes if the parent change the property value, but that pinned value will be applied to inherited objects if they haven't modified the value (wich is kinda similar to what is currently implemented, so is a tool, but not a really necessary one for now).

Literally, doing:

class ClassA extends Resource
# extends Resource because we want to save it to disk
    var foo_int:int := 1 setget set_foo_int
    
    func _init() -> void:
        set_meta("foo_int_default", 1) # yeah, let's play with meta dark magic
    

    func set_foo_int(value:int) -> void:
        foo_int = value
        emit_changed()
    

    func property_can_revert(property:String) -> void:
        if property == "foo_int":
            return true
        return false
    

    func property_get_revert(property:String): # -> Variant
        if property == "foo_int":
            var default_value = 1
            
            if has_meta(property):
                # <property>_default is defined by the class on its constructor, to tell inherited classes what is the default value
                # probably this will not work in all cases, but for now is the idea that crosses my mind. There should be a better one.
                default_value = get_meta(property+"_default")
            
            return default_value
            

    func _get_property_list() -> Array:
        return [{"name":"foo_int","type":TYPE_INT,"usage":PROPERTY_USAGE_DEFAULT}]

class ClassB extends ClassA:
    pass
    
class ClassC extends ClassB:
    pass

Will (kinda) replicate the behaviour of https://forum.unity.com/threads/asset-inheritance-no-boilerplate-material-and-scriptableobject-variants.1191088/ , the part where modified values from parent and modified default values will be set from inherited classes too.

Edit: Probably is better to get_base_script() and see if there's a default value defined to get things working, but for now I think you got the idea

@me2beats
Copy link

I'd like to have resource inheritance like this:

  • properties are inherited
  • subresources are inherited

Say we have resource A and resource B which is inherited from A.
Then:
if we change any resource A property, it is also changed in B.
But not vice versa

@maximkulkin
Copy link

Here is another use case: animations. I export my models with animations from Blender. They are imported as a scene, which I can either inherit or include as an instance in another scene. That scene has animations. In those animations in addition to bones movement I would like to have other stuff, e.g. a methods be called (like emit_signal(), animation events), sounds be played, etc. Right now I can choose to save those animations in a separate files and thus edit them, but the moment I decide to tweak my animations in Blender and re-export, all animations are reimported and all my changes to them are lost. And frankly, there is no convenient way to support this workflow of tuning imported resources preserving the ability to reimport them.

I think, supporting inheritance for resources would be a good symmetry to scene inheritance and would enable new and better workflows.

As of implementation, I think it should be possible to

  1. reference base resource from other resources;
  2. add new option "Inherit" to resource menu in the inspector in addition to "Make Unique";
  3. add "New Inherited Resource" in File System window;
  4. update inspector to highlight in yellow properties which values equal to base resource value.

@Calinou Calinou changed the title Resource Inheritance (Prototypal Inheritance) Add support for Resource inheritance (prototypal inheritance) Sep 27, 2023
@DaloLorn
Copy link

Say we have resource A and resource B which is inherited from A.
Then:
if we change any resource A property, it is also changed in B.
But not vice versa

I would finetune this rule a bit: "If we change any property of resource A, it is also changed in B unless B has overridden it".

@OhiraKyou
Copy link

OhiraKyou commented Nov 12, 2023

If this could work for materials, that would be great.

It was a huge workflow improvement when material inheritance was added to Unity. I typically create a default master material, have level materials inherit from that master, and have unique variants inherit from level materials.

It also helped that shader-based batching was added to Unity, making extra materials less costly. But, that's another topic.

@DaloLorn
Copy link

Heh, I ended up using a customized shader and a ton of instance uniforms to achieve a similar effect to what you're describing. I have no idea how the performance worked out, but it did let me have basically just one material for most of the game.

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.

10 participants