Implement ClassDB singleton

[Daylily-Zeleen]

Implement ClassDB singleton.

Relate to this issue.

And where can I find the code style for godot?

[DmitriySalnikov]

SOUECE

typo?

[Daylily-Zeleen]

Yes, thanks for your warning.

[DmitriySalnikov]

I can approve it, but I don't really have any permissions here😅
At least class_get_property_list works

But actually I would probably just rename classes/ClassDB to something like ClassDBImp if possible. This would allow me to exclude this file from the build if necessary, as I am doing now with the rest of the classes in the classes/* folder. But I am not from the core team to solve such issues.

[Daylily-Zeleen]

@akien-mga , please review and consider to merge this pr, I already overhaul this pr multi times because of the changes from later commits.

[dsnopek]

Thanks!

I'm definitely in favor of exposing the "real" ClassDB - being able to get metadata about classes declared outside of the current GDExtension would be very useful.

Assuming I'm understanding correctly, the approach in this PR is to generate usual class declaration for ClassDB, but do it as a preprocessor macro, which can then be expanded in the hand-written ClassDB that we already have.

I'm not so sure about this approach for a couple reasons:

1. The changes to bindings_generator.py are pretty large, just to accommodate this one case.
2. Functions that are static when writing a Godot module (ex. ClassDB::get_class_list()) become members of the singleton in godot-cpp (ex. ClassDB::get_singleton()->get_class_list()). Ideally, the code would be the same for both Godot modules and godot-cpp.

As an alternative: what if we changed binding_generator.py to generate ClassDB under a different name, for example, ClassDBSingleton and then added static methods to the hand-written ClassDB to forward them? So:

static PackedStringArray get_class_list() {
    return ClassDBSingleton::get_singleton()->get_class_list();
}

If we wanted, these "forwarding functions" could be also generated into a preprocessor macro by binding_generator.py similar to what this PR is doing, and then expanded into the hand-written ClassDB in the same way.

I think this would require fewer changes to binding_generator.py, because we're just swapping a name, and adding some new code to generator (as opposed to changing how some existing code is generated). And it would keep the API similar between Godot modules and godot-cpp.

What do you think?

[dsnopek]

What does DECLEAR mean? Or, is this a type-o of DECLARE?

[Daylily-Zeleen]

Yes, it should be DECLARE, my terrible English.

[Daylily-Zeleen]

@dsnopek the principle of this pr is like your description.

The reson why ClassDB in GDExtension is be implemented as singleton in this pr is due to my misunderstand. At that time, I had no use ClassDB in module. Now I know it should work as "static class" style.

I think this would require fewer changes to binding_generator.py, because we're just swapping a name, and adding some new code to generator (as opposed to changing how some existing code is generated).

Generated preprocess macros similar to normal codes of other classes, but they are alaway have a "" character at the end of line.
In order to reuse generating code (hard-coded strings in binds_generator.py), I must use a independent method to append new line, I name it as append_line_method, a lambda to append line.

def append_continue_line(lines, line="", slash_pos=200, tab_len=4):
    tabs_len = 0
    for c in line:
        if c == "\t":
            tabs_len += tab_len - 1
    for i in range(len(line) + tabs_len, slash_pos):
        line += " "
    lines.append(line + "\\")


def append_line(lines, line="", slash_pos=200, tab_len=4):
    lines.append(line)

For ClassDB, append_line_method will be append_continue_line, for other class, it will be append_line.

In other word, the most of changes of binds_generator.py is the change of lines.append("xxx") to append_line_method(line, "xxx").

[Daylily-Zeleen]

Maybe I should generated preprocess macro like other classes normal code first, then append "\" character for lines after generating ClassDB code finished.

Anyway, I will refactor this pr again.

[Daylily-Zeleen]

And it would keep the API similar between Godot modules and godot-cpp.

@dsnopek I found that ClassDB is generated as a singleton in extension_api.json.

Classes which in @GlobalScope in GDScript, some are singletons in godot source code, like ResourceUID, Engine , some are work as a static class, like ResourceLoader, ClassDB.
In normal use case, they only have a different of get_singleton()->, I don't know the reason why they are designed like this, I don't have much professional knowledge about this. What I want to say is that they don't have consistency at the source code level, and they are unified by binding to ClassDB.

Is it means that GDExtension should work like GDScript, not godot module? Because extension_api.json is generated by using ClassDB in core side, and only contain bound apis.

If we want some classes work with a "static" style in GDExtension like in godot module, refactor the logic of generating extension_api.json, let their methods to be marked as static is more reasonable.
It will be a huge change, the topic of "should GDExtension work more like godot module?" need more conversations between core develop members.

[DmitriySalnikov]

What about generating ClassDB as a regular singleton, and rename static ClassDB from GDExtension to something else?
In this case, ClassDB will be used like all other singletons (ClassDB::get_singleton()->do_something()), and there will be no need to perform hacks to combine these classes.

ClassDB - as in GDScript
ClassDBStatic::/CDB::/GDE:: - for registration and bindings

This will require renaming in all existing libraries, but it will allow us to use ClassDB as in GDScript.

[dsnopek]

For ClassDB, append_line_method will be append_continue_line, for other class, it will be append_line.

In other word, the most of changes of binds_generator.py is the change of lines.append("xxx") to append_line_method(line, "xxx").

Understood. My thought was simply that this is a pretty large refactor just to account for one special class, especially when there's alternative ways to do it.

Is it means that GDExtension should work like GDScript, not godot module? Because extension_api.json is generated by using ClassDB in core side, and only contain bound apis.

If we want some classes work with a "static" style in GDExtension like in godot module, refactor the logic of generating extension_api.json, let their methods to be marked as static is more reasonable.
It will be a huge change, the topic of "should GDExtension work more like godot module?" need more conversations between core develop members.

While, in general, GDExtension ends up working kinda like GDScript, I think that godot-cpp is special case where we want to maintain some level of source compatibility with Godot modules. For other bindings, like the Rust or Python binding, this doesn't matter. But for C++, we want to be able to use some minimal #ifdefs and compile the same code as a module or GDExtension. For example, the text_server_adv module in Godot can be optionally compiled as GDExtension.

In any case, because extension_api.json is used by all bindings, not just godot-cpp, I don't think it makes sense to mark classes as "static" in there. This would just be something for godot-cpp, and so keeping it in godot-cpp's binding_generator.py makes sense to me.

This is something we could discuss further at upcoming GDExtension meetings!

[dsnopek]

What about generating ClassDB as a regular singleton, and rename static ClassDB from GDExtension to something else?

Like I described above, I personally think we want some level of source compatibility with Godot modules, and so all the methods would need to end up on the same class (even if one class is just forwarding to the other).

If I find the time, I may make an alternative PR to demonstrate the idea I had in my comment above.

[Daylily-Zeleen]

I alraedy refactor this pr in a way that fewer change, please review it.

[dsnopek]

I alraedy refactor this pr in a way that fewer change, please review it.

Thanks! I skimmed it, and it definitely is fewer changes. :-)

I think we just need to figure out the static method vs get_singleton()->method() issue.

[dsnopek]

I've created this alternate PR #1164

Which implements the approach that I described in this comment.

[Daylily-Zeleen]

Alternated by #1164.