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 GDScript support for defining virtual/prototype/must-override methods in base classes #1631

Open
tx350z opened this issue Oct 8, 2020 · 7 comments · May be fixed by godotengine/godot#82987

Comments

@tx350z
Copy link

tx350z commented Oct 8, 2020

Describe the project you are working on:
My project has a GUI containing a large number of inter-operating custom controls and related logic modules. There is a need for the various controls and logic modules to inter-communicate. Many controls need to implement the same methods but with different logic.

Describe the problem or limitation you are having in your project:
Due to parser cyclic reference errors I'm using call-groups as a way for the GUI controls and logic modules to intercommunicate. For example, my game has "power", "engines", "weapons" and several other "systems". All of the systems have a power button. When the power system's power button is pressed all other systems are notified of the event. This is done using call_group("pwr","pwr_mstr_pwr_btn_pressed"). Every system must implement the method to receive group call. However, each system has unique logic associated with the master power button press.

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
Add support to GDScript for method prototypes. There may be a need to also indicate a class is virtual/prototype so the parser/runtime will raise an error only on concrete classes.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
Possible options:

prototype func_name(arg:int)->String;

virtual func_name(arg:int)->String;

mustoverride func func_name(arg:int)->String;

If this enhancement will not be used often, can it be worked around with a few lines of script?:
The only work around I have found is to implement methods in base classes to programmatically check that required methods are implemented by descendant classes. This is a crude and rather ugly solution prone to errors and difficult to maintain.

Is there a reason why this should be core and not an add-on in the asset library?:
Not sure if it can be implemented as an add-on.

@Calinou
Copy link
Member

Calinou commented Oct 8, 2020

With the new GDScript syntax, this would be better implemented as an @virtual annotation rather than keywords.

We could also replicate the @const and @override annotations as used in the C++ API. (const is for methods that can't change the instance's member variables, override is for methods that must override a base class' method.)

@Calinou Calinou changed the title Add GDScript support for defining virtual/prototype/must-override methods in base classes. Add GDScript support for defining virtual/prototype/must-override methods in base classes Oct 8, 2020
@hilfazer
Copy link

hilfazer commented Mar 9, 2021

Every method in GDScript is virtual so let's use something else. That prototype sounds all right. There's also abstract that's used in Java and i think Smalltalk as well.

@me2beats
Copy link

me2beats commented May 29, 2023

I'm creating a plugin like the Command Palette.
Its main idea is to allow users to create their own commands easily.
Each command (I call it an action) is a gdscript file or gdscript class which extends Action class.
It has static func run() that should be overridden by the user. This function is called when the command is executed.
Also it has fields (properties) like id (uuid string), info (Dictionary with the command name, author, version and description) and these properties should also be "overridden"/set by the user.
Idk how implement it better than having must-override methods and properties, so I would really like to have this feature in GDScript.
It also could help us, developers, not allowing making mistakes like forgotten methods overrides

@dalexeev
Copy link
Member

At the moment you can use breakpoint, assert(false, "Message"), etc. in a base class method.

@me2beats
Copy link

Nice idea, but it still doesn't look intuitive for the user

@me2beats
Copy link

me2beats commented May 29, 2023

I even think some Godot classes can benefit from having this feature.
For example EditorScript's _run() function can be must-override

@ryanabx
Copy link

ryanabx commented Sep 4, 2023

I want to reopen the conversation on implementing support for these kinds of methods. If there were interest in this kind of feature, I'd be interested in implementing it. Some questions posed to me by @aaronfranke were:

Would abstract methods only be allowed on abstract classes? Or, would those methods just not be available unless overridden in a derived class, but the class itself could be instantiated and the other methods called?

Note: There is a PR currently open implementing abstract classes (godotengine/godot#67777). This PR uses an @abstract decorator for classes, and I think that the abstract methods should use an @abstract decorator as well.

In the context of that PR, I think that abstract methods should only be allowed on abstract classes. I also believe that if class A extends abstract class B, then class A should implement all the abstract methods of class B, or else it should also be declared abstract. If an abstract method is not properly implemented, then a warning should pop up on intellisense.

Would this be just a feature enabled in debug builds to help with better code, or would the check also happen in release builds?

In the PR Aaron created, abstract classes are only checked in debug builds, and I think that I could be convinced either way on this feature.

What do others think?

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.

6 participants