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

Ensure MainLoop and its custom script is set right after it's resolved #70771

Merged
merged 1 commit into from
Jul 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 0 additions & 8 deletions core/os/main_loop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,7 @@ void MainLoop::_bind_methods() {
GDVIRTUAL_BIND(_finalize);
}

void MainLoop::set_initialize_script(const Ref<Script> &p_initialize_script) {
initialize_script = p_initialize_script;
}

void MainLoop::initialize() {
if (initialize_script.is_valid()) {
set_script(initialize_script);
}

GDVIRTUAL_CALL(_initialize);
}

Expand Down
4 changes: 0 additions & 4 deletions core/os/main_loop.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@
class MainLoop : public Object {
GDCLASS(MainLoop, Object);

Ref<Script> initialize_script;

protected:
static void _bind_methods();

Expand Down Expand Up @@ -69,8 +67,6 @@ class MainLoop : public Object {
virtual bool process(double p_time);
virtual void finalize();

void set_initialize_script(const Ref<Script> &p_initialize_script);

MainLoop() {}
virtual ~MainLoop() {}
};
Expand Down
8 changes: 4 additions & 4 deletions main/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2657,7 +2657,7 @@ bool Main::start() {
ERR_FAIL_V_MSG(false, vformat("Can't load the script \"%s\" as it doesn't inherit from SceneTree or MainLoop.", script));
}

script_loop->set_initialize_script(script_res);
script_loop->set_script(script_res);
main_loop = script_loop;
} else {
return false;
Expand All @@ -2680,7 +2680,7 @@ bool Main::start() {
OS::get_singleton()->alert("Error: Invalid MainLoop script base type: " + script_base);
ERR_FAIL_V_MSG(false, vformat("The global class %s does not inherit from SceneTree or MainLoop.", main_loop_type));
}
script_loop->set_initialize_script(script_res);
script_loop->set_script(script_res);
main_loop = script_loop;
}
}
Expand All @@ -2705,6 +2705,8 @@ bool Main::start() {
}
}

OS::get_singleton()->set_main_loop(main_loop);
Copy link
Member

Choose a reason for hiding this comment

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

Why moving this here?

Copy link
Member Author

@kleonc kleonc Jan 19, 2023

Choose a reason for hiding this comment

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

The if below handling SceneTree-derived main_loop adds nodes to the tree (autoloads, main scene), meaning it can trigger some user code. If they'd use Engine.get_main_loop() in there it would return null at this point because it was not set yet (pre moving it in here). This fixes e.g. snippet from #70764 (comment).

Copy link
Contributor

@RedwanFox RedwanFox Jan 19, 2023

Choose a reason for hiding this comment

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

Isn't it strange that main_loop (base or overriden) becomes initialized after first scene is initialized?
UPD Did some research #71695


SceneTree *sml = Object::cast_to<SceneTree>(main_loop);
if (sml) {
#ifdef DEBUG_ENABLED
Expand Down Expand Up @@ -3054,8 +3056,6 @@ bool Main::start() {
DisplayServer::get_singleton()->set_icon(icon);
}

OS::get_singleton()->set_main_loop(main_loop);

if (movie_writer) {
movie_writer->begin(DisplayServer::get_singleton()->window_get_size(), fixed_fps, Engine::get_singleton()->get_write_movie_path());
}
Expand Down