Refactor main.cpp setup #49362
Refactor main.cpp setup #49362
Conversation
Initialization process broken up into multiple parts. - Parsing command line - Loading project settings - Finalizing configuration - Initialization - Starting (running tool or creating main loop)
akien-mga
requested review from
akien-mga and
Calinou
and removed request for
a team
|
Just mentioning that this is on my radar, I'll need time to do a thorough review and testing, but in general I like the direction it's going, and it's extremely needed. |
That's good to hear. My intent was to give this a shot and see how far I can go without actually changing all that much. That means there is room for some changes in command line or initialization logic. But I think it's better if this PR is mostly just a refactor, that should make it easier to implement actual changes later on. |
|
The PR looks great! I want only leave a note that #44594 could be using for parsing in |
It probably could and I would be interested to see it used on Godot args. But sadly, it seems like we are kind of stuck in limbo at the moment. On the one hand, the devs have not been too outspoken about a separate parser, so it doesn't make sense for me to use it in this PR until they change their minds. On the other hand, parsing Godot args would be a good use case for your parser and it would not be possible without some major refactoring to main.cpp like in this PR. So, I am not sure what is the best thing to do. |
|
@akien-mga I have tried rebasing but it's kind of a nightmare as you can imagine. What do you think should happen with this PR? Frankly, I find it strange that there hasn't been any feedback from anyone in two months, given what I outlined in the PR and what has been said about the state of |
|
Sorry for the lack of update, with 20 PRs per day and 1200 PRs of backlog my attention is spread thin. As I mentioned previously, I think this goes in the right direction and I want to take time to review in depth and merge at least parts if not all of it. But we discussed with @reduz a few weeks ago that it's something that should wait for once 4.0 has been released, as we're too busy with other tasks for now. So we'll revisit that once we can give it more attention and make sure that everything works as intended, without the high entropy of a daily refactored Until then, it might be tedious to rebase this regularly so I would suggest leaving it in limbo for some months until we're ready to work on it together. |
|
I understand you are busy and I am fine with that decision, I just needed some clarity. |
|
@stijn-h Will you be able to work on this PR? It needs to be rebased before it can be considered. Considering it has not been updated for over 2 years, it may need to be remade from scratch. |
This PR refactors the setup code in main.cpp. When looking to contribute to a project, I always start with the program entrypoint. I think main.cpp being a bit of a mess sets off a lot of new contributors. It's easy to get lost in chunks of code with overlapping logic and the dependencies between various singletons isn't conveyed very well. I hope this makes it easier for new users to understand the engine.
Because things were so tangled up, it wasn't easy to isolate a specific part and just refactor that. So in the end, most of the setup code has been refactored, but only a few intentional changes have been introduced. That increases the chance of a regression, so it should obviously be tested in as many ways as possible.
Main goals
The PR mostly restructures and groups code in a way that makes it easier to understand program flow and dependencies between certain classes. I think the code should speak for itself, but a short explanation might help with reviewing, and if we at least agree on these principles, it makes it easier to discuss the implementation.
Separating distinct phases.
While the interface of main was easy to understand, under the hood all kinds of things were blended together. I have tried to separate the code in Main into the following stages:
Clear setup based on application type.
To me, I felt that it's important to understand what kind of application we want to run (project, script, editor, project manager, a tool) to understandi the whole initialization process. Currently, it's not clear until the very last moment. It also seems like most code assumes that we are running the editor or project manager, while running a project is somewhat of an afterthought. I have made the application type explicit and it is specified as early as possible.
Important here is the ApplicationConfiguration, which holds most of the variables for initializing singletons. It first gets initialized in step 1. Then, after loading project settings, it is manually adjusted in step 3, after which it doesn't have to change anymore. That leaves us with an invariant configuration for the rest of the program, which makes the code much simpler to understand. Not much has fundamentally changed to step 4 (initialization), because I don't have enough knowledge of this part. Step 6 has not changed at all.
Intentional changes
In the CLI, there were a few arguments that only had meaning in combination with other arguments. They have been changed so that they are optional parameters to those arguments.
--export-debug,--export-packare no longer separate commands. Instead you use--export <preset> <path> [option], where option can be--export-debugor--export-pack. Options can be combined in any order.--no-docbaseis no longer a separate command. Instead you use--doc-tool <path> --no-docbase.--check-onlyis no longer a separate command. Instead you use--script <path> --check-only.EditorSettings is now created in main.cpp instead of duplicated in Editor and PM.
A few open questions
Things that are not in this PR, but could be looked into in the future: