Merge pull request #89229 from akien-mga/main-refactor-os-exit-code

Refactor OS exit code to be `EXIT_SUCCESS` by default

core/os/os.h

@@ -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;

main/main.cpp

@@ -767,6 +767,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);
@@ -978,8 +979,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) {
@@ -1029,12 +1031,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
@@ -2461,7 +2463,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) {
@@ -3143,7 +3145,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;
@@ -3280,7 +3285,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
@@ -3315,11 +3320,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));
 			}
 		}
 
@@ -3327,11 +3332,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...");
@@ -3340,20 +3345,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.
@@ -3368,32 +3372,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
 
@@ -3410,7 +3406,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
 
@@ -3424,15 +3420,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()) {
@@ -3444,13 +3435,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)) {
@@ -3458,7 +3449,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);
@@ -3468,7 +3459,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;
@@ -3482,15 +3473,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.");
 			}
 		}
 	}
@@ -3614,7 +3605,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);
@@ -3629,14 +3620,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
 
@@ -3751,7 +3741,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 {
@@ -3801,7 +3791,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
@@ -3874,7 +3864,7 @@ bool Main::start() {
 	OS::get_singleton()->benchmark_end_measure("Startup", "Total");
 	OS::get_singleton()->benchmark_dump();
 
-	return true;
+	return EXIT_SUCCESS;
 }
 
 /* Main iteration
@@ -3904,10 +3894,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();

main/main.h

@@ -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();

platform/ios/godot_ios.mm

@@ -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() {

platform/linuxbsd/godot_linuxbsd.cpp

@@ -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();
 

platform/macos/godot_main_macos.mm

@@ -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 {

platform/web/web_main.cpp

@@ -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();
 }

platform/windows/godot_windows.cpp

@@ -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();