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 the ability to export sub classes #9343

Open
sydist opened this issue Mar 20, 2024 · 9 comments
Open

Add the ability to export sub classes #9343

sydist opened this issue Mar 20, 2024 · 9 comments

Comments

@sydist
Copy link

sydist commented Mar 20, 2024

Describe the project you are working on

I'm working on an rpg where I want different characters to have different "indicators" for when they're talking with the text box.
Maybe one character would float, another would switch to a animated sprite where the mouth moves, etc....

Describe the problem or limitation you are having in your project

Like any programmer I pull out the Strategy pattern, I would like to export the variable of type "TalkStrategy" to the property editor.
However, I can't do this because resources only appear in the property editor if they're in their own script file!
So I would have to create seperate script files when I need them to be consolidated in one place (the npc class, since only npcs will ever talk and only they will ever have the talking strategy)

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

Instead I would rather be able to inline classes inside the npc class that define the behaviour for different types of talking strategies, and be still be able to select those from the editor.
That way everything is encapsulated neatly.

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

@export var talk_strategy: TalkStrategy

func animate_talk():
    talk_strategy.talk(sprite)

class TalkStrategy extends Resource:
	func talk(_sprite: Sprite2D):
		pass


class TalkStrategyAnimation extends TalkStrategy:
	@export var animated_texture: AnimatedTexture

	func talk(sprite: Sprite2D):
		if sprite.texture != animated_texture:
			animated_texture.current_frame = 0
			sprite.texture = animated_texture


class TalkStrategyFloat extends TalkStrategy:
	@export var speed: float = 1.0

	func talk(sprite: Sprite2D):
		sprite.offset.y += sin(Time.get_ticks_msec())

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

I wasn't able to find a way to work around this.

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

It's intuitive for the property editor to be able to detect sub classes of the node.

@AThousandShips
Copy link
Member

This would be very complicated and have very limited use I'd say, implementing this would be difficult and error prone, and would not be used by the vast majority of people

@sydist
Copy link
Author

sydist commented Mar 20, 2024

I believe it's very important for the clarity of code.
Having to make seperate files for logic that belongs to another class causes confusion in the code base.
Unless there's a way to avoid this and I'm missing it.

@AThousandShips
Copy link
Member

AThousandShips commented Mar 20, 2024

If they're internal to the class they are internal, no? That would logically mean they're not to be exported IMO

I'd say that if this is a serious issue for you you might need to rethink how you structure things, I'm not sure your data is well structured, rather than this being an issue with the engine or missing feature

@sydist
Copy link
Author

sydist commented Mar 20, 2024

I understand your point about internal class elements not needing to be exported. However, my concern is more about the organization and readability of the code. When a class grows large, having related logic in separate files can make it harder to navigate and maintain the codebase. Now include the amount of times you need a strategy pattern "exported" and it just becomes a mess.

Essentially it's a less annoying version of exporting an enum and doing a switch statement that links the enum with the strategies

@AThousandShips
Copy link
Member

AThousandShips commented Mar 20, 2024

I'd argue the opposite is true, having exported details not be in dedicated files makes it impossible to find them

I'd suggest exporting the relevant data directly instead

@dalexeev
Copy link
Member

I think there was a miscommunication here. Probably @sydist did not mean that the exported variables of the inner class should somehow be displayed in the inspector of the outer class (without instantiation the inner class as a property of the outer class). I think that @sydist meant a bug that the inspector cannot work with non-global classes (GlobalClass is ok, but not GlobalClass.InnerClass, "res://unnamed_class.gd" or "res://unnamed_class.gd".InnerClass). This is a problem, we don't have a unified type system or at least a universal FQTN (fully qualified type name) that the inspector and other parts of the engine can work with.

@hilfazer
Copy link

The concept OP means isn't a sub class but rather an inner (a.k.a nested) class.

@sydist
Copy link
Author

sydist commented Mar 22, 2024

i did not even notice the inner classes had @exports in them. LOL.
completely my fault, my bad.
and yes I do mean inner classes (although im not sure what sub classes are in this case then, i thought they meant the same thing.)

@romlok
Copy link

romlok commented Nov 30, 2024

I've just hit this desire too, for setting multiple attribution links on the credits screen of my game:

class_name CreditsPerson
extends Control

...

class Link extends Resource:
	var title : String
	var uri : String

@export var links : Array[Link] = []

Right now, adding items to the links export in the inspector just gives a list of all Resource subclasses, not including the internal Link I want to reference. To get it to work as I want, I have to move the three lines into a separate file, which is more cognitive/maintenance burden than having it in-line in the only place it's used!

yes I do mean inner classes (although im not sure what sub classes are in this case then, i thought they meant the same thing.)

In case you're still not sure, in my example above Link is an inner class of CreditsPerson (relating to where it's defined), and a sub-class of Resource (relating to what it is).

So I think the title of this issue should probably be changed from "sub classes" to "inner/nested classes".

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

5 participants