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.

Sign up for GitHub

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

Refactor module initialization #60723

Merged
merged 1 commit into from
May 4, 2022
Merged

Refactor module initialization #60723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion core/extension/gdnative_interface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,6 @@ typedef enum {
GDNATIVE_INITIALIZATION_CORE,
GDNATIVE_INITIALIZATION_SERVERS,
GDNATIVE_INITIALIZATION_SCENE,
GDNATIVE_INITIALIZATION_DRIVER,
GDNATIVE_INITIALIZATION_EDITOR,
GDNATIVE_MAX_INITIALIZATION_LEVEL,
} GDNativeInitializationLevel;
Expand Down
1 change: 0 additions & 1 deletion core/extension/native_extension.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,6 @@ void NativeExtension::_bind_methods() {
BIND_ENUM_CONSTANT(INITIALIZATION_LEVEL_CORE);
BIND_ENUM_CONSTANT(INITIALIZATION_LEVEL_SERVERS);
BIND_ENUM_CONSTANT(INITIALIZATION_LEVEL_SCENE);
BIND_ENUM_CONSTANT(INITIALIZATION_LEVEL_DRIVER);
BIND_ENUM_CONSTANT(INITIALIZATION_LEVEL_EDITOR);
}

Expand Down
9 changes: 4 additions & 5 deletions core/extension/native_extension.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,10 @@ class NativeExtension : public Resource {
void close_library();

enum InitializationLevel {
INITIALIZATION_LEVEL_CORE,
INITIALIZATION_LEVEL_SERVERS,
INITIALIZATION_LEVEL_SCENE,
INITIALIZATION_LEVEL_DRIVER,
INITIALIZATION_LEVEL_EDITOR,
INITIALIZATION_LEVEL_CORE = GDNATIVE_INITIALIZATION_CORE,
INITIALIZATION_LEVEL_SERVERS = GDNATIVE_INITIALIZATION_SERVERS,
INITIALIZATION_LEVEL_SCENE = GDNATIVE_INITIALIZATION_SCENE,
INITIALIZATION_LEVEL_EDITOR = GDNATIVE_INITIALIZATION_EDITOR
};

bool is_library_open() const;
Expand Down
4 changes: 1 addition & 3 deletions doc/classes/NativeExtension.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@
</constant>
<constant name="INITIALIZATION_LEVEL_SCENE" value="2" enum="InitializationLevel">
</constant>
<constant name="INITIALIZATION_LEVEL_DRIVER" value="3" enum="InitializationLevel">
</constant>
<constant name="INITIALIZATION_LEVEL_EDITOR" value="4" enum="InitializationLevel">
<constant name="INITIALIZATION_LEVEL_EDITOR" value="3" enum="InitializationLevel">
</constant>
</constants>
</class>
2 changes: 0 additions & 2 deletions drivers/register_driver_types.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,7 @@ void unregister_core_driver_types() {
}

void register_driver_types() {
NativeExtensionManager::get_singleton()->initialize_extensions(NativeExtension::INITIALIZATION_LEVEL_DRIVER);
}

void unregister_driver_types() {
NativeExtensionManager::get_singleton()->deinitialize_extensions(NativeExtension::INITIALIZATION_LEVEL_DRIVER);
}
4 changes: 0 additions & 4 deletions editor/editor_node.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#include "editor_node.h"

#include "core/config/project_settings.h"
#include "core/extension/native_extension_manager.h"
#include "core/input/input.h"
#include "core/io/config_file.h"
#include "core/io/file_access.h"
Expand Down Expand Up @@ -3942,12 +3941,9 @@ void EditorNode::register_editor_types() {
GDREGISTER_CLASS(EditorScenePostImport);
GDREGISTER_CLASS(EditorCommandPalette);
GDREGISTER_CLASS(EditorDebuggerPlugin);

NativeExtensionManager::get_singleton()->initialize_extensions(NativeExtension::INITIALIZATION_LEVEL_EDITOR);
}

void EditorNode::unregister_editor_types() {
NativeExtensionManager::get_singleton()->deinitialize_extensions(NativeExtension::INITIALIZATION_LEVEL_EDITOR);
_init_callbacks.clear();
if (EditorPaths::get_singleton()) {
EditorPaths::free();
Expand Down
49 changes: 38 additions & 11 deletions main/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include "core/crypto/crypto.h"
#include "core/debugger/engine_debugger.h"
#include "core/extension/extension_api_dump.h"
#include "core/extension/native_extension_manager.h"
#include "core/input/input.h"
#include "core/input/input_map.h"
#include "core/io/dir_access.h"
Expand Down Expand Up @@ -406,15 +407,18 @@ Error Main::test_setup() {
tsman->add_interface(ts);
}

// From `Main::setup2()`.
initialize_modules(MODULE_INITIALIZATION_LEVEL_CORE);
register_core_extensions();

// From `Main::setup2()`.
preregister_module_types();
preregister_server_types();

register_core_singletons();

/** INITIALIZE SERVERS **/
register_server_types();
initialize_modules(MODULE_INITIALIZATION_LEVEL_SERVERS);
NativeExtensionManager::get_singleton()->initialize_extensions(NativeExtension::INITIALIZATION_LEVEL_SERVERS);

translation_server->setup(); //register translations, load them, etc.
if (!locale.is_empty()) {
Expand All @@ -428,16 +432,20 @@ Error Main::test_setup() {
register_scene_types();
register_driver_types();

initialize_modules(MODULE_INITIALIZATION_LEVEL_SCENE);
NativeExtensionManager::get_singleton()->initialize_extensions(NativeExtension::INITIALIZATION_LEVEL_SCENE);

#ifdef TOOLS_ENABLED
ClassDB::set_current_api(ClassDB::API_EDITOR);
EditorNode::register_editor_types();

initialize_modules(MODULE_INITIALIZATION_LEVEL_EDITOR);
NativeExtensionManager::get_singleton()->initialize_extensions(NativeExtension::INITIALIZATION_LEVEL_EDITOR);

ClassDB::set_current_api(ClassDB::API_CORE);
#endif
register_platform_apis();

register_module_types();

// Theme needs modules to be initialized so that sub-resources can be loaded.
initialize_theme();

Expand Down Expand Up @@ -479,13 +487,19 @@ void Main::test_cleanup() {
ResourceSaver::remove_custom_savers();

#ifdef TOOLS_ENABLED
NativeExtensionManager::get_singleton()->deinitialize_extensions(NativeExtension::INITIALIZATION_LEVEL_EDITOR);
uninitialize_modules(MODULE_INITIALIZATION_LEVEL_EDITOR);
Comment on lines +490 to +491
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistency: deinitialize / uninitialize. I think the latter is correct, but not sure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel we should rename stuff in GDExtension from deinitialize to uninitialize, but it likely should be done in a subsequent PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to Google:

  • "deinitialize" 51700 results
  • "uninitialize" 64000 results
    Kinda seems the later is a bit more popular.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For fun, but I guess we should probably ensure a single term for the whole engine - https://twitter.com/reduzio/status/1521782429526200321

Copy link
Member

@akien-mga akien-mga May 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "finalize" as proposed in your poll might be a better term. "uninitialize" might be a bit more common than "deinitialize" but it's also used for uninitialized variable, which means something which has not been initialized yet. While here things have been initialized, used, and now need to be cleaned up (which is usually different from just de-initializing variables).

Actually as I write this, "deinitialize" sounds a bit better to avoid the confusion with not yet initialized "uninitialized". That is if we want to keep "initialize" as part of the name since the enums are about INITIALIZATION_LEVEL. Otherwise I think another term like "finalize" can be clearer and less prone to typos (it's always super difficult to spell uninitialize IMO with all those ni's :P).

Alternatively, I like "cleanup" too for this kind of task.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can discuss this with other contributors I guess, but anything we decide we should probably make it consistent across the codebase.

EditorNode::unregister_editor_types();
#endif

unregister_module_types();
NativeExtensionManager::get_singleton()->deinitialize_extensions(NativeExtension::INITIALIZATION_LEVEL_SCENE);
uninitialize_modules(MODULE_INITIALIZATION_LEVEL_SCENE);
unregister_platform_apis();
unregister_driver_types();
unregister_scene_types();

NativeExtensionManager::get_singleton()->deinitialize_extensions(NativeExtension::INITIALIZATION_LEVEL_SERVERS);
uninitialize_modules(MODULE_INITIALIZATION_LEVEL_SERVERS);
unregister_server_types();

OS::get_singleton()->finalize();
Expand All @@ -507,6 +521,7 @@ void Main::test_cleanup() {
}

unregister_core_driver_types();
uninitialize_modules(MODULE_INITIALIZATION_LEVEL_CORE);
unregister_core_extensions();
unregister_core_types();

Expand Down Expand Up @@ -1166,6 +1181,7 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph
// Initialize user data dir.
OS::get_singleton()->ensure_user_data_dir();

initialize_modules(MODULE_INITIALIZATION_LEVEL_CORE);
register_core_extensions(); // core extensions must be registered after globals setup and before display

ResourceUID::get_singleton()->load_from_cache(); // load UUIDs from cache.
Expand Down Expand Up @@ -1584,7 +1600,6 @@ Error Main::setup2(Thread::ID p_main_tid_override) {
tsman->add_interface(ts);
}

preregister_module_types();
preregister_server_types();

// Print engine name and version
Expand Down Expand Up @@ -1751,6 +1766,8 @@ Error Main::setup2(Thread::ID p_main_tid_override) {
}

register_server_types();
initialize_modules(MODULE_INITIALIZATION_LEVEL_SERVERS);
NativeExtensionManager::get_singleton()->initialize_extensions(NativeExtension::INITIALIZATION_LEVEL_SERVERS);

MAIN_PRINT("Main: Load Boot Image");

Expand Down Expand Up @@ -1925,14 +1942,16 @@ Error Main::setup2(Thread::ID p_main_tid_override) {
MAIN_PRINT("Main: Load Scene Types");

register_scene_types();

MAIN_PRINT("Main: Load Driver Types");

register_driver_types();

initialize_modules(MODULE_INITIALIZATION_LEVEL_SCENE);
NativeExtensionManager::get_singleton()->initialize_extensions(NativeExtension::INITIALIZATION_LEVEL_SCENE);

#ifdef TOOLS_ENABLED
ClassDB::set_current_api(ClassDB::API_EDITOR);
EditorNode::register_editor_types();
initialize_modules(MODULE_INITIALIZATION_LEVEL_EDITOR);
NativeExtensionManager::get_singleton()->initialize_extensions(NativeExtension::INITIALIZATION_LEVEL_EDITOR);

ClassDB::set_current_api(ClassDB::API_CORE);

Expand All @@ -1941,7 +1960,6 @@ Error Main::setup2(Thread::ID p_main_tid_override) {
MAIN_PRINT("Main: Load Modules");

register_platform_apis();
register_module_types();

// Theme needs modules to be initialized so that sub-resources can be loaded.
initialize_theme();
Expand Down Expand Up @@ -2852,15 +2870,23 @@ void Main::cleanup(bool p_force) {
}

#ifdef TOOLS_ENABLED
NativeExtensionManager::get_singleton()->deinitialize_extensions(NativeExtension::INITIALIZATION_LEVEL_EDITOR);
uninitialize_modules(MODULE_INITIALIZATION_LEVEL_EDITOR);
EditorNode::unregister_editor_types();

#endif

ImageLoader::cleanup();

unregister_module_types();
NativeExtensionManager::get_singleton()->deinitialize_extensions(NativeExtension::INITIALIZATION_LEVEL_SCENE);
uninitialize_modules(MODULE_INITIALIZATION_LEVEL_SCENE);

unregister_platform_apis();
unregister_driver_types();
unregister_scene_types();

NativeExtensionManager::get_singleton()->deinitialize_extensions(NativeExtension::INITIALIZATION_LEVEL_SERVERS);
uninitialize_modules(MODULE_INITIALIZATION_LEVEL_SERVERS);
unregister_server_types();

EngineDebugger::deinitialize();
Expand Down Expand Up @@ -2929,6 +2955,7 @@ void Main::cleanup(bool p_force) {

unregister_core_driver_types();
unregister_core_extensions();
uninitialize_modules(MODULE_INITIALIZATION_LEVEL_CORE);
unregister_core_types();

OS::get_singleton()->finalize_core();
Expand Down
35 changes: 12 additions & 23 deletions methods.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,25 +266,19 @@ def write_disabled_classes(class_list):

def write_modules(modules):
includes_cpp = ""
preregister_cpp = ""
register_cpp = ""
unregister_cpp = ""
initialize_cpp = ""
uninitialize_cpp = ""

for name, path in modules.items():
try:
with open(os.path.join(path, "register_types.h")):
includes_cpp += '#include "' + path + '/register_types.h"\n'
preregister_cpp += "#ifdef MODULE_" + name.upper() + "_ENABLED\n"
preregister_cpp += "#ifdef MODULE_" + name.upper() + "_HAS_PREREGISTER\n"
preregister_cpp += "\tpreregister_" + name + "_types();\n"
preregister_cpp += "#endif\n"
preregister_cpp += "#endif\n"
register_cpp += "#ifdef MODULE_" + name.upper() + "_ENABLED\n"
register_cpp += "\tregister_" + name + "_types();\n"
register_cpp += "#endif\n"
unregister_cpp += "#ifdef MODULE_" + name.upper() + "_ENABLED\n"
unregister_cpp += "\tunregister_" + name + "_types();\n"
unregister_cpp += "#endif\n"
initialize_cpp += "#ifdef MODULE_" + name.upper() + "_ENABLED\n"
initialize_cpp += "\tinitialize_" + name + "_module(p_level);\n"
initialize_cpp += "#endif\n"
uninitialize_cpp += "#ifdef MODULE_" + name.upper() + "_ENABLED\n"
uninitialize_cpp += "\tuninitialize_" + name + "_module(p_level);\n"
uninitialize_cpp += "#endif\n"
except OSError:
pass

Expand All @@ -296,22 +290,17 @@ def write_modules(modules):

%s

void preregister_module_types() {
void initialize_modules(ModuleInitializationLevel p_level) {
%s
}

void register_module_types() {
%s
}

void unregister_module_types() {
void uninitialize_modules(ModuleInitializationLevel p_level) {
%s
}
""" % (
includes_cpp,
preregister_cpp,
register_cpp,
unregister_cpp,
initialize_cpp,
uninitialize_cpp,
)

# NOTE: It is safe to generate this file here, since this is still executed serially
Expand Down
12 changes: 10 additions & 2 deletions modules/basis_universal/register_types.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,11 @@ static Ref<Image> basis_universal_unpacker(const Vector<uint8_t> &p_buffer) {
return basis_universal_unpacker_ptr(r, size);
}

void register_basis_universal_types() {
void initialize_basis_universal_module(ModuleInitializationLevel p_level) {
if (p_level != MODULE_INITIALIZATION_LEVEL_SCENE) {
return;
}

#ifdef TOOLS_ENABLED
using namespace basisu;
using namespace basist;
Expand All @@ -277,7 +281,11 @@ void register_basis_universal_types() {
Image::basis_universal_unpacker_ptr = basis_universal_unpacker_ptr;
}

void unregister_basis_universal_types() {
void uninitialize_basis_universal_module(ModuleInitializationLevel p_level) {
if (p_level != MODULE_INITIALIZATION_LEVEL_SCENE) {
return;
}

#ifdef TOOLS_ENABLED
Image::basis_universal_packer = nullptr;
#endif
Expand Down
6 changes: 4 additions & 2 deletions modules/basis_universal/register_types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@
#ifndef BASIS_UNIVERSAL_REGISTER_TYPES_H
#define BASIS_UNIVERSAL_REGISTER_TYPES_H

void register_basis_universal_types();
void unregister_basis_universal_types();
#include "modules/register_module_types.h"

void initialize_basis_universal_module(ModuleInitializationLevel p_level);
void uninitialize_basis_universal_module(ModuleInitializationLevel p_level);

#endif // BASIS_UNIVERSAL_REGISTER_TYPES_H
12 changes: 10 additions & 2 deletions modules/bmp/register_types.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,19 @@

static ImageLoaderBMP *image_loader_bmp = nullptr;

void register_bmp_types() {
void initialize_bmp_module(ModuleInitializationLevel p_level) {
if (p_level != MODULE_INITIALIZATION_LEVEL_SCENE) {
return;
}

image_loader_bmp = memnew(ImageLoaderBMP);
ImageLoader::add_image_format_loader(image_loader_bmp);
}

void unregister_bmp_types() {
void uninitialize_bmp_module(ModuleInitializationLevel p_level) {
if (p_level != MODULE_INITIALIZATION_LEVEL_SCENE) {
return;
}

memdelete(image_loader_bmp);
}
6 changes: 4 additions & 2 deletions modules/bmp/register_types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@
#ifndef BMP_REGISTER_TYPES_H
#define BMP_REGISTER_TYPES_H

void register_bmp_types();
void unregister_bmp_types();
#include "modules/register_module_types.h"

void initialize_bmp_module(ModuleInitializationLevel p_level);
void uninitialize_bmp_module(ModuleInitializationLevel p_level);

#endif // BMP_REGISTER_TYPES_H
11 changes: 9 additions & 2 deletions modules/camera/register_types.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@
#include "camera_osx.h"
#endif

void register_camera_types() {
void initialize_camera_module(ModuleInitializationLevel p_level) {
if (p_level != MODULE_INITIALIZATION_LEVEL_SCENE) {
return;
}

#if defined(WINDOWS_ENABLED)
CameraServer::make_default<CameraWindows>();
#endif
Expand All @@ -46,5 +50,8 @@ void register_camera_types() {
#endif
}

void unregister_camera_types() {
void uninitialize_camera_module(ModuleInitializationLevel p_level) {
if (p_level != MODULE_INITIALIZATION_LEVEL_SCENE) {
return;
}
}
Loading