Fix Godot exitcode

[ePirat]

Fix unexpected error exit codes when closing the game.

Fixes #62898 for Windows and macOS as was done in #67421 for Linux already.

Another possibility would be to change how _exit_code is initialized in the OS base class,
but I don't know if this would have undesired side-effects for other platforms.

[dalexeev]

I'm not sure this is the right solution. See:

godot/core/os/os.h

Line 59 in fc99492

int _exit_code = EXIT_FAILURE; // unexpected exit is marked as failure

• Exit MainLoop script with a custom error code godot-proposals#7557
• When the game crashes it still exits with code 0 #30207
• Fix Godot returned status code on unexpected error #38929

[ePirat]

I guess then #67421 is wrong as well?

Though looking at #38929 I do not quite follow what effect the default error value would have there, the comment mentions "unexpected exit" which I do not fully understand what is meant by that. If the application crashes, it would never reach the normal process return case anyway?

Maybe @touilleMan can clarify the reason of the error assigned on init and how this should be fixed differently?

[touilleMan]

If the application crashes, it would never reach the normal process return case anyway?

The issue comes if Godot doesn't crash, but the game is doesn't load either (typically the main scene fails to load because a resource is missing).
Godot is designed around the idea of "keep going when something goes wrong" (from what I understand the idea is that it's better to have a game run with quirks than to have straight crash).
But in our case this strategy means in case the main scene cannot be loaded, the game engine will spit an error log on stderr and carry on, only to discover it has nothing more to do, and exit. So if the exit code is not initialized at an error value, we will end up with a success value returned here.

So I wouldn't be surprised #67421 reintroduced the issue solved by #38929

Also see my comment #38929 (comment), so as long as SceneTree.quit() is called the status code must be correct (given it is provided in as parameter).
So my guess here is closing the game is done is some exotic way instead of relying on SceneTree.quit(), the "correct" to fix this issue would be to investigate this and correct this exotic way of close to rely on SceneTree.quit() (or to set the status code before exiting)

(It's been a long time I haven't work on this, so I might be totally wrong here 😄 )

[akien-mga]

Just an early draft, but after discussing with @mihe who was looking into this, I decided to have a go at changing the default exit code to EXIT_SUCCESS: akien-mga@32a2b1f

I notably changed Main::start() to return the exit code instead of a bool, which makes it much easier to keep track of whether we're returning early with a failure state (set exit code) or not (proceed further, exit code is success by default, or may be overridden by iteration failures).

Haven't really tested it yet, there might well be issues when the application crashes. @mihe and I will try to get to the bottom of this and finally aim to solve all these pesky issues.

[akien-mga]

So I wouldn't be surprised #67421 reintroduced the issue solved by #38929

Did a quick test with my approach, failing to loading a main scene seems to properly report 1 as exit code:

$ godot-git
Godot Engine v4.3.dev.custom_build.32a2b1f6b (2024-03-06 19:39:10 UTC) - https://godotengine.org
Vulkan 1.3.267 - Forward+ - Using Device #1: AMD - AMD Radeon RX 7600M XT (RADV NAVI33)

ERROR: Cannot open file 'res://Node2.tscn'.
   at: load (./scene/resources/resource_format_text.cpp:1656)
ERROR: Failed loading resource: res://Node2.tscn. Make sure resources have been imported by opening the project in the editor at least once.
   at: _load (./core/io/resource_loader.cpp:277)
ERROR: Failed loading scene: res://Node2.tscn.
   at: start (main/main.cpp:3790)
~/.../bug/crashing$ echo $?
1

Needs a lot more testing on all kinds of scenarios where things may fail. I guess I'll open a draft PR and move the discussion there.

[akien-mga]

Let's move the discussion to #89229.

[akien-mga]

Superseded by #83479.

Well this might still be up for discussion actually, since indeed #67421 did this change for Linux.

[ePirat]

@akien-mga #83479 seems like a good fix without messing with the default exit code. If this approach is taken, #67421 can probably be reverted.

Would still be good to make sure in which case the default failure exit code is needed for future reference (and maybe mention it in a comment in the source).

[ePirat]

Closing as #89229 is a more proper way to solve that generally.