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

[Editor] Separate editor_node.cpp for SCU to prevent errors #96522

Closed

Conversation

AThousandShips
Copy link
Member

Due to defines introduced by platform/windows/platform_gl.h SCU builds break on MSVC, alternative solution would be to explicitly udefine like this, but felt this was the least hacky solution:

diff --git a/editor/editor_node.cpp b/editor/editor_node.cpp
index 32e6126225..54f3715bf7 100644
--- a/editor/editor_node.cpp
+++ b/editor/editor_node.cpp
@@ -169,6 +169,13 @@

 #if defined(GLES3_ENABLED)
 #include "drivers/gles3/rasterizer_gles3.h"
+
+// Cleanup nasty defines.
+#undef CreateDialog
+#undef ERROR
+#undef MOD_ALT
+#undef MOD_SHIFT
+#undef MOD_META
 #endif

 EditorNode *EditorNode::singleton = nullptr;
diff --git a/scu_builders.py b/scu_builders.py
index 041056fd68..fc5461196f 100644
--- a/scu_builders.py
+++ b/scu_builders.py
@@ -290,7 +290,7 @@ def generate_scu_files(max_includes_per_scu):
     process_folder(["drivers/unix"])
     process_folder(["drivers/png"])

-    process_folder(["editor"], ["file_system_dock", "editor_resource_preview", "editor_node"], 32)
+    process_folder(["editor"], ["file_system_dock", "editor_resource_preview"], 32)
     process_folder(["editor/debugger"])
     process_folder(["editor/debugger/debug_adapter"])
     process_folder(["editor/export"])

Or there might be some way to prevent these weird defines being made, but this should be the most simple solution

@AThousandShips
Copy link
Member Author

As discussed in:

A more complete fix would be to separate out this functionality into a separate OS function, but this will fix SCU builds in the meantime (and editor_node.cpp is pretty huge anyway so I don't think it's critical to keep it in the same SCU block)

Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea what it does, but it fixes the problem.

@AThousandShips
Copy link
Member Author

AThousandShips commented Sep 3, 2024

We'd have to see if we want this separate in SCU builds or not (I haven't compared but I would suspect that this improves general compilation anyway when changing small files in the editor directory, as this file is so big)

Testing and it seems that the time to recompile is essentially unchanged if editing another file sharing the same SCU file with editor_node.cpp, but recompiling changes to editor_node.cpp is a few seconds faster in my case, but it's not worsened, but it matters little in the end so we can remove this exception in the SCU builders later if wanted, but I think it might be useful in itself

@AThousandShips
Copy link
Member Author

@AThousandShips AThousandShips removed this from the 4.4 milestone Sep 3, 2024
@AThousandShips AThousandShips deleted the fix_editor_compile branch September 3, 2024 14:03
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.

2 participants