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

Allow adding custom export platforms using scripts / GDExtension. #90782

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Apr 17, 2024

Adds option to make custom export platforms using scripts or GDExtension without editor rebuild.

TODO:

@bruvzg bruvzg added this to the 4.x milestone Apr 17, 2024
@bruvzg bruvzg force-pushed the export_platform_extension branch 3 times, most recently from 03efc5e to b454652 Compare April 17, 2024 12:50
@dsnopek
Copy link
Contributor

dsnopek commented Apr 17, 2024

Thanks for taking this on! I started a local branch with the same goal back in January, but just haven't had the time to finish it up. Your version is already further along than mine :-)

@bruvzg bruvzg force-pushed the export_platform_extension branch 3 times, most recently from d39b196 to 252a630 Compare April 18, 2024 11:54
@bruvzg

This comment was marked as outdated.

@bruvzg bruvzg force-pushed the export_platform_extension branch 3 times, most recently from 895e9b7 to 754dc47 Compare April 22, 2024 06:35
@bruvzg bruvzg changed the title [WIP] Allow adding custom export platforms using scripts / GDExtension. Allow adding custom export platforms using scripts / GDExtension. Apr 22, 2024
@bruvzg bruvzg marked this pull request as ready for review April 22, 2024 08:03
@bruvzg bruvzg requested review from a team as code owners April 22, 2024 08:03
editor/export/editor_export_platform_extension.cpp Outdated Show resolved Hide resolved
editor/export/editor_export_platform_extension.cpp Outdated Show resolved Hide resolved
doc/classes/EditorExportPlatform.xml Outdated Show resolved Hide resolved
doc/classes/EditorExportPlatform.xml Outdated Show resolved Hide resolved
doc/classes/EditorExportPlatformExtension.xml Outdated Show resolved Hide resolved
editor/plugins/script_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/plugins/script_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/export/editor_export_platform_pc.h Outdated Show resolved Hide resolved
editor/plugins/editor_plugin.cpp Outdated Show resolved Hide resolved
editor/plugins/editor_plugin.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

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

Have yet to test it but the logic looks good.

@raulsntos
Copy link
Member

I tested this with the C# ExportPlugin to implement the C# changes from my superseded PRs (raulsntos@c069790) and works great.

I don't understand the changes to expose ScriptEditor::get_breakpoints, it seems unrelated. Is this needed for scripts implementing export platforms?

Also, since EditorExportPlugin has no way to make the export process fail, my superseded PR (#90555) was relying on checking if there were any error messages added to determine if the export failed. Something like this:

diff --git a/editor/export/editor_export_platform.cpp b/editor/export/editor_export_platform.cpp
index eb9e9b9adf..53d6d689a1 100644
--- a/editor/export/editor_export_platform.cpp
+++ b/editor/export/editor_export_platform.cpp
@@ -71,7 +71,7 @@ bool EditorExportPlatform::fill_log_messages(RichTextLabel *p_log, Error p_err)
 	p_log->add_text(" ");
 	p_log->add_text(get_name());
 	p_log->add_text(" - ");
-	if (p_err == OK) {
+	if (p_err == OK && get_worst_message_type() < EditorExportPlatform::EXPORT_MESSAGE_ERROR) {
 		if (get_worst_message_type() >= EditorExportPlatform::EXPORT_MESSAGE_WARNING) {
 			p_log->add_image(p_log->get_editor_theme_icon(SNAME("StatusWarning")), 16 * EDSCALE, 16 * EDSCALE, Color(1.0, 1.0, 1.0), INLINE_ALIGNMENT_CENTER);
 			p_log->add_text(" ");

Otherwise, an ExportPlugin can add error messages but the export log dialog will show Completed with warnings:

image

@bruvzg bruvzg force-pushed the export_platform_extension branch from 22375d4 to aadc8e9 Compare May 13, 2024 16:46
@bruvzg
Copy link
Member Author

bruvzg commented May 13, 2024

Also, since EditorExportPlugin has no way to make the export process fail, my superseded PR (#90555) was relying on checking if there were any error messages added to determine if the export failed. Something like this

Added it.

I don't understand the changes to expose ScriptEditor::get_breakpoints, it seems unrelated. Is this needed for scripts implementing export platforms?

It might be necessary if you want to implement custom one click deploy for the platform with remote debugging support. Usually the list of breakpoints is passed to the remote executable with --breakpoints argument, and handled by gen_export_flags, but for some platforms you can't just pass command line arguments.

Copy link
Contributor

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

Thanks for your epic work on this! It's looking really great :-)

I haven't tested it, but all the important code looks good to me - I just have two little notes.

editor/export/editor_export_platform.cpp Show resolved Hide resolved
editor/export/editor_export_platform_extension.cpp Outdated Show resolved Hide resolved
@bruvzg bruvzg force-pushed the export_platform_extension branch from aadc8e9 to fe06d18 Compare May 13, 2024 17:09
@bruvzg bruvzg force-pushed the export_platform_extension branch from fe06d18 to 4a80b01 Compare May 13, 2024 17:19
@akien-mga akien-mga modified the milestones: 4.x, 4.4 May 13, 2024
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.

None yet

6 participants