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

Godot's C++ modules should be self-contained #569

Closed
Xrayez opened this issue Mar 8, 2020 · 5 comments
Closed

Godot's C++ modules should be self-contained #569

Xrayez opened this issue Mar 8, 2020 · 5 comments

Comments

@Xrayez
Copy link
Contributor

Xrayez commented Mar 8, 2020

Describe the project you are working on:
Developing various Godot modules.

Describe the problem or limitation you are having in your project:
Cannot compile Godot while disabling certain modules, or filled with runtime errors/crashes: Bullet, gdnavigation, glslang (Vulkan).

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
By definition modules should be self-contained, meaning that ideally there should be no other dependencies which make core classes stop functioning without them.

Resolving build and/or runtime errors would allow to disable certain modules in order to decrease resulting binary size and/or speed-up compilation time of editor and export templates.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:

  1. Regarding front-end, some things like ClassDB::register_class() related methods can be moved to modules without much hassle. Currently initialization of some module classes happen alongside core classes.

  2. Regarding back-end, Godot servers may be difficult to make self-contained because they need to be instantiated and/or operated from within the main loop, so at least there must exist dummy implementations which won't cause any run-time errors when you compile modules without them. With the feature added in SCons: Refactor module defines into a generated header, cleanup godot#35963, this process can become much easier.

  3. There should be a list of modules which are treated as core, so that users are instantly aware of them when they attempt to compile the engine without them. For instance, Sconstruct does similar filtering logic for regex module while trying to build editor without regex support.

  4. As a last resort, some of the modules should not be considered as such, because they create enough dependencies with the rest of the codebase, so they need to be moved to core instead.

If this enhancement will not be used often, can it be worked around with a few lines of script?:
I can live with the fact that certain modules can't be disabled. I've made a proposal instead of a general bug report because I've already made some attempts as in godotengine/godot#32272, so just asking whether it makes sense/worth it (either for me or other contributors) to work on making existing modules to be self-contained.

Is there a reason why this should be core and not an add-on in the asset library?:
Just trying to make awareness of this for other contributors making modules or trying to fix existing limitations, consider this to be a call to action and a potential guide. 🙂

@aaronfranke
Copy link
Member

Related: #190. Would really be nice for more modules to be made optional and not depended upon, since it would mean that we could potentially disable more of the engine for specific use cases.

@nonunknown
Copy link

Maybe related to #573

@vnen
Copy link
Member

vnen commented Mar 12, 2020

Is this proposal really needed? If a module can't be disabled on compilation, it's a bug. The reports you mention are already tagged as bug. If there's anything specific we can act on, then just open a regular bug report and we'll try to solve it.

The only thing is that some modules can't be disabled (like FreeType, cf. godotengine/godot#28650) so maybe those should be in a special category or moved to core. But again, should be on a case-by-case basis, I don't see what else we can do to make this proposal work.


  1. Currently initialization of some module classes happen alongside core classes.

This is a bug, should be reported as such and fixed.

2. Regarding back-end, Godot servers may be difficult to make self-contained because they need to be instantiated and/or operated from within the main loop

Is there any server on module that is being initialized outside the module code? The doc page you mention on how to create custom servers don't need to touch anything outside the module.

3. There should be a list of modules which are treated as core, so that users are instantly aware of them when they attempt to compile the engine without them. For instance, Sconstruct does similar filtering logic for regex module while trying to build editor without regex support.

I believe this is only for the editor, which needs many of the modules to work properly. The export template should be able to disable everything (again, currently FreeType is an exception).

4. As a last resort, some of the modules should not be considered as such, because they create enough dependencies with the rest of the codebase, so they need to be moved to core instead.

Agree, but do we have any example?

@Xrayez
Copy link
Contributor Author

Xrayez commented Mar 12, 2020

But again, should be on a case-by-case basis, I don't see what else we can do to make this proposal work.

IMO, that's the problem of GIP, they don't let you track specific problems associated with common problems which are "not a bug, but a feature".

This is a bug, should be reported as such and fixed.

Already tried to fix some problems such as in godotengine/godot#32272, still having issues. I wouldn't create this proposal otherwise. 4.1 should receive some treatment to this I suppose, but
even if I have to wait a year for bugs to be fixed, that's still a problem IMO.

Is there any server on module that is being initialized outside the module code? The doc page you mention on how to create custom servers don't need to touch anything outside the module.

Physics and Navigation servers, where both implementations are provided by bullet and gdnavigation modules.

https://github.com/godotengine/godot/blob/b8ddaf9c33107e01928e04ed462aa08d4017247b/main/main.cpp#L171-L214

Well I think I just needed someone to tell me that's a bug, so thanks for clarification on this, I'll just close this.

The issue is still open for discussion on philosophical principles of module development though. 🙂

@Xrayez
Copy link
Contributor Author

Xrayez commented Mar 12, 2020

By the way, I want to add ability to compile custom/user/external C++ modules, and here's a nice way to check whether modules have core dependencies:

godotengine/godot#36922 (comment)

@vnen

  1. As a last resort, some of the modules should not be considered as such, because they create enough dependencies with the rest of the codebase, so they need to be moved to core instead.

Agree, but do we have any example?

It seems like GDNative module cannot be compiled outside of Godot, not sure if it has something to do with the way include paths are written on the GDNative source level.

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

No branches or pull requests

5 participants