From 1074017f043ec9155b12ea97cd00cf11361ccdf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Verschelde?= Date: Wed, 9 Jun 2021 10:47:32 +0200 Subject: [PATCH] Refactor editor paths validation in EditorPaths and EditorSettings - EditorSettings: Ensure that `create()` makes a valid singleton. Fixes #49179, fixes #49450. - EditorPaths: Cleanup code, properly set `paths_valid`. - EditorPaths: Move more paths validation (check, mkdir) from EditorSettings for a better separation of concerns. - EditorPaths: Move EditorFileSystem creation of `.godot/imported` next to other paths. --- editor/editor_file_system.cpp | 19 +----- editor/editor_node.cpp | 4 -- editor/editor_node.h | 1 - editor/editor_paths.cpp | 120 ++++++++++++++++++++++++-------- editor/editor_paths.h | 28 ++++---- editor/editor_settings.cpp | 125 +++++++++------------------------- editor/editor_settings.h | 1 - main/main.cpp | 8 +-- 8 files changed, 142 insertions(+), 164 deletions(-) diff --git a/editor/editor_file_system.cpp b/editor/editor_file_system.cpp index 050e55c624..c61b097ccd 100644 --- a/editor/editor_file_system.cpp +++ b/editor/editor_file_system.cpp @@ -1934,19 +1934,6 @@ void EditorFileSystem::_reimport_thread(uint32_t p_index, ImportThreadData *p_im } void EditorFileSystem::reimport_files(const Vector &p_files) { - { - // Ensure that ProjectSettings::IMPORTED_FILES_PATH exists. - DirAccess *da = DirAccess::open("res://"); - if (da->change_dir(ProjectSettings::IMPORTED_FILES_PATH) != OK) { - Error err = da->make_dir_recursive(ProjectSettings::IMPORTED_FILES_PATH); - if (err || da->change_dir(ProjectSettings::IMPORTED_FILES_PATH) != OK) { - memdelete(da); - ERR_FAIL_MSG("Failed to create '" + ProjectSettings::IMPORTED_FILES_PATH + "' folder."); - } - } - memdelete(da); - } - importing = true; EditorProgress pr("reimport", TTR("(Re)Importing Assets"), p_files.size()); @@ -2177,13 +2164,9 @@ EditorFileSystem::EditorFileSystem() { scanning_changes = false; scanning_changes_done = false; - DirAccess *da = DirAccess::create(DirAccess::ACCESS_RESOURCES); - if (da->change_dir(ProjectSettings::IMPORTED_FILES_PATH) != OK) { - da->make_dir(ProjectSettings::IMPORTED_FILES_PATH); - } // This should probably also work on Unix and use the string it returns for FAT32 or exFAT + DirAccessRef da = DirAccess::create(DirAccess::ACCESS_RESOURCES); using_fat32_or_exfat = (da->get_filesystem_type() == "FAT32" || da->get_filesystem_type() == "exFAT"); - memdelete(da); scan_total = 0; update_script_classes_queued.clear(); diff --git a/editor/editor_node.cpp b/editor/editor_node.cpp index d8c5a14d4f..9a94597f0b 100644 --- a/editor/editor_node.cpp +++ b/editor/editor_node.cpp @@ -3728,10 +3728,6 @@ bool EditorNode::is_scene_in_use(const String &p_path) { return false; } -void EditorNode::register_editor_paths(bool p_for_project_manager) { - EditorPaths::create(p_for_project_manager); -} - void EditorNode::register_editor_types() { ResourceLoader::set_timestamp_on_load(true); ResourceSaver::set_timestamp_on_save(true); diff --git a/editor/editor_node.h b/editor/editor_node.h index 9625b318e0..ce42fd3c8a 100644 --- a/editor/editor_node.h +++ b/editor/editor_node.h @@ -798,7 +798,6 @@ public: Error export_preset(const String &p_preset, const String &p_path, bool p_debug, bool p_pack_only); - static void register_editor_paths(bool p_for_project_manager); static void register_editor_types(); static void unregister_editor_types(); diff --git a/editor/editor_paths.cpp b/editor/editor_paths.cpp index ddba36c187..323707ec6c 100644 --- a/editor/editor_paths.cpp +++ b/editor/editor_paths.cpp @@ -29,8 +29,12 @@ /*************************************************************************/ #include "editor_paths.h" + +#include "core/config/engine.h" +#include "core/config/project_settings.h" #include "core/io/dir_access.h" #include "core/os/os.h" +#include "main/main.h" // For `is_project_manager`. EditorPaths *EditorPaths::singleton = nullptr; @@ -41,23 +45,32 @@ bool EditorPaths::are_paths_valid() const { String EditorPaths::get_data_dir() const { return data_dir; } + String EditorPaths::get_config_dir() const { return config_dir; } + String EditorPaths::get_cache_dir() const { return cache_dir; } + +String EditorPaths::get_project_data_dir() const { + return project_data_dir; +} + bool EditorPaths::is_self_contained() const { return self_contained; } + String EditorPaths::get_self_contained_file() const { return self_contained_file; } -void EditorPaths::create(bool p_for_project_manager) { +void EditorPaths::create() { ERR_FAIL_COND(singleton != nullptr); - memnew(EditorPaths(p_for_project_manager)); + memnew(EditorPaths()); } + void EditorPaths::free() { ERR_FAIL_COND(singleton == nullptr); memdelete(singleton); @@ -71,9 +84,10 @@ void EditorPaths::_bind_methods() { ClassDB::bind_method(D_METHOD("get_self_contained_file"), &EditorPaths::get_self_contained_file); } -EditorPaths::EditorPaths(bool p_for_project_mamanger) { +EditorPaths::EditorPaths() { singleton = this; + // Self-contained mode if a `._sc_` or `_sc_` file is present in executable dir. String exe_path = OS::get_singleton()->get_executable_path().get_base_dir(); { DirAccessRef d = DirAccess::create_for_path(exe_path); @@ -100,13 +114,13 @@ EditorPaths::EditorPaths(bool p_for_project_mamanger) { cache_path = exe_path; cache_dir = data_dir.plus_file("cache"); } else { - // Typically XDG_DATA_HOME or %APPDATA% + // Typically XDG_DATA_HOME or %APPDATA%. data_path = OS::get_singleton()->get_data_path(); data_dir = data_path.plus_file(OS::get_singleton()->get_godot_dir_name()); - // Can be different from data_path e.g. on Linux or macOS + // Can be different from data_path e.g. on Linux or macOS. config_path = OS::get_singleton()->get_config_path(); config_dir = config_path.plus_file(OS::get_singleton()->get_godot_dir_name()); - // Can be different from above paths, otherwise a subfolder of data_dir + // Can be different from above paths, otherwise a subfolder of data_dir. cache_path = OS::get_singleton()->get_cache_path(); if (cache_path == data_path) { cache_dir = data_dir.plus_file("cache"); @@ -116,37 +130,85 @@ EditorPaths::EditorPaths(bool p_for_project_mamanger) { } paths_valid = (data_path != "" && config_path != "" && cache_path != ""); + ERR_FAIL_COND_MSG(!paths_valid, "Editor data, config, or cache paths are invalid."); - if (paths_valid) { - DirAccessRef dir = DirAccess::create(DirAccess::ACCESS_FILESYSTEM); + // Validate or create each dir and its relevant subdirectories. + + DirAccessRef dir = DirAccess::create(DirAccess::ACCESS_FILESYSTEM); + + // Data dir. + { if (dir->change_dir(data_dir) != OK) { dir->make_dir_recursive(data_dir); if (dir->change_dir(data_dir) != OK) { - ERR_PRINT("Cannot create data directory!"); + ERR_PRINT("Could not create editor data directory: " + data_dir); paths_valid = false; } } - // Validate/create cache dir - - if (dir->change_dir(EditorPaths::get_singleton()->get_cache_dir()) != OK) { - dir->make_dir_recursive(cache_dir); - if (dir->change_dir(cache_dir) != OK) { - ERR_PRINT("Cannot create cache directory!"); - } - } - - if (p_for_project_mamanger) { - Engine::get_singleton()->set_shader_cache_path(get_data_dir()); - } else { - DirAccessRef dir2 = DirAccess::open("res://"); - if (dir2->change_dir(".godot") != OK) { //ensure the .godot subdir exists - if (dir2->make_dir(".godot") != OK) { - ERR_PRINT("Cannot create res://.godot directory!"); - } - } - - Engine::get_singleton()->set_shader_cache_path("res://.godot"); + if (!dir->dir_exists("templates")) { + dir->make_dir("templates"); } } + + // Config dir. + { + if (dir->change_dir(config_dir) != OK) { + dir->make_dir_recursive(config_dir); + if (dir->change_dir(config_dir) != OK) { + ERR_PRINT("Could not create editor config directory: " + config_dir); + paths_valid = false; + } + } + + if (!dir->dir_exists("text_editor_themes")) { + dir->make_dir("text_editor_themes"); + } + if (!dir->dir_exists("script_templates")) { + dir->make_dir("script_templates"); + } + if (!dir->dir_exists("feature_profiles")) { + dir->make_dir("feature_profiles"); + } + } + + // Cache dir. + { + if (dir->change_dir(cache_dir) != OK) { + dir->make_dir_recursive(cache_dir); + if (dir->change_dir(cache_dir) != OK) { + ERR_PRINT("Could not create editor cache directory: " + cache_dir); + paths_valid = false; + } + } + } + + // Validate or create project-specific editor data dir (`res://.godot`), + // including shader cache subdir. + + if (Main::is_project_manager()) { + // Nothing to create, use shared editor data dir for shader cache. + Engine::get_singleton()->set_shader_cache_path(data_dir); + } else { + DirAccessRef dir_res = DirAccess::create(DirAccess::ACCESS_RESOURCES); + if (dir_res->change_dir(project_data_dir) != OK) { + dir_res->make_dir_recursive(project_data_dir); + if (dir_res->change_dir(project_data_dir) != OK) { + ERR_PRINT("Could not create project data directory (" + project_data_dir + ") in: " + dir_res->get_current_dir()); + paths_valid = false; + } + } + Engine::get_singleton()->set_shader_cache_path(project_data_dir); + + // Editor metadata dir. + if (!dir_res->dir_exists("editor")) { + dir_res->make_dir("editor"); + } + // Imported assets dir. + if (!dir_res->dir_exists(ProjectSettings::IMPORTED_FILES_PATH)) { + dir_res->make_dir(ProjectSettings::IMPORTED_FILES_PATH); + } + } + + print_line("paths valid: " + itos((int)paths_valid)); } diff --git a/editor/editor_paths.h b/editor/editor_paths.h index c1be33f5c2..2c156b7c96 100644 --- a/editor/editor_paths.h +++ b/editor/editor_paths.h @@ -28,20 +28,22 @@ /* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ /*************************************************************************/ -#ifndef EDITORPATHS_H -#define EDITORPATHS_H +#ifndef EDITOR_PATHS_H +#define EDITOR_PATHS_H -#include "core/config/engine.h" +#include "core/object/class_db.h" +#include "core/string/ustring.h" class EditorPaths : public Object { GDCLASS(EditorPaths, Object) - bool paths_valid = false; - String data_dir; //editor data dir - String config_dir; //editor config dir - String cache_dir; //editor cache dir - bool self_contained = false; //true if running self contained - String self_contained_file; //self contained file with configuration + bool paths_valid = false; // If any of the paths can't be created, this is false. + String data_dir; // Editor data (templates, shader cache, etc.). + String config_dir; // Editor config (settings, profiles, themes, etc.). + String cache_dir; // Editor cache (thumbnails, tmp generated files). + String project_data_dir = "res://.godot"; // Project-specific data (metadata, shader cache, etc.). + bool self_contained = false; // Self-contained means everything goes to `editor_data` dir. + String self_contained_file; // Self-contained file with configuration. static EditorPaths *singleton; @@ -54,6 +56,8 @@ public: String get_data_dir() const; String get_config_dir() const; String get_cache_dir() const; + String get_project_data_dir() const; + bool is_self_contained() const; String get_self_contained_file() const; @@ -61,10 +65,10 @@ public: return singleton; } - static void create(bool p_for_project_manager); + static void create(); static void free(); - EditorPaths(bool p_for_project_mamanger = false); + EditorPaths(); }; -#endif // EDITORPATHS_H +#endif // EDITOR_PATHS_H diff --git a/editor/editor_settings.cpp b/editor/editor_settings.cpp index 949638f0c9..fa7980b95f 100644 --- a/editor/editor_settings.cpp +++ b/editor/editor_settings.cpp @@ -768,8 +768,8 @@ void EditorSettings::_load_defaults(Ref p_extra_config) { for (int i = 0; i < list.size(); i++) { String name = list[i].replace("/", "::"); set("projects/" + name, list[i]); - }; - }; + } + } if (p_extra_config->has_section("presets")) { List keys; @@ -779,9 +779,9 @@ void EditorSettings::_load_defaults(Ref p_extra_config) { String key = E->get(); Variant val = p_extra_config->get_value("presets", key); set(key, val); - }; - }; - }; + } + } + } } void EditorSettings::_load_godot2_text_editor_theme() { @@ -871,9 +871,8 @@ static void _create_script_templates(const String &p_path) { Dictionary templates = _get_builtin_script_templates(); List keys; templates.get_key_list(&keys); - FileAccess *file = FileAccess::create(FileAccess::ACCESS_FILESYSTEM); - - DirAccess *dir = DirAccess::create(DirAccess::ACCESS_FILESYSTEM); + FileAccessRef file = FileAccess::create(FileAccess::ACCESS_FILESYSTEM); + DirAccessRef dir = DirAccess::create(DirAccess::ACCESS_FILESYSTEM); dir->change_dir(p_path); for (int i = 0; i < keys.size(); i++) { if (!dir->file_exists(keys[i])) { @@ -883,9 +882,6 @@ static void _create_script_templates(const String &p_path) { file->close(); } } - - memdelete(dir); - memdelete(file); } // PUBLIC METHODS @@ -895,103 +891,48 @@ EditorSettings *EditorSettings::get_singleton() { } void EditorSettings::create() { + // IMPORTANT: create() *must* create a valid EditorSettings singleton, + // as the rest of the engine code will assume it. As such, it should never + // return (incl. via ERR_FAIL) without initializing the singleton member. + if (singleton.ptr()) { - return; //pointless + ERR_PRINT("Can't recreate EditorSettings as it already exists."); + return; } + ClassDB::register_class(); // Otherwise it can't be unserialized. + + String config_file_path; Ref extra_config = memnew(ConfigFile); + if (!EditorPaths::get_singleton()) { + ERR_PRINT("Bug (please report): EditorPaths haven't been initialized, EditorSettings cannot be created properly."); + goto fail; + } + if (EditorPaths::get_singleton()->is_self_contained()) { Error err = extra_config->load(EditorPaths::get_singleton()->get_self_contained_file()); if (err != OK) { - ERR_PRINT("Can't load extra config from path :" + EditorPaths::get_singleton()->get_self_contained_file()); + ERR_PRINT("Can't load extra config from path: " + EditorPaths::get_singleton()->get_self_contained_file()); } } - DirAccess *dir = nullptr; - - ClassDB::register_class(); //otherwise it can't be unserialized - - String config_file_path; - if (EditorPaths::get_singleton()->are_paths_valid()) { - // Validate/create data dir and subdirectories + _create_script_templates(EditorPaths::get_singleton()->get_config_dir().plus_file("script_templates")); - String data_dir = EditorPaths::get_singleton()->get_data_dir(); - - dir = DirAccess::create(DirAccess::ACCESS_FILESYSTEM); - if (dir->change_dir(data_dir) != OK) { - dir->make_dir_recursive(data_dir); - if (dir->change_dir(data_dir) != OK) { - ERR_PRINT("Cannot create data directory!"); - memdelete(dir); - goto fail; - } - } - - if (dir->change_dir("templates") != OK) { - dir->make_dir("templates"); - } else { - dir->change_dir(".."); - } - - // Validate/create config dir and subdirectories - - if (dir->change_dir(EditorPaths::get_singleton()->get_config_dir()) != OK) { - dir->make_dir_recursive(EditorPaths::get_singleton()->get_config_dir()); - if (dir->change_dir(EditorPaths::get_singleton()->get_config_dir()) != OK) { - ERR_PRINT("Cannot create config directory!"); - memdelete(dir); - goto fail; - } - } - - if (dir->change_dir("text_editor_themes") != OK) { - dir->make_dir("text_editor_themes"); - } else { - dir->change_dir(".."); - } - - if (dir->change_dir("script_templates") != OK) { - dir->make_dir("script_templates"); - } else { - dir->change_dir(".."); - } - - if (dir->change_dir("feature_profiles") != OK) { - dir->make_dir("feature_profiles"); - } else { - dir->change_dir(".."); - } - - _create_script_templates(dir->get_current_dir().plus_file("script_templates")); - - { - // Validate/create project-specific editor settings dir. - DirAccessRef da = DirAccess::create(DirAccess::ACCESS_RESOURCES); - if (da->change_dir(EditorSettings::PROJECT_EDITOR_SETTINGS_PATH) != OK) { - Error err = da->make_dir_recursive(EditorSettings::PROJECT_EDITOR_SETTINGS_PATH); - if (err || da->change_dir(EditorSettings::PROJECT_EDITOR_SETTINGS_PATH) != OK) { - ERR_FAIL_MSG("Failed to create '" + EditorSettings::PROJECT_EDITOR_SETTINGS_PATH + "' folder."); - } - } - } - - // Validate editor config file + // Validate editor config file. + DirAccessRef dir = DirAccess::open(EditorPaths::get_singleton()->get_config_dir()); String config_file_name = "editor_settings-" + itos(VERSION_MAJOR) + ".tres"; config_file_path = EditorPaths::get_singleton()->get_config_dir().plus_file(config_file_name); if (!dir->file_exists(config_file_name)) { - memdelete(dir); goto fail; } - memdelete(dir); - singleton = ResourceLoader::load(config_file_path, "EditorSettings"); if (singleton.is_null()) { - WARN_PRINT("Could not open config file."); + ERR_PRINT("Could not load editor settings from path: " + config_file_path); goto fail; } @@ -1009,7 +950,6 @@ void EditorSettings::create() { } fail: - // patch init projects String exe_path = OS::get_singleton()->get_executable_path().get_base_dir(); @@ -1017,9 +957,9 @@ fail: Vector list = extra_config->get_value("init_projects", "list"); for (int i = 0; i < list.size(); i++) { list.write[i] = exe_path.plus_file(list[i]); - }; + } extra_config->set_value("init_projects", "list", list); - }; + } singleton = Ref(memnew(EditorSettings)); singleton->save_changed_setting = true; @@ -1251,16 +1191,15 @@ void EditorSettings::add_property_hint(const PropertyInfo &p_hint) { hints[p_hint.name] = p_hint; } -// Data directories +// Editor data and config directories +// EditorPaths::create() is responsible for the creation of these directories. String EditorSettings::get_templates_dir() const { return EditorPaths::get_singleton()->get_data_dir().plus_file("templates"); } -// Config directories - String EditorSettings::get_project_settings_dir() const { - return EditorSettings::PROJECT_EDITOR_SETTINGS_PATH; + return EditorPaths::get_singleton()->get_project_data_dir().plus_file("editor"); } String EditorSettings::get_text_editor_themes_dir() const { @@ -1275,8 +1214,6 @@ String EditorSettings::get_project_script_templates_dir() const { return ProjectSettings::get_singleton()->get("editor/script/templates_search_path"); } -// Cache directory - String EditorSettings::get_feature_profiles_dir() const { return EditorPaths::get_singleton()->get_config_dir().plus_file("feature_profiles"); } diff --git a/editor/editor_settings.h b/editor/editor_settings.h index 4c361403a9..d6a9a9d1b9 100644 --- a/editor/editor_settings.h +++ b/editor/editor_settings.h @@ -47,7 +47,6 @@ class EditorSettings : public Resource { _THREAD_SAFE_CLASS_ public: - inline static const String PROJECT_EDITOR_SETTINGS_PATH = "res://.godot/editor"; struct Plugin { EditorPlugin *instance = nullptr; String path; diff --git a/main/main.cpp b/main/main.cpp index 13a9986564..7d4e4ef372 100644 --- a/main/main.cpp +++ b/main/main.cpp @@ -1453,7 +1453,7 @@ Error Main::setup2(Thread::ID p_main_tid_override) { #ifdef TOOLS_ENABLED if (editor || project_manager) { - EditorNode::register_editor_paths(project_manager); + EditorPaths::create(); } #endif @@ -2379,10 +2379,8 @@ bool Main::start() { } // Load SSL Certificates from Editor Settings (or builtin) - Crypto::load_default_certificates(EditorSettings::get_singleton()->get_setting( - "network/ssl/editor_ssl_certificates") - . - operator String()); + Crypto::load_default_certificates( + EditorSettings::get_singleton()->get_setting("network/ssl/editor_ssl_certificates").operator String()); } #endif }