Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Main: Fixup bogus fallback to project manager with more bolognese #49661

Merged

Conversation

akien-mga
Copy link
Member

WARNING: Hacks everywhere!

The logic in main.cpp is due a full rewrite as it's extremely hacky,
splitting argument parsing over several functions, with a mess of global state
and assumptions about what combinations of arguments or lack thereof should
mean in terms of what we want to read: game, editor, project manager, or
command line tools such as --doctool, --export or --script.

Until this is fully rewritten, this patch hacks things some more to ensure
that we don't fall back to the project manager in cases where it's not
warranted, and especially not too late, as it can mean that we haven't
properly initialized stuff like EditorPaths needed by the PM (which in turn
impacts what kind of path will be used for logs and the shader cache, etc...
the rabbit hole goes deep).

Fixes #41435.
Fixes #49392.
Fixes #49658.
Fixes #38202 (comment).

xkcd: Data Pipeline


This has a lot of potential for regressions as this code is really brittle, and my preferred approach to making things better (and fixing potential regressions) would be to rewrite the whole thing from scratch, which I might do soon, after reviewing some pending PRs which tried to poke at this mess.

WARNING: Hacks everywhere!

The logic in `main.cpp` is due a full rewrite as it's extremely hacky,
splitting argument parsing over several functions, with a mess of global state
and assumptions about what combinations of arguments or lack thereof should
mean in terms of what we want to read: game, editor, project manager, or
command line tools such as `--doctool`, `--export` or `--script`.

Until this is fully rewritten, this patch hacks things some more to ensure
that we don't fall back to the project manager in cases where it's not
warranted, and especially not *too late*, as it can mean that we haven't
properly initialized stuff like `EditorPaths` needed by the PM (which in turn
impacts what kind of path will be used for logs and the shader cache, etc...
the rabbit hole goes deep).

Fixes godotengine#41435.
Fixes godotengine#49392.
Fixes godotengine#49658.
Fixes godotengine#38202 (comment).
@AaronRecord
Copy link
Contributor

Is #49362 relevant?

@akien-mga
Copy link
Member Author

Is #49362 relevant?

Yeah definitely, I haven't had time to review it yet but it seems to be going in the right direction to fix all this mess. My PR here just adds a tiny amount of additional code smell to fix regressions, but once reviewed and ideally merged, #49362 will supersede it.

@akien-mga akien-mga merged commit afd4cc7 into godotengine:master Jun 17, 2021
@akien-mga akien-mga deleted the main-fallback-to-projectmanager branch June 17, 2021 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants