Skip to content

Commit

Permalink
Rework the surface upgrade tool to inform users without blocking
Browse files Browse the repository at this point in the history
This removes the immediate confirmation dialog and insteads prints the
message to the editor log (and it also appears as a toast). The immediate dialog
is a devil's plaything, and it cannot be used in this scenario (if it can be used
anywhere at all). The condition that triggers the SUT can happen during any
attempt by the rendering server to read a mesh. This means it will conflict
with a number of editor processes, like loading, importing, preview
generation, export, CLI mode, etc.

So while this is less on the nose as far as informing users goes, it's also
our best option to use the log and the toaster.
  • Loading branch information
YuriSizov committed Nov 22, 2023
1 parent a34814b commit e874499
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 30 deletions.
12 changes: 6 additions & 6 deletions editor/editor_node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1047,11 +1047,6 @@ void EditorNode::_sources_changed(bool p_exist) {
// loading textures, as they are now properly imported.
RenderingServer::get_singleton()->global_shader_parameters_load_settings(true);

// Start preview thread now that it's safe.
if (!singleton->cmdline_export_mode) {
EditorResourcePreview::get_singleton()->start();
}

_load_editor_layout();

if (!defer_load_scene.is_empty()) {
Expand All @@ -1066,6 +1061,11 @@ void EditorNode::_sources_changed(bool p_exist) {
if (SurfaceUpgradeTool::get_singleton()->is_show_requested()) {
SurfaceUpgradeTool::get_singleton()->show_popup();
}

// Start preview thread now that it's safe.
if (!singleton->cmdline_export_mode) {
EditorResourcePreview::get_singleton()->start();
}
}
}

Expand Down Expand Up @@ -3055,7 +3055,7 @@ void EditorNode::_tool_menu_option(int p_idx) {
orphan_resources->show();
} break;
case TOOLS_SURFACE_UPGRADE: {
surface_upgrade_dialog->popup_centered(Size2(750 * EDSCALE, 0));
surface_upgrade_dialog->popup_on_demand();
} break;
case TOOLS_CUSTOM: {
if (tool_menu->get_item_submenu(p_idx) == "") {
Expand Down
3 changes: 3 additions & 0 deletions editor/editor_node.h
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,9 @@ class EditorNode : public Node {

bool call_build();

// This is a very naive estimation, but we need something now. Will be reworked later.
bool is_editor_ready() const { return is_inside_tree() && !waiting_for_first_scan; }

static EditorNode *get_singleton() { return singleton; }

static EditorLog *get_log() { return singleton->log; }
Expand Down
52 changes: 30 additions & 22 deletions editor/surface_upgrade_tool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,40 +58,39 @@ void SurfaceUpgradeTool::_add_files(EditorFileSystemDirectory *p_dir, Vector<Str
}

void SurfaceUpgradeTool::_try_show_popup() {
if (singleton->show_requested || singleton->popped_up) {
if (singleton->show_requested || singleton->updating) {
return;
}
singleton->show_requested = true;

RS::get_singleton()->set_warn_on_surface_upgrade(false);

if (EditorNode::get_singleton()->is_project_exporting()) {
return; // We suppress the tool during the export routine to avoid possible conflicts.
}

singleton->show_requested = true;

if (!EditorNode::get_singleton()->is_editor_ready()) {
// EditorNode may not be ready yet. It will call this tool when it is.
return;
}

if (EditorFileSystem::get_singleton()->is_importing()) {
EditorFileSystem::get_singleton()->connect("resources_reimported", callable_mp(singleton, &SurfaceUpgradeTool::_show_popup), CONNECT_ONE_SHOT);
} else if (EditorNode::get_singleton()->is_inside_tree()) {
} else {
singleton->_show_popup();
}

// EditorNode may not be ready yet. It will call this tool when it is.
}

void SurfaceUpgradeTool::_show_popup() {
MutexLock lock(mutex);
if (EditorNode::get_singleton()->is_project_exporting()) {
return; // We suppress the tool during the export routine, because the immediate dialog breaks everything.
}
if (!show_requested || popped_up) {
return;
if (!show_requested) {
return; // We only show the dialog if it was previously requested.
}
show_requested = false;
popped_up = true;

const String confirmation_message = TTR("This project uses meshes with an outdated mesh format from previous Godot versions. The engine needs to update the format in order to use those meshes.\n\nPress 'Restart & Upgrade' to run the surface upgrade tool which will update and re-save all meshes and scenes. This update will restart the editor and may take several minutes. Upgrading will make the meshes incompatible with previous versions of Godot.\n\nPress 'Upgrade Only' to continue opening the scene as normal. The engine will update each mesh in memory, but the update will not be saved. Choosing this option will lead to slower load times every time this project is loaded.");
bool accepted = EditorNode::immediate_confirmation_dialog(confirmation_message, TTR("Restart & Upgrade"), TTR("Upgrade Only"), 500);
if (accepted) {
prepare_upgrade();
} else {
RS::get_singleton()->set_warn_on_surface_upgrade(true);
}
const String confirmation_message = TTR("This project uses meshes with an outdated mesh format from previous Godot versions. The engine needs to update the format in order to use those meshes. Please use the 'Upgrade Mesh Surfaces' tool from the 'Project > Tools' menu. You can ignore this message and keep using outdated meshes, but keep in mind that this leads to increased load times every time you load the project.");
WARN_PRINT_ED(confirmation_message);
}

void SurfaceUpgradeTool::prepare_upgrade() {
Expand All @@ -110,10 +109,11 @@ void SurfaceUpgradeTool::prepare_upgrade() {

// Ensure that the warnings and popups are skipped.
void SurfaceUpgradeTool::begin_upgrade() {
updating = true;

EditorSettings::get_singleton()->set_project_metadata("surface_upgrade_tool", "run_on_restart", false);
RS::get_singleton()->set_surface_upgrade_callback(nullptr);
RS::get_singleton()->set_warn_on_surface_upgrade(false);
popped_up = true;
}

void SurfaceUpgradeTool::finish_upgrade() {
Expand Down Expand Up @@ -170,7 +170,17 @@ SurfaceUpgradeTool::SurfaceUpgradeTool() {
RS::get_singleton()->set_surface_upgrade_callback(_try_show_popup);
}

SurfaceUpgradeTool::~SurfaceUpgradeTool() {}
SurfaceUpgradeTool::~SurfaceUpgradeTool() {
singleton = nullptr;
}

void SurfaceUpgradeDialog::popup_on_demand() {
const String confirmation_message = TTR("The mesh format has changed in Godot 4.2, which affects both imported meshes and meshes authored inside of Godot. The engine needs to update the format in order to use those meshes.\n\nIf your project predates Godot 4.2 and contains meshes we recommend you run this one time conversion tool. This update will restart the editor and may take several minutes. Upgrading will make the meshes incompatible with previous versions of Godot.\n\nYou can still use your existing meshes as is. The engine will update each mesh in memory, but the update will not be saved. Choosing this option will lead to slower load times every time this project is loaded.");
set_text(confirmation_message);
get_ok_button()->set_text(TTR("Restart & Upgrade"));

popup_centered(Size2(750 * EDSCALE, 0));
}

void SurfaceUpgradeDialog::_notification(int p_what) {
switch (p_what) {
Expand All @@ -182,8 +192,6 @@ void SurfaceUpgradeDialog::_notification(int p_what) {
}

SurfaceUpgradeDialog::SurfaceUpgradeDialog() {
const String confirmation_message = TTR("The mesh format has changed in Godot 4.2, which affects both imported meshes and meshes authored inside of Godot. The engine needs to update the format in order to use those meshes.\n\nIf your project predates Godot 4.2 and contains meshes we recommend you run this one time conversion tool. This update will restart the editor and may take several minutes. Upgrading will make the meshes incompatible with previous versions of Godot.\n\nYou can still use your existing meshes as is. The engine will update each mesh in memory, but the update will not be saved. Choosing this option will lead to slower load times every time this project is loaded.");
set_text(confirmation_message);
set_autowrap(true);
get_label()->set_custom_minimum_size(Size2(750 * EDSCALE, 0));
}
8 changes: 6 additions & 2 deletions editor/surface_upgrade_tool.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,14 @@ class SurfaceUpgradeTool : public Object {

static SurfaceUpgradeTool *singleton;

bool show_requested = false;
bool popped_up = false;
Mutex mutex;

bool show_requested = false;
bool updating = false;

static void _try_show_popup();
void _show_popup();

void _add_files(EditorFileSystemDirectory *p_dir, Vector<String> &r_reimport_paths, Vector<String> &r_resave_paths);

protected:
Expand All @@ -72,6 +74,8 @@ class SurfaceUpgradeDialog : public ConfirmationDialog {
void _notification(int p_what);

public:
void popup_on_demand();

SurfaceUpgradeDialog();
};

Expand Down

0 comments on commit e874499

Please sign in to comment.