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

Enable mono editor build in CI #47600

Merged
merged 1 commit into from
Nov 9, 2021

Conversation

qarmin
Copy link
Contributor

@qarmin qarmin commented Apr 3, 2021

Depends on #47414 which removes partial editor mono compilation instructions

It would allow to get from CI fully working Godot mono build exactly as in 3.2 branch

@qarmin qarmin requested a review from a team April 3, 2021 17:05
@qarmin qarmin added this to the 4.0 milestone Apr 3, 2021
@qarmin qarmin force-pushed the enable_mono_editor branch 2 times, most recently from fb497ad to 8bd537b Compare April 3, 2021 17:49
@qarmin
Copy link
Contributor Author

qarmin commented Apr 8, 2021

I tested artifacts and seems that editor works fine and can build project

@qarmin qarmin force-pushed the enable_mono_editor branch 2 times, most recently from 56389ef to 829fa76 Compare August 12, 2021 14:12
@qarmin qarmin marked this pull request as draft August 15, 2021 20:29
@qarmin qarmin force-pushed the enable_mono_editor branch 4 times, most recently from ad34933 to e1df1ed Compare October 2, 2021 05:43
@qarmin qarmin force-pushed the enable_mono_editor branch 2 times, most recently from 148310e to 6826f7e Compare October 16, 2021 10:21
@akien-mga
Copy link
Member

akien-mga commented Oct 16, 2021

I actually broke the Mono build yesterday, #53882 should fix it (at least the current error).

Another proof that we need this finalized and merged :P

@qarmin
Copy link
Contributor Author

qarmin commented Oct 17, 2021

I copied compilation instructions from 3.x branch and looks that now after producing binary, editor freeze when executing unit tests

@qarmin qarmin force-pushed the enable_mono_editor branch 2 times, most recently from 9d58368 to e063f49 Compare October 20, 2021 05:14
@qarmin
Copy link
Contributor Author

qarmin commented Oct 20, 2021

Looks that mono build is again broken

 modules/mono/mono_gd/gd_mono_utils.cpp:453:111: error: no matching function for call to 'ScriptDebugger::send_error(String&, String&, int&, String&, String&, ErrorHandlerType, Vector<ScriptLanguage::StackInfo>&)'
  453 |  EngineDebugger::get_script_debugger()->send_error(func, file, line, error_msg, exc_msg, ERR_HANDLER_ERROR, si);
      |                                                                                                               ^
In file included from modules/mono/mono_gd/gd_mono_utils.cpp:37:
./core/debugger/script_debugger.h:74:7: note: candidate: 'void ScriptDebugger::send_error(const String&, const String&, int, const String&, const String&, bool, ErrorHandlerType, const Vector<ScriptLanguage::StackInfo>&)'
   74 |  void send_error(const String &p_func, const String &p_file, int p_line, const String &p_err, const String &p_descr, bool p_editor_notify, ErrorHandlerType p_type, const Vector<StackInfo> &p_stack_info);
      |       ^~~~~~~~~~
./core/debugger/script_debugger.h:74:7: note:   candidate expects 8 arguments, 7 provided

@akien-mga
Copy link
Member

Fixed by #54016.

@qarmin
Copy link
Contributor Author

qarmin commented Oct 20, 2021

Looks that there is another problem, because entire job freeze for 6 hours and fails - https://github.com/godotengine/godot/runs/3914980436?check_suite_focus=true

 WARNING: Changing the VSync mode is not supported by this display server.
     at: window_get_vsync_mode (servers/display_server.cpp:320)
ERROR: Sub-windows not supported by this display server.
   at: create_sub_window (servers/display_server.cpp:188)
ERROR: Condition "window_id == DisplayServer::INVALID_WINDOW_ID" is true.
   at: _make_window (scene/main/window.cpp:229)

I tried to add --headless to tests but still I see that this job freeze

@qarmin qarmin force-pushed the enable_mono_editor branch 3 times, most recently from d1411bf to c33577d Compare November 9, 2021 07:36
@qarmin qarmin marked this pull request as ready for review November 9, 2021 08:59
@qarmin qarmin requested review from a team as code owners November 9, 2021 08:59
Comment on lines -1318 to -1319
<member name="mono/project/auto_update_project" type="bool" setter="" getter="" default="true">
</member>
Copy link
Member

Choose a reason for hiding this comment

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

You should update main.cpp too:

                // From editor/csharp_project.cpp.
                GLOBAL_DEF("mono/project/auto_update_project", true);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What should be done here?
I removed this line because i checked that auto_update_project is not used in other files

Copy link
Member

Choose a reason for hiding this comment

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

You should remove the dummy define from main.cpp, it's no longer used.

@qarmin qarmin requested a review from a team as a code owner November 9, 2021 10:02
@akien-mga akien-mga merged commit bc6ec58 into godotengine:master Nov 9, 2021
@akien-mga
Copy link
Member

Thanks!

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