Refactor OS exit code to be EXIT_SUCCESS by default

- `Main::setup` early exits (failure or `--help`/`--version`) now
  consistently return `EXIT_FAILURE` or `EXIT_SUCCESS` on all platforms,
  instead of 255 on some and a Godot Error code on others.
- `Main::start` now returns the exit code, simplifying the handling of early
  failures.
- `Main::iteration` needs to explicit set the exit code in OS if it errors
  out.
- Web and iOS now properly return `OS::get_exit_code()` instead of 0.
This commit is contained in:
Rémi Verschelde 2024-03-06 18:50:35 +01:00
parent ffc41fb76d
commit 023dcd44c1
No known key found for this signature in database
GPG Key ID: C3336907360768E1
8 changed files with 82 additions and 86 deletions

View File

@ -56,7 +56,8 @@ class OS {
bool _verbose_stdout = false;
bool _debug_stdout = false;
String _local_clipboard;
int _exit_code = EXIT_FAILURE; // unexpected exit is marked as failure
// Assume success by default, all failure cases need to set EXIT_FAILURE explicitly.
int _exit_code = EXIT_SUCCESS;
bool _allow_hidpi = false;
bool _allow_layered = false;
bool _stdout_enabled = true;

View File

@ -766,6 +766,7 @@ Error Main::test_setup() {
return OK;
}
// The order is the same as in `Main::cleanup()`.
void Main::test_cleanup() {
ERR_FAIL_COND(!_start_success);
@ -977,8 +978,9 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph
packed_data->add_pack_source(zip_packed_data);
#endif
// Default exit code, can be modified for certain errors.
Error exit_code = ERR_INVALID_PARAMETER;
// Exit error code used in the `goto error` conditions.
// It's returned as the program exit code. ERR_HELP is special cased and handled as success (0).
Error exit_err = ERR_INVALID_PARAMETER;
I = args.front();
while (I) {
@ -1028,12 +1030,12 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph
} else if (I->get() == "-h" || I->get() == "--help" || I->get() == "/?") { // display help
show_help = true;
exit_code = ERR_HELP; // Hack to force an early exit in `main()` with a success code.
exit_err = ERR_HELP; // Hack to force an early exit in `main()` with a success code.
goto error;
} else if (I->get() == "--version") {
print_line(get_full_version_string());
exit_code = ERR_HELP; // Hack to force an early exit in `main()` with a success code.
exit_err = ERR_HELP; // Hack to force an early exit in `main()` with a success code.
goto error;
} else if (I->get() == "-v" || I->get() == "--verbose") { // verbose output
@ -2459,7 +2461,7 @@ error:
OS::get_singleton()->finalize_core();
locale = String();
return exit_code;
return exit_err;
}
Error _parse_resource_dummy(void *p_data, VariantParser::Stream *p_stream, Ref<Resource> &r_res, int &line, String &r_err_str) {
@ -3139,7 +3141,10 @@ String Main::get_rendering_driver_name() {
// everything the main loop needs to know about frame timings
static MainTimerSync main_timer_sync;
bool Main::start() {
// Return value should be EXIT_SUCCESS if we start successfully
// and should move on to `OS::run`, and EXIT_FAILURE otherwise for
// an early exit with that error code.
int Main::start() {
ERR_FAIL_COND_V(!_start_success, false);
bool has_icon = false;
@ -3276,7 +3281,7 @@ bool Main::start() {
{
Ref<DirAccess> da = DirAccess::open(doc_tool_path);
ERR_FAIL_COND_V_MSG(da.is_null(), false, "Argument supplied to --doctool must be a valid directory path.");
ERR_FAIL_COND_V_MSG(da.is_null(), EXIT_FAILURE, "Argument supplied to --doctool must be a valid directory path.");
}
#ifndef MODULE_MONO_ENABLED
@ -3311,11 +3316,11 @@ bool Main::start() {
// Create the module documentation directory if it doesn't exist
Ref<DirAccess> da = DirAccess::create_for_path(path);
err = da->make_dir_recursive(path);
ERR_FAIL_COND_V_MSG(err != OK, false, "Error: Can't create directory: " + path + ": " + itos(err));
ERR_FAIL_COND_V_MSG(err != OK, EXIT_FAILURE, "Error: Can't create directory: " + path + ": " + itos(err));
print_line("Loading docs from: " + path);
err = docsrc.load_classes(path);
ERR_FAIL_COND_V_MSG(err != OK, false, "Error loading docs from: " + path + ": " + itos(err));
ERR_FAIL_COND_V_MSG(err != OK, EXIT_FAILURE, "Error loading docs from: " + path + ": " + itos(err));
}
}
@ -3323,11 +3328,11 @@ bool Main::start() {
// Create the main documentation directory if it doesn't exist
Ref<DirAccess> da = DirAccess::create_for_path(index_path);
err = da->make_dir_recursive(index_path);
ERR_FAIL_COND_V_MSG(err != OK, false, "Error: Can't create index directory: " + index_path + ": " + itos(err));
ERR_FAIL_COND_V_MSG(err != OK, EXIT_FAILURE, "Error: Can't create index directory: " + index_path + ": " + itos(err));
print_line("Loading classes from: " + index_path);
err = docsrc.load_classes(index_path);
ERR_FAIL_COND_V_MSG(err != OK, false, "Error loading classes from: " + index_path + ": " + itos(err));
ERR_FAIL_COND_V_MSG(err != OK, EXIT_FAILURE, "Error loading classes from: " + index_path + ": " + itos(err));
checked_paths.insert(index_path);
print_line("Merging docs...");
@ -3336,20 +3341,19 @@ bool Main::start() {
for (const String &E : checked_paths) {
print_line("Erasing old docs at: " + E);
err = DocTools::erase_classes(E);
ERR_FAIL_COND_V_MSG(err != OK, false, "Error erasing old docs at: " + E + ": " + itos(err));
ERR_FAIL_COND_V_MSG(err != OK, EXIT_FAILURE, "Error erasing old docs at: " + E + ": " + itos(err));
}
print_line("Generating new docs...");
err = doc.save_classes(index_path, doc_data_classes);
ERR_FAIL_COND_V_MSG(err != OK, false, "Error saving new docs:" + itos(err));
ERR_FAIL_COND_V_MSG(err != OK, EXIT_FAILURE, "Error saving new docs:" + itos(err));
print_line("Deleting docs cache...");
if (FileAccess::exists(EditorHelp::get_cache_full_path())) {
DirAccess::remove_file_or_error(EditorHelp::get_cache_full_path());
}
OS::get_singleton()->set_exit_code(EXIT_SUCCESS);
return false;
return EXIT_SUCCESS;
}
// GDExtension API and interface.
@ -3364,32 +3368,24 @@ bool Main::start() {
}
if (dump_gdextension_interface || dump_extension_api) {
OS::get_singleton()->set_exit_code(EXIT_SUCCESS);
return false;
return EXIT_SUCCESS;
}
if (validate_extension_api) {
Engine::get_singleton()->set_editor_hint(true); // "extension_api.json" should always contains editor singletons.
bool valid = GDExtensionAPIDump::validate_extension_json_file(validate_extension_api_file) == OK;
OS::get_singleton()->set_exit_code(valid ? EXIT_SUCCESS : EXIT_FAILURE);
return false;
return valid ? EXIT_SUCCESS : EXIT_FAILURE;
}
}
#ifndef DISABLE_DEPRECATED
if (converting_project) {
int ret = ProjectConverter3To4(converter_max_kb_file, converter_max_line_length).convert();
if (ret) {
OS::get_singleton()->set_exit_code(EXIT_SUCCESS);
}
return false;
return ret ? EXIT_SUCCESS : EXIT_FAILURE;
}
if (validating_converting_project) {
bool ret = ProjectConverter3To4(converter_max_kb_file, converter_max_line_length).validate_conversion();
if (ret) {
OS::get_singleton()->set_exit_code(EXIT_SUCCESS);
}
return false;
return ret ? EXIT_SUCCESS : EXIT_FAILURE;
}
#endif // DISABLE_DEPRECATED
@ -3406,7 +3402,7 @@ bool Main::start() {
// this might end up triggered by valid usage, in which case we'll have to
// fine-tune further.
OS::get_singleton()->alert("Couldn't detect whether to run the editor, the project manager or a specific project. Aborting.");
ERR_FAIL_V_MSG(false, "Couldn't detect whether to run the editor, the project manager or a specific project. Aborting.");
ERR_FAIL_V_MSG(EXIT_FAILURE, "Couldn't detect whether to run the editor, the project manager or a specific project. Aborting.");
}
#endif
@ -3420,15 +3416,10 @@ bool Main::start() {
if (!script.is_empty()) {
Ref<Script> script_res = ResourceLoader::load(script);
ERR_FAIL_COND_V_MSG(script_res.is_null(), false, "Can't load script: " + script);
ERR_FAIL_COND_V_MSG(script_res.is_null(), EXIT_FAILURE, "Can't load script: " + script);
if (check_only) {
if (!script_res->is_valid()) {
OS::get_singleton()->set_exit_code(EXIT_FAILURE);
} else {
OS::get_singleton()->set_exit_code(EXIT_SUCCESS);
}
return false;
return script_res->is_valid() ? EXIT_SUCCESS : EXIT_FAILURE;
}
if (script_res->can_instantiate()) {
@ -3440,13 +3431,13 @@ bool Main::start() {
memdelete(obj);
}
OS::get_singleton()->alert(vformat("Can't load the script \"%s\" as it doesn't inherit from SceneTree or MainLoop.", script));
ERR_FAIL_V_MSG(false, vformat("Can't load the script \"%s\" as it doesn't inherit from SceneTree or MainLoop.", script));
ERR_FAIL_V_MSG(EXIT_FAILURE, vformat("Can't load the script \"%s\" as it doesn't inherit from SceneTree or MainLoop.", script));
}
script_loop->set_script(script_res);
main_loop = script_loop;
} else {
return false;
return EXIT_FAILURE;
}
} else { // Not based on script path.
if (!editor && !ClassDB::class_exists(main_loop_type) && ScriptServer::is_global_class(main_loop_type)) {
@ -3454,7 +3445,7 @@ bool Main::start() {
Ref<Script> script_res = ResourceLoader::load(script_path);
if (script_res.is_null()) {
OS::get_singleton()->alert("Error: Could not load MainLoop script type: " + main_loop_type);
ERR_FAIL_V_MSG(false, vformat("Could not load global class %s.", main_loop_type));
ERR_FAIL_V_MSG(EXIT_FAILURE, vformat("Could not load global class %s.", main_loop_type));
}
StringName script_base = script_res->get_instance_base_type();
Object *obj = ClassDB::instantiate(script_base);
@ -3464,7 +3455,7 @@ bool Main::start() {
memdelete(obj);
}
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));
ERR_FAIL_V_MSG(EXIT_FAILURE, vformat("The global class %s does not inherit from SceneTree or MainLoop.", main_loop_type));
}
script_loop->set_script(script_res);
main_loop = script_loop;
@ -3478,15 +3469,15 @@ bool Main::start() {
if (!main_loop) {
if (!ClassDB::class_exists(main_loop_type)) {
OS::get_singleton()->alert("Error: MainLoop type doesn't exist: " + main_loop_type);
return false;
return EXIT_FAILURE;
} else {
Object *ml = ClassDB::instantiate(main_loop_type);
ERR_FAIL_NULL_V_MSG(ml, false, "Can't instance MainLoop type.");
ERR_FAIL_NULL_V_MSG(ml, EXIT_FAILURE, "Can't instance MainLoop type.");
main_loop = Object::cast_to<MainLoop>(ml);
if (!main_loop) {
memdelete(ml);
ERR_FAIL_V_MSG(false, "Invalid MainLoop type.");
ERR_FAIL_V_MSG(EXIT_FAILURE, "Invalid MainLoop type.");
}
}
}
@ -3610,7 +3601,7 @@ bool Main::start() {
Error err;
Vector<String> paths = get_files_with_extension(gdscript_docs_path, "gd");
ERR_FAIL_COND_V_MSG(paths.is_empty(), false, "Couldn't find any GDScript files under the given directory: " + gdscript_docs_path);
ERR_FAIL_COND_V_MSG(paths.is_empty(), EXIT_FAILURE, "Couldn't find any GDScript files under the given directory: " + gdscript_docs_path);
for (const String &path : paths) {
Ref<GDScript> gdscript = ResourceLoader::load(path);
@ -3625,14 +3616,13 @@ bool Main::start() {
Ref<DirAccess> da = DirAccess::create_for_path(doc_tool_path);
err = da->make_dir_recursive(doc_tool_path);
ERR_FAIL_COND_V_MSG(err != OK, false, "Error: Can't create GDScript docs directory: " + doc_tool_path + ": " + itos(err));
ERR_FAIL_COND_V_MSG(err != OK, EXIT_FAILURE, "Error: Can't create GDScript docs directory: " + doc_tool_path + ": " + itos(err));
HashMap<String, String> doc_data_classes;
err = docs.save_classes(doc_tool_path, doc_data_classes, false);
ERR_FAIL_COND_V_MSG(err != OK, false, "Error saving GDScript docs:" + itos(err));
ERR_FAIL_COND_V_MSG(err != OK, EXIT_FAILURE, "Error saving GDScript docs:" + itos(err));
OS::get_singleton()->set_exit_code(EXIT_SUCCESS);
return false;
return EXIT_SUCCESS;
}
#endif // MODULE_GDSCRIPT_ENABLED
@ -3747,7 +3737,7 @@ bool Main::start() {
if (sep == -1) {
Ref<DirAccess> da = DirAccess::create(DirAccess::ACCESS_FILESYSTEM);
ERR_FAIL_COND_V(da.is_null(), false);
ERR_FAIL_COND_V(da.is_null(), EXIT_FAILURE);
local_game_path = da->get_current_dir().path_join(local_game_path);
} else {
@ -3797,7 +3787,7 @@ bool Main::start() {
scene = scenedata->instantiate();
}
ERR_FAIL_NULL_V_MSG(scene, false, "Failed loading scene: " + local_game_path + ".");
ERR_FAIL_NULL_V_MSG(scene, EXIT_FAILURE, "Failed loading scene: " + local_game_path + ".");
sml->add_current_scene(scene);
#ifdef MACOS_ENABLED
@ -3870,7 +3860,7 @@ bool Main::start() {
OS::get_singleton()->benchmark_end_measure("Startup", "Total");
OS::get_singleton()->benchmark_dump();
return true;
return EXIT_SUCCESS;
}
/* Main iteration
@ -3900,10 +3890,10 @@ static uint64_t physics_process_max = 0;
static uint64_t process_max = 0;
static uint64_t navigation_process_max = 0;
// Return false means iterating further, returning true means `OS::run`
// will terminate the program. In case of failure, the OS exit code needs
// to be set explicitly here (defaults to EXIT_SUCCESS).
bool Main::iteration() {
//for now do not error on this
//ERR_FAIL_COND_V(iterating, false);
iterating++;
const uint64_t ticks = OS::get_singleton()->get_ticks_usec();

View File

@ -78,7 +78,7 @@ public:
static Error test_setup();
static void test_cleanup();
#endif
static bool start();
static int start();
static bool iteration();
static void force_redraw();

View File

@ -102,15 +102,16 @@ int ios_main(int argc, char **argv) {
Error err = Main::setup(fargv[0], argc - 1, &fargv[1], false);
if (err == ERR_HELP) { // Returned by --help and --version, so success.
return 0;
} else if (err != OK) {
return 255;
if (err != OK) {
if (err == ERR_HELP) { // Returned by --help and --version, so success.
return EXIT_SUCCESS;
}
return EXIT_FAILURE;
}
os->initialize_modules();
return 0;
return os->get_exit_code();
}
void ios_finish() {

View File

@ -72,18 +72,19 @@ int main(int argc, char *argv[]) {
char *ret = getcwd(cwd, PATH_MAX);
Error err = Main::setup(argv[0], argc - 1, &argv[1]);
if (err != OK) {
free(cwd);
if (err == ERR_HELP) { // Returned by --help and --version, so success.
return 0;
return EXIT_SUCCESS;
}
return 255;
return EXIT_FAILURE;
}
if (Main::start()) {
os.set_exit_code(EXIT_SUCCESS);
os.run(); // it is actually the OS that decides how to run
if (Main::start() == EXIT_SUCCESS) {
os.run();
} else {
os.set_exit_code(EXIT_FAILURE);
}
Main::cleanup();

View File

@ -69,18 +69,21 @@ int main(int argc, char **argv) {
err = Main::setup(argv[0], argc - first_arg, &argv[first_arg]);
}
if (err == ERR_HELP) { // Returned by --help and --version, so success.
return 0;
} else if (err != OK) {
return 255;
if (err != OK) {
if (err == ERR_HELP) { // Returned by --help and --version, so success.
return EXIT_SUCCESS;
}
return EXIT_FAILURE;
}
bool ok;
int ret;
@autoreleasepool {
ok = Main::start();
ret = Main::start();
}
if (ok) {
os.run(); // It is actually the OS that decides how to run.
if (ret) {
os.run();
} else {
os.set_exit_code(EXIT_FAILURE);
}
@autoreleasepool {

View File

@ -109,24 +109,22 @@ extern EMSCRIPTEN_KEEPALIVE int godot_web_main(int argc, char *argv[]) {
// Proper shutdown in case of setup failure.
if (err != OK) {
int exit_code = (int)err;
if (err == ERR_HELP) {
exit_code = 0; // Called with --help.
}
os->set_exit_code(exit_code);
// Will only exit after sync.
emscripten_set_main_loop(exit_callback, -1, false);
godot_js_os_finish_async(cleanup_after_sync);
return exit_code;
if (err == ERR_HELP) { // Returned by --help and --version, so success.
return EXIT_SUCCESS;
}
return EXIT_FAILURE;
}
os->set_exit_code(0);
main_started = true;
// Ease up compatibility.
ResourceLoader::set_abort_on_missing_resources(false);
Main::start();
int ret = Main::start();
os->set_exit_code(ret);
os->get_main_loop()->initialize();
#ifdef TOOLS_ENABLED
if (Engine::get_singleton()->is_project_manager_hint() && FileAccess::exists("/tmp/preload.zip")) {
@ -140,5 +138,5 @@ extern EMSCRIPTEN_KEEPALIVE int godot_web_main(int argc, char *argv[]) {
// We are inside an animation frame, we want to immediately draw on the newly setup canvas.
main_loop_callback();
return 0;
return os->get_exit_code();
}

View File

@ -171,13 +171,15 @@ int widechar_main(int argc, wchar_t **argv) {
delete[] argv_utf8;
if (err == ERR_HELP) { // Returned by --help and --version, so success.
return 0;
return EXIT_SUCCESS;
}
return 255;
return EXIT_FAILURE;
}
if (Main::start()) {
if (Main::start() == EXIT_SUCCESS) {
os.run();
} else {
os.set_exit_code(EXIT_FAILURE);
}
Main::cleanup();