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

Err when trying to build the editor without its required modules #74980

Conversation

YuriSizov
Copy link
Contributor

See #74546 (comment). I didn't add the same check for the text server being disabled, because you can extend it with a third party module instead, and I don't think we can detect that there is no text server implementation whatsoever during this validation step.

@ajreckof
Copy link
Member

ajreckof commented Mar 31, 2023

I really like what is done here but i think there is way more modules that are needed for the editor not to crash. I tried a bit, with only the two modules checked here plus the text server, it crashes. So tried to find which ones are needed but could only find that it needed a language (added gdscript). After that vulkan driver complains about no pipeline being set.
Not sure which module is needed for the pipeline and the driver part of the source code is too hard for me.
Capture d’écran 2023-03-31 à 08 37 47

@akien-mga
Copy link
Member

This check used to exist (still does in 3.x) but was removed in 951a101, possibly as an oversight. CC @Faless

Makes sense to add back.

I really like what is done here but i think there is way more modules that are needed for the editor not to crash. I tried a bit, with only the two modules checked here plus the text server, it crashes. So tried to find which ones are needed but could only find that it needed a language (added gdscript). After that vulkan driver complains about no pipeline being set.
Not sure which module is needed for the pipeline and the driver part of the source code is too hard for me.

The editor should be able to run without GDScript, as long as you're not trying to run GDScript code. So if it doesn't, this should be fixed.

Likewise for rendering, it should work in headless mode if no valid renderer is compiled, so this would be worth looking into separately.

@YuriSizov
Copy link
Contributor Author

Makes sense to add back.

Do you want me to try and restore it as it was, without hardcoding these two specific dependencies?

@akien-mga
Copy link
Member

Yeah I think the old format was a bit more flexible to add more modules to the array.

In theory the module dependency graph added by Fabio could be used for this too but since the editor isn't a module, it doesn't run this check.

@YuriSizov YuriSizov marked this pull request as draft April 3, 2023 10:27
@YuriSizov YuriSizov force-pushed the build-err-without-required-editor-modules branch from bf2d06e to a145194 Compare April 7, 2023 17:28
@YuriSizov
Copy link
Contributor Author

YuriSizov commented Apr 7, 2023

Okay, made the changes. The output looks something like this now:

scons: Reading SConscript files ...
Found MSVC version 14.2, arch x86_64
Building for platform "windows", architecture "x86_64", target "editor".
NOTE: Developer build, with debug optimization level and debug symbols (unless overridden).
Disabling 'editor' module as the following dependencies are not satisfied: freetype, svg
Not all modules required by editor builds are enabled.

We still need to explicitly list the dependencies, if I understand correctly. I don't see that being done in the old version, I'm not sure it even worked?

@YuriSizov YuriSizov marked this pull request as ready for review April 7, 2023 17:28
@akien-mga akien-mga merged commit c16821e into godotengine:master Apr 26, 2023
@akien-mga
Copy link
Member

Thanks!

@YuriSizov YuriSizov deleted the build-err-without-required-editor-modules branch April 26, 2023 09:43
@YuriSizov
Copy link
Contributor Author

Cherry-picked for 4.0.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants