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

GDScript: Add keyword to access the current class #391

Open
Tracked by #15
ChibiDenDen opened this issue Jan 18, 2020 · 17 comments
Open
Tracked by #15

GDScript: Add keyword to access the current class #391

ChibiDenDen opened this issue Jan 18, 2020 · 17 comments

Comments

@ChibiDenDen
Copy link

Describe the project you are working on:
gdscript

Describe the problem or limitation you are having in your project:
No access to class from the following places:

  1. From static functions when unnamed Can't use new() or reference own class name in static function (fixed in master) godot#27491
    Mostly for factory methods trying to call new

  2. As type from within the same class when unnamed
    For example:

    • As return type for function
    • As variable type
    • etc..
  3. As type from within the same class even when named
    Allow scripts marked with class_name to use their class within the script without causing a cyclic dependency godot#25252

Describe how this feature / enhancement will help you overcome this problem or limitation:
The keyword will allow access to the “static_class” Type, thus allowing access to the class type without giving it a name
Show a mock up screenshots/video or a flow diagram explaining how your proposal will work:
Factory function with this_class return type, this_class local variable and access to this_class.new:

func make_with_color(color) -> this_class:
	var obj : this_class = this_class.new()
	obj.color = color
	return obj

Describe implementation detail for your proposal (in code), if possible:
This will be implemented as an additional keyword to the parser and compiler of gdscript
the keyword I used for my POC was "this_class" but I would be more than happy to hear any better suggestions.
See Commit for actual code:
ChibiDenDen/godot@2ff8c1b

If this enhancement will not be used often, can it be worked around with a few lines of script?:
The current “script” solution is to use the following expression to get the static class
load(“res://path_to_script_file.gd”)
Which is a hacky way, creates a dependency on the scripts path and looks bad
Furthermore, this is not documented and very hard for new users to find.

Is there a reason why this should be core and not an add-on in the asset library?:
This is part of gdscript syntax

@KoBeWi
Copy link
Member

KoBeWi commented Jan 18, 2020

What if we go full-Ruby and just make self.class (or similar) return the current class? No need for another keyword then.

@ChibiDenDen
Copy link
Author

since self should return an instance of the class self is not accessible from most of the cases:
static function
type specifier for functions (args / return value)

@Calinou
Copy link
Member

Calinou commented Jan 18, 2020

You can use get_script().new() to create a new instance of the current class. However, you can't use it in static functions as you pointed out.

@ChibiDenDen
Copy link
Author

I know, but this is not consistent, for example, in an inner class, would this line create an instance of the inner class, or of the top-level class in the script?
I feel like some method of referring to the class type is required, specifically since most top-level classes are anonymous, and since the class_name keyword is very noisy and limiting (makes the class a global class accessible from everywhere, not allowing self referencing)

@Xrayez
Copy link
Contributor

Xrayez commented Jan 19, 2020

If only get_script could be static somehow indeed. I've tried to simplify the process of instancing scenes. The script path can determine a scene path assuming both scene and script remain in the same directory:

static func instance():
    return load(get_script().resource_path.get_basename() + ".tscn").instance()

It means that I could instance a scene from within global class_name without hardcoding the scene path.

var player = Player.instance()

See also #260.

@me2beats
Copy link

Should a static function be called "static" in such a case?

@bluenote10
Copy link

the keyword I used for my POC was "this_class" but I would be more than happy to hear any better suggestions.

The problem with this_class could be that it is lower case and uses underscore. Typically types are PascalCase so a reader may get confused that this_class is supposed to refer to a type.

The same mechanism exists in Rust, and there it is called Self, which makes it very obvious that Self is the type of self.

@me2beats
Copy link

me2beats commented May 8, 2021

The problem I have now is I can't make these functions static and move them to my Utils class (NodeUtils):

func get_children_sorted_by_x(node:Control)->Array:
	var children = node.get_children().duplicate()
	children.sort_custom(self, '_sort_by_x')
	return children


func _sort_by_x(a:Control, b:Control):
	if a.rect_position.x<b.rect_position.x:
		return true
	return false

this is because of self in sort_custom()

I work-round it by creating SortUtils class and moving _sort_by_x() there so

node_utils.gd:

const SortUtils = preload("res://sort_utils.gd")

static func get_children_sorted_by_x(node:Control)->Array:
	var children = node.get_children().duplicate()
	children.sort_custom(SortUtils, '_sort_by_x')
	return children

sort_utils.gd:

static func _sort_by_x(a:Control, b:Control):
	if a.rect_position.x<b.rect_position.x:
		return true
	return false

this is not so convenient tho.

The thing I tried to achieve is reusable get_children_sorted_by_x() without inheritance or creating multiple objects. Also I don't want to use singletons for that.

@hilfazer
Copy link

hilfazer commented Aug 5, 2023

Another use case is handling the following warning:
The function "[...]" is a static function but was called from an instance. Instead, it should be directly called from the type: "Node.[...]"

It feels wrong to make a script require a global reference to itself or to preload itself in order to handle this warning.
#7447

@geekley
Copy link

geekley commented Dec 12, 2023

To avoid introducing keywords and breaking changes, I suggest the following:

  1. Why does get_script() return a Variant?
    If it can only be a Script (which is a nullable Object) it should return at least Script directly to avoid unnecessary type unsafe lines. Can this be changed without being considered a breaking change?
  2. Make preload() without arguments be considered equivalent to preload("<ThisFilename>.gd") so you don't have to specify (or depend on) the specific script's name. This solves this issue the cleanest way IMO.
  3. Please allow using preload() and preload("Foo.gd") on type hints directly: never mind this part
# TypeA.gd
const TypeA := preload() # or const Self := preload()
static func create() -> TypeA:
  return new()

What if we go full-Ruby and just make self.class (or similar) return the current class? No need for another keyword then.

If x.class syntax was implemented, I'd expect it to return something like the GDScriptNativeClass object itself, instead of the script (because we already have get_script(), which is different from get_class()). So basically same as get_class() but an object instead of string.

@AThousandShips
Copy link
Member

AThousandShips commented Dec 12, 2023

Why does get_script() return a Variant? ... Can this be changed without being considered a breaking change?

For legacy reasons AFAIK, and it can not, changing return types is breaking compatibility, see godotengine/godot#81734

Changing the return type at all should generally be considered compatibility breaking, but specifically changing to a non-compatible type is absolutely compatibility breaking (regardless of the behaviour, even if the code errors with an "invalid" type), in this case even with an error message feeding it an invalid type the error changes from a message to a compile error (as you now can't call the method at all)

@geekley
Copy link

geekley commented Dec 12, 2023

in this case even with an error message feeding it an invalid type the error changes from a message to a compile error (as you now can't call the method at all)

I don't understand why moving a correct error message from runtime to compile time is supposed to be a bad thing instead of a good thing. This would only lead to the developer being able to catch and fix hidden bugs, no?

@AThousandShips
Copy link
Member

AThousandShips commented Dec 12, 2023

It breaks compatibility? It goes from an error (which doesn't stop things or prevent you from compiling) to a compile time error, this very obviously breaks compatibility, as the code goes from "this compiles and works but errors" to "you can't run this code"

It is an improvement, but you are missing the point, this is about how that breaks compatibility 🙃

@geekley
Copy link

geekley commented Dec 13, 2023

Another syntax I thought would make sense for the current class/script object without having to introduce a new keyword is (static self). I think it makes intent very clear. For example:

const Self := static self
static var instance: Self = new()
static func get_instance() -> Self:
  return (static self).instance # == return Self.instance == return instance
static func this_script() -> GDScript:
  return static self # == return Self

Could also be static.self but I think the dot is unnecessary. It's better to not use any PascalCase keywords, but you can still assign it yourself as a type alias (const) like Self, ThisClass or whatever you prefer.

The difference between this and my preload() without arguments suggestion above would be when it's used in inner classes, where static self would refer to the inner class' Script object while preload() would still return the outer class' GDScript object regardless of where it appears in the file.
Arguably, with preload() you wouldn't also need static self because you can already refer to inner classes by name. But it would be useful if they ever decide to implement something like anonymous class expressions for GDScript in the future.

@geekley
Copy link

geekley commented Dec 13, 2023

If only get_script could be static somehow indeed.

For the top-level class you can use preload with the filename to define a type alias which even works as type hints. But you can't add the type hint in the const itself like : GDScript if you intend to access members (e.g. static callbacks) or you might have issues - you have to use :=.

const Self := preload('this_filename.gd')
func get_this() -> Self:
  return self

If you don't want to depend on the filename, there's a workaround, but it's a bit iffy (tested on 4.2.1). It's working even on static functions to get the class (or inner class) at that location in the code. You just can't use it on type hints, since it can't be const. It works as a local variable too, even in non-static methods, it seems it'll return the class, not self.

static var Self: GDScript = (func() -> void:).get_object() as GDScript

@pineapplemachine
Copy link

I recently stumbled on this myself. @geekley thank you for the preload workaround! This feels pretty clunky, but at least it lets me still use type annotations in my code.

A general way to refer to the class defined within the current file would be great. I would ideally like to be able to generically refer to the type of self or any other variable via something like typeof(self) for use in type annotations, rather than introducing a keyword for the sole purpose of referring to the type of self. But even just being able to refer to the class via its own class_name in type annotations would be a great start.

@jamonholmgren
Copy link

My use case for this is as follows:

class_name LobbyPlayer extends Node

var me: LobbyPlayer = null

def _enter_tree():
  LobbyPlayer.me = self

However, this is designed to be subclassed.

class_name MyPlayer extends LobbyPlayer

I would use this elsewhere as a singleton; after I create one with:

var my_player = MyPlayer.new()
add_child(my_player)

... then I can access it anywhere with

MyPlayer.me

The problem is ... it'll be typed as LobbyPlayer, not MyPlayer. :(

Not sure if there's another way to handle this?

Image

But having the ability to type as "current class" which works with subclasses would be fantastic and help with this.

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.