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

"export var" for custom types/classes #359

Closed
Vitrebenki opened this issue Jan 5, 2020 · 20 comments
Closed

"export var" for custom types/classes #359

Vitrebenki opened this issue Jan 5, 2020 · 20 comments

Comments

@Vitrebenki
Copy link

Vitrebenki commented Jan 5, 2020

Describe the project you are working on:
I am working on turn-based RPG. I am trying to implement complex non-standard Stats system for my game, and I want to have possibility to edit stats via Godot Inspector

Describe the problem or limitation you are having in your project:
Currently, we can use "export var" for build-in and native resources type only.
But it will be great to have the possibility to use "export var" for custom type/class to provide the possibility to edit it via the inspector.
Example from Unity(video describes this feature really well):
https://www.youtube.com/watch?v=gmXChf5PPRs

Describe how this feature / enhancement will help you overcome this problem or limitation:
This feature looks logical and it provides the possibility to edit exported custom class variables via the inspector. It should simplify complex stats editing in my RPG game.

Show a mock up screenshots/video or a flow diagram explaining how your proposal will work:
Export should work "recursively" and display exported var as some kind of tree(see "Transform" property in the inspector as an example).
Watch video https://www.youtube.com/watch?v=gmXChf5PPRs

For example, I have DefenceStat class with the following exported properties:

extends Object 
class_name DefenceStat
export var current_value : int 
export var max_value: int
export var regen: int

And I should have the possibility to export DefenceStat variable like this one:
export var fire_defence: DefenceStat

And it should be displayed in inspector somehow like this(it is similar to Transform property):

fire_defence
    current_value 0
    max_value 0
    regen 0

For more complex variables export tree will contain more sub-levels.. but it still a tree and finally all custom classes use base data types and combine them in more complex structures and it can be displayed like a tree in the properties editor.

Describe implementation detail for your proposal (in code), if possible:
We should allow importing custom class properties and use existing editor features(it may display tree-like structures) to show custom class variables(like in Unity).
Unfortunately, I can't provide code( Still investigating godot source code.

If this enhancement will not be used often, can it be worked around with a few lines of script?:
Actually we can use custom Resources for similar functionality.. but it has some limitations and problems due to the nature of resources. Exported Resource is Null/empty by default and it is impossible to set a default value for the Resource type(so need to make a few clicks and mouse scrolls to create required Resource in the inspector). Resources are "global" by default so I need to make sure that all resources are "unique" and changing data in one object doesn't affect other objects that use the same resource.

Is there a reason why this should be core and not an add-on in the asset library?:
This functionality looks like base game engine/editor functionality and it is available in Unity and it looks like good feature and it shouldn't be hard to implement, because all required functionality exists in current Godot editor.

@willnationsdev
Copy link
Contributor

This is basically a duplicate of #18 which my PR godotengine/godot#32018 already fulfills.

The only Object type that can be exported, at all, is a Resource type, because only Resources can automatically serialize themselves (and exporting means to serialize to the scene file, i.e. the PackedScene resource).

The mentioned Proposal is already proposing to add resources as "first-party" types that can be exported just like Godot's default classes.

And I should have the possibility to export DefenceStat variable like this one:
export var fire_defence: DefenceStat

My pull request that I mentioned showcases this scenario functioning, assuming your script were to extend Resource and not Object.

Exported Resource is Null/empty by default and it is impossible to set a default value for the Resource type

Actually, they can. If you preload a resource, then it will appear directly in the Inspector as the default value, like so:

export(StreamTexture) var tex := preload("icon.png")

Resources are "global" by default so I need to make sure that all resources are "unique" and changing data in one object doesn't affect other objects that use the same resource.

All you have to do is check on the "Local to Scene" property at the bottom of the Resource's sub-inspector within your scene, and then every single instance of the scene will get its own instance of the Resource. Is there something about this workflow that you don't like? Something that could be improved? Maybe you would prefer the default to always be local to scene (in which case, maybe the setting could be toggled one way or the other in ProjectSettings?)?

@Vitrebenki
Copy link
Author

Vitrebenki commented Jan 6, 2020

Tested "Local to Scene" without this change-list godotengine/godot@7160f66 in default Godot 3.1 using test project https://github.com/LaRusCat/ExportVar. I will try to test my project with Your changes-list soon.

Unfortunately "Local to Scene" doesn't work correctly in "complex" cases. I tested it with using a resource inside resource and all values syncs after changing value and saving(ctrl+s) or after switching to another file and back... it looks like a bug. Video with ExportVar project: https://youtu.be/pfk5mliLOZ4
I used following syntax:
export var health_attack : Resource = preload("res://AttackStat.tres")
Possible it is incorrect syntax(for required behavior) and I have preloaded the same resource so it was shared, but I don't know another way to preload my Stat resource and make it non-shared.

There is button "Make Sub-Resources Unique" in Godot editor
MakeSubResourcesUnique
"
And it solves this issue. But anyway it doesn't look like "best practice" and the nature of Resources makes it harder to implement and required a few additional steps(and if these steps were missed then You can get a strange bug with synced/shared Stats or some other Resource specific behavior that isn't required)
"Local to Scene" for ALL resources in the project(ProjectSettings) isn't the best solution too, because some resources should be shared("Shaders, Sprites, etc.") and some shouldn't(Stats and some other user-specific things)

I am not sure that Resource usage is a good solution for my issue.
And actually it is not a critical issue, because there a few other ways to do it. For example, using Nodes tree. Each sub-stat will be Node and I will be able to edit tree of stats in nodes editor and inspector. But Node is "scene object" and Character stat actually isn't "scene object" it is "logical object".
But anyway it will be great to have similar with Unity functionality form this video https://www.youtube.com/watch?v=gmXChf5PPRs without Resource-specific behavior.

@willnationsdev
Copy link
Contributor

Seems like you'd either have to introduce some sort of exportable struct concept to GDScript or fix the issues / improve the UX of using resources for this purpose. With the current design, the only way to serialize data to a file is with a resource, and nodes are serialized by the PackedScene resource which knows how to iterate through nodes' properties and store them. So, unless you added a new user-defined value-type (that wouldn't share references but is still typed, i.e. a struct type) and taught PackedScene how to store those values, you wouldn't be able to use just a Node; it would need to be a user-defined Resource, in which case, the solution would be fixing the problems with that data type (which I actually think would probably be a better solution all around since people run into these syncing/sharing issues quite often).

@Vitrebenki
Copy link
Author

"exportable struct" is a good name. Currently, the Dictionary can be used for this purpose but it doesn't provide the possibility to define functions and signals inside it.
Will try to use Resource and fix issues with resource sharing.
Thanks for the detailed explanation!

@Shadowblitz16
Copy link

Shadowblitz16 commented Mar 13, 2020

why is this labeled as a topic?
of course this should be added

static typing ensures that people don't mess up the project and help the devs to know what type it accepts.

extends Resource // or Any Buildin Type
class_name MyType1

//All these should be possible
export(MyType1 ) var test1                  //same as below
export(Resource) var test2 : MyType1        //same as above
export(Array, MyType1 ) var test1           //same as below
export(Array, Resource) var test2 : MyType1 //same as above

@willnationsdev
Copy link
Contributor

why is this labeled as a topic?

Some labels have an explicit purpose (like cherry-pick:<version>, <version>, or pr meeting. Others exist purely for organizational purposes so that people can find Issues related to the same topic. Because this proposal is related to GDScript as a language, it has been given the topic:gdscript label. There's no judgmental/evaluative effect of topic:* labels.

static typing ensures that people don't mess up the project and help the devs to know what type it accepts.

This is true, but the examples you've written below are not actually comparable.

extends Resource // or Any Buildin Type
class_name MyType1

//All these should be possible
export(MyType1 ) var test1 // dynamically typed, but scene-assigned value is of type MyType1
export(Resource) var test2 : MyType1 // statically MyType1, but scene-assigned value is of type Resource
export(Array, MyType1 ) var test1 // dynamically typed, but scene-assigned value is an Array of MyType1 resources
export(Array, Resource) var test2 : MyType1 // statically typed. Must be a MyType1 instance. 
// ^ Scene-assigned value is an Array of resources. Will have runtime error, if not parse-time error.

The type you designate for export simply tells the Godot Editor what type of value it can assign to the property. The type hint after the variable name tells you what the static type is. For example, with dynamically typed variables, I can make the scene-assigned value be used as part of the script-assigned value:

export(NodePath) onready var my_node = get_node(my_node)

You assign a NodePath in the Inspector. When the game runs, the scene is instantiated, initializing the variable with the serialized value from the scene file. Then just before the _ready() callback, the variable's script initialization occurs, assigning the assignment operation to the variable. That operation makes use of the variable's current value, meaning that it uses the value provided by the scene configuration. Even though the two types are NodePath and Node, respectively, it is possible because the variable is not statically typed since it has no type hint.

@Calinou
Copy link
Member

Calinou commented May 16, 2020

Duplicate of #18.

@Shadowblitz16
Copy link

Shadowblitz16 commented Oct 25, 2020

@Calinou #18 is about resources no?
I think LaRusCat wants to export classes without needing to pass a resource

@KoBeWi
Copy link
Member

KoBeWi commented Oct 25, 2020

What LaRusCat describes can be achieved with Resources. The proposal would need to describe a use-case where Resources can't be used and you need to export a non-resource type, otherwise it's covered by #18

Seeing how this was closed 5 months ago and the OP never commented, it can be assumed they agree.

@Shadowblitz16
Copy link

Shadowblitz16 commented Oct 25, 2020

@KoBeWi I mean its still a valid issue. people are probably not going to want to create a new resource for every custom parameter

I for one would not like to create a new issue if this one already covers it

Unity has this because it is useful godot needs it too.
image

@KoBeWi
Copy link
Member

KoBeWi commented Oct 25, 2020

Ok so I actually watched the video linked in the OP and it definitely looks like serializable objects in Unity are just Godot's resources. Godot just handles them a bit worse xd For example, seems like you can't create empty resources using export, only exporting with preload works (which doesn't allow to create unique resources). Also typed arrays don't exist yet (I think), exporting resource array should automatically create resource for each element. If this was changed, you could achieve the same thing as in Unity.

This is as close as I could get in 3.2.3
image

@Shadowblitz16
Copy link

@KoBeWi the issue is hiding P 0, P 1, and P 2 so they can't be messed with but still having the Pos and Dir Fields

@KoBeWi
Copy link
Member

KoBeWi commented Oct 25, 2020

That's not something non-resource classes would fix. We'd need a more customizable inspector for that.
I quickly looked over inspector proposals and found this #852 heh
There's also #123

@godotengine godotengine deleted a comment from sttifer Mar 10, 2021
@DriNeo
Copy link

DriNeo commented May 13, 2022

I want to use resource based class for my game save, this game save contains another custom resource (one for each opponent). This limitation was a bad surprise.

@stephanbogner
Copy link

stephanbogner commented May 22, 2023

I know this is an old thread, but has custom resources been a solution to people here?


Context:
What I wanted to build is a flexible event system (e.g. "start cutscene mode" then "wait 1s" then "start dialog XYZ" then "end cutscene mode").

Approach with custom resources:
Technically I had in mind to use a custom EventChain-Node that exports an array of custom Event-Resource which allows the user to choose the desired event:
grafik

This seemed like a good approach until I noticed that Resources might not be suitable for this (e.g. can't use timers, as they are not part of the node tree).

Approach with node tree:
My new approach uses the normal node-tree for the same approach but that seems kinda dirty 😅 ... but maybe I thinking about this completely wrong.
grafik

@abcjjy
Copy link

abcjjy commented Oct 6, 2023

Resource can't reference node meaningfully. For example, node class A has a resource B export field. B holds a reference to child node. When A is instantiated, the B field of the new instance is still holding reference of the node in PackedScene.

AFAIK, resource should only reference builtin types or other resources. That's why we need to export custom classes.

In Unity, gameobject reference in serializables will be updated automatically when instantiating.

@SpaceJellyfishDev
Copy link

Stride, Flax, Unity... almost all engines export class and struct, even like 10 years ago.
I hope Godot can someday keep up with modern engines, without requiring Resource.
I find Resource work-around is kind of buggy, if the node containing the Resource has [Tool] attribute that tries to update the values in Resource, the values would not update and keep blinking.

@Calinou
Copy link
Member

Calinou commented Oct 27, 2023

Please continue the discussion in #7329. Implementing structs needs to be done before they can be exposed in the inspector.

@Vitrebenki
Copy link
Author

Vitrebenki commented Oct 28, 2023

Unfortunately, new structures from #7329 have limitations. They have no methods at least.
Don't understand why Godot can't provide the possibility to export types/classes( but just propose different workarounds with different limitations.
Unity can do it and Godot should be able to do it. This functionality looks logical and can be used to simplify editing of exported variables from the editor at least.

@ShirenY
Copy link

ShirenY commented Dec 23, 2023

Agree with @Vitrebenki, this proposal shouldn't be closed. Although a struct in GDScript can be serialization and showing in inspector is great. But people may still want a fully featured 'class' can be export in inspector in same way.
And other languages need exportable custom types too. As OP said, the inspector should be able to handle nested types recursively.

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

No branches or pull requests

10 participants