Style: Convert *.gen.inc to *.hpp for ninja#1927
Style: Convert *.gen.inc to *.hpp for ninja#1927Repiteo wants to merge 1 commit intogodotengine:masterfrom
*.gen.inc to *.hpp for ninja#1927Conversation
Ivorforce
left a comment
There was a problem hiding this comment.
Works for me.
For godot-cpp we're trying to have a bit of a higher source code compat baseline than in the main repo, but I'm not sure how common it is to include gdvirtual manually.
The best workaround I can think of is still generating the .gen.inc, but making it an empty file except for a gdvirtual.hpp include. Not sure that's necessary.
dsnopek
left a comment
There was a problem hiding this comment.
Thanks!
Overall, I think this change is fine with regards to compatibility, especially, since we're still in pre-release mode for godot-cpp v10.
On the one hand, it does seem a little weird for developers to have to include this file directly (unlike Godot where it's included through some other common header). But on the other hand, it shouldn't be included if it's unused for better compile times, so I guess this is right. And sense developer are including it directly, it makes the *.hpp suffix even more appropriate
|
|
||
| #include <godot_cpp/core/binder_common.hpp> | ||
| #include <godot_cpp/core/gdvirtual.gen.inc> | ||
| #include <godot_cpp/core/gdvirtual.hpp> |
There was a problem hiding this comment.
Given that this file is called gdvirtual.gen.h in Godot, I think I'd prefer we call it gdvirtual.gen.hpp in godot-cpp (with the gen in there)
There was a problem hiding this comment.
The .gen is only a convention because it lets us automatically ignore those files in Git. Because we already have a generated folder, the files are already ignored, so there's no need for it to use .gen.hpp
There was a problem hiding this comment.
I'm not thinking about the files in the file system, but developers renaming their includes between a module and godot-cpp
I feel like it's slightly less confusing to have to switch between #include "core/object/gdvirtual.gen.h and #include <godot-cpp/core/gdvirtual.gen.hpp> without having to remember that you also need to drop the gen part for some reason
| files.append(str((source_gen_folder / "gdextension_interface_loader.cpp").as_posix())) | ||
| files.append(str((core_gen_folder / "ext_wrappers.gen.inc").as_posix())) | ||
| files.append(str((core_gen_folder / "gdvirtual.gen.inc").as_posix())) | ||
| files.append(str((core_gen_folder / "ext_wrappers.hpp").as_posix())) |
There was a problem hiding this comment.
Same here for ext_wrappers.gen.hpp
*.gen.incto*.gen.hfor ninja godot#115935While less pertinent on godot-cpp, the same issue could theoretically occur, so this moves away from the
.incsyntax for generated headers. However, the context of generating files in godot-cpp is much different than godot, so the extension is.hppinstead of.gen.h, as the.genparadigm isn't needed when we have a dedicated folder for generated files