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

Allow passing in packed scene or node to variable as long as it has that script. also allow exporting it #2699

Closed
Shadowblitz16 opened this issue May 8, 2021 · 8 comments

Comments

@Shadowblitz16
Copy link

Shadowblitz16 commented May 8, 2021

Describe the project you are working on

Spaceship game

Describe the problem or limitation you are having in your project

I need to export a certain type of packed scene which I went on discord to get help with..
@nisovin suggested I use something like this..

func is_packed_scene_of_type(packed_scene, type):
	var scene_state = packed_scene.get_state()
	for i in scene_state.get_node_property_count(0):
		if scene_state.get_node_property_name(0, i) == "script":
			return type == scene_state.get_node_property_value(0, i)
	return false

however using it creates a cylindrical refrence error and requires a tool property

tool
extends Resource

export var projectile        : PackedScene setget _set_projectile
export var projectile_cost   : float
export var projectile_power  : float
export var projectile_speed  : float
export var projectile_rapid  : float
export var projectile_range  : float
export var projectile_energy : float

func _set_projectile(value):
	if Util.is_packed_scene_of_type(value, Projectile):
		projectile = value 

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

Can we have the ability to export class and class_names which take in a Node or PackedScene as long as it has that script?

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

extends Area2D
class_name Projectile
#or
class Projectile extends Area2D:
#takes both packed scene and node as long as it has a Projectile script
export var projectile : Projectile

This might require a api change on how packed scenes are instanced but I think it would benefit godot

Unity3D has something similar to this with MonoBehaviors and GameObjects where you can pass in GameObjects to a exported MonoBehavior as long as it has it.

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

It would be used a lot.
also I'm not sure but its not easy to do if it can be worked around

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

It makes godot easier to use

@Shadowblitz16 Shadowblitz16 changed the title Allow exporting class_name which takes both packed scene or node Allow exporting class_name or class which takes both packed scene or node May 8, 2021
@Shadowblitz16
Copy link
Author

Note this would also work on non exports

@Shadowblitz16 Shadowblitz16 changed the title Allow exporting class_name or class which takes both packed scene or node Allow passing in packed scene or node to variable as long as it has that script May 8, 2021
@Shadowblitz16 Shadowblitz16 changed the title Allow passing in packed scene or node to variable as long as it has that script Allow passing in packed scene or node to variable as long as it has that script and exporting it May 8, 2021
@Shadowblitz16 Shadowblitz16 changed the title Allow passing in packed scene or node to variable as long as it has that script and exporting it Allow passing in packed scene or node to variable as long as it has that script. also allow exporting it May 8, 2021
@Shadowblitz16
Copy link
Author

@dalexeev can you provide why you are confused about this?
you seem to do this with all my suggestions which is making me think your being a bit biased

@Shadowblitz16
Copy link
Author

these are related
godotengine/godot#7821
godotengine/godot#21875
#1048

@dalexeev
Copy link
Member

dalexeev commented May 8, 2021

@dalexeev can you provide why you are confused about this?
you seem to do this with all my suggestions which is making me think your being a bit biased

Because there is a mixture of different types. Don't mix PackedScene with types that don't inherit it. This is against OOP and I won't even explain why.

You are trying to solve the same problem of linking scenes and scripts in different ways and have already created about 5 proposals on this topic, but IMO they are bad. I like willnationsdev's suggestion much more.

No, I'm not biased against you, most of your proposals have not been rated by me, and even, as far as I remember, there are one or two that I rated positively. Yes, I noted for myself that you make a lot of insufficiently thought out proposals (IMO), but I always try to be objective, and if I'm not sure that your proposal is bad, then I simply don't rate it. After all, even if suddenly I'm the only one to be biased against you, it won't be a problem if you can convince others that you are right. After all, I'm not even a maintainer, my opinion is not taken into account somehow especially. I am not conducting an agitation campaign against you, calm down. 😃

@Shadowblitz16
Copy link
Author

@dalexeev I wasn't angry 😃

I don't like willnationsdev's suggestion because it forces us to use a annotation in the script.

if implicitness is what you don't like we could have a generic type for it. although I'm not sure what to call it. maybe NodeRef?
I just like the implicit version better because it requires less typing and seems to be somewhat understandable even though it is implicit.

its like saying as long as the root of the scene or the node has that script it is that type because it contains the logic and interface
I know godot doesn't use inheritance for node's and instead uses composition but the composition looks like inheritance.

class name extends type

@dalexeev
Copy link
Member

dalexeev commented May 8, 2021

PackedScene is a Resource and Node is not a Resource. These are different branches of the inheritance hierarchy, and since multiple inheritance is not used, these branches cannot intersect. If GDScript supported parametric types, it could be PackedScene<Node> for a scene with a root node Node, and PackedScene<MyNodeType> for a scene with a root node extending Node. And then PackedScene<T>.instance() would return type T. (Scripts in Godot are actually resources, not complete types, but from the point of view of GDScript, we can not think about it.)

@Shadowblitz16
Copy link
Author

Shadowblitz16 commented May 8, 2021

I guess that is true.
how could we support typed packed scenes without parametric types?
I know not all languages support them

EDIT:
I guess we could always either instance the packed scenes when passed or pack the nodes when passed
and use either duplicate or instance

EDIT2:
It seems to make more sense to just unpack the scene since it would be a instance of that script

@Shadowblitz16
Copy link
Author

I'm closing my bad proposals

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

3 participants