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

Swap the names of the engine's internal "abstract" and "virtual" #11791

Open
aaronfranke opened this issue Feb 17, 2025 · 4 comments
Open

Swap the names of the engine's internal "abstract" and "virtual" #11791

aaronfranke opened this issue Feb 17, 2025 · 4 comments
Labels
breaks compat Proposal will inevitably break compatibility topic:core
Milestone

Comments

@aaronfranke
Copy link
Member

aaronfranke commented Feb 17, 2025

Describe the project you are working on

Godot itself.

Describe the problem or limitation you are having in your project

Not a limitation with a project, just something that I think is needlessly confusing.

Godot has macros for registering "abstract" classes and "virtual" classes.

  • Abstract classes cannot be instantiated period, and cannot be extended by script or GDExtension.
  • Virtual classes can be instantiated by the engine on a technical level, so they can be extended by script or GDExtension. However, in every other regard, they appear to the user the same as abstract.

Now, on the C++ side. C++ has a keyword called virtual that indicates a function should be a part of the vtable, a table of function pointers, which allows derived classes to override the function. C++ provides 2 options here:

  • A virtual function can be given a default implementation that is used if the function is not overridden.
  • A virtual function can have = 0; after its definition to indicate it has no default implementation. Without an implementation defined, the class cannot be instantiated, so this means it is required to be overridden in defined classes.

In typical C++ terminology, a function with = 0; is called a "pure virtual function". However, Godot calls a class with these "abstract". Godot had these first before the other one was added in Godot 4.0 for GDExtension.

In the future, with PR godotengine/godot#67777, the plan is to add an abstract keyword to GDScript. This will make a class abstract, but this is equivalent to what Godot calls "virtual", since the engine can still technically instantiate the class, and it can be extended by other scripts.

Basically, the problem is that these names seem to be completely swapped. A class with "pure virtual" being called "abstract" makes sense on its own, but since we have 2 different types of abstract classes, we should use the word "abstract" for what users expect abstract classes to be (classes that can be extended), and use the word "virtual" to match C++ pure virtual.

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

Swap the names of the internal GDREGISTER_ABSTRACT_CLASS and GDREGISTER_VIRTUAL_CLASS macros, and any other mentions of "abstract" and "virtual". In the end, "abstract" should be one that can technically be instantiated by the engine, can be extended in script and GDExtension, and matches GDScript abstract. While "virtual" should be the C++ "pure virtual" that cannot be instantiated by the engine, cannot be extended by script or GDExtension, and is the one that differs from GDScript abstract.

This will break compatibility for some third-party modules and GDExtensions, but it won't affect user scripts and projects.

Also note, when I first opened PR godotengine/godot#67777, I initially used the keyword "virtual" to match the engine's macros. However, feedback from users has been overwhelming that "abstract" is a better name. And you know what, I completely agree. I wasn't sure when I opened the PR, but the more I think about it, the more I am convinced that the word "abstract" is better to use for things that are actually extendable by user scripts or GDExtension, and we can use "virtual" for the C++ pure virtual stuff that can't be instantiated by the engine due to internal technical limitations.

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

The code change is to rename things, so this question doesn't really apply.

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

It can be easily worked around. You can just ignore the problem and use the current names.

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

This is a core part of the engine.

@aaronfranke aaronfranke added this to the 4.x milestone Feb 17, 2025
@ArchercatNEO
Copy link

As a C# user which has clear definitions of virtual/abstract it seems like godot internal abstract classes do not behave like good abstract classes which lead to this confusion.

To me "abstract class" means can be extended by any class, but not able to be instantiated in any way.
A "virtual class" means a class that can be extended or instantiated.
A "sealed class" (not present here) is a class that can be instantiated but not extended by any class

Then for methods
A "virtual method" is a method that is part of the object's vtable and can be overriden by any derived class. These methods have a default implementation.
An "abstract method" is a virtual method with no implementation. This means a vtable cannot be constructed so the class it is a member of must be abstract.

If we look at godot's definitions of abstract/virtual there are 2 sides: engine and user (scripting/gdextension)

At the engine level

  • Abstract classes are abstract classes within c# terminology
  • Virtual classes are just standard classes

At the user level

  • Abstract classes fit no definition. They cannot be instantiated (abstract) and cannot be extended (sealed) this combination is confusing, unhelpful and doesn't correlate with wider use of "abstract"
  • Virtual classes are abstract classes because they can be extended but not instantiated

This is where confusion lies. abstract/virtual mean different things at the engine side and user side.

My suggestion is that instead of swapping the names at engine level (to confuse everyone with prior understanding of these terms) we rename how they are presented to the user. Engine side current use of abstract/virtual is preserved.

In node type selection (and any other significant UI locations)

  • Abstract is renamed to "internal" (or other term with less overloading barring virtual)
  • Virtual is renamed to "abstract"

This way the engine keeps it's internal abstract/virtual separation in line with standard use of abstract and users get class keywords that more closely align with how abstract is used in wider programming.

Unfortunately I am not aware if/how the GDREGISTER_ABSTRACT_CLASS macros are exposed in gdextension and how they are then exposed to scripting so instead I propose that abstract/virtual should preserve their definitions within a context (engine/gdextension/scripting) and at the point of contact be renamed to fit what the other context expects of such classes.

@AThousandShips
Copy link
Member

AThousandShips commented Feb 17, 2025

This will break compatibility for some third-party modules, but it won't affect user scripts, projects, or GDExtensions.

How will this not break compatibility with GDExtension? It uses the same system of the virtual/abstract, you can register abstract classes in GDExtension as well

@aaronfranke
Copy link
Member Author

@AThousandShips Oh, never mind. Since those ClassDB APIs exist, yeah this would break compatibility there.

@aaronfranke aaronfranke modified the milestones: 4.x, 5.0 Feb 17, 2025
@aaronfranke
Copy link
Member Author

aaronfranke commented Feb 17, 2025

Another option would be to choose new names instead of simply swapping the names. For example, the "engine internal can't be extended" classes can have the macro named GDREGISTER_PURE_VIRTUAL_CLASS. Or we could use names like "virtual abstract", "unextendable abstract", "extendable abstract", "sealed", "final", etc. If we use a new name like this, we can partially migrate to it without breaking compatibility, by allowing both old and new names.

@dsnopek dsnopek added the breaks compat Proposal will inevitably break compatibility label Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks compat Proposal will inevitably break compatibility topic:core
Projects
None yet
Development

No branches or pull requests

4 participants