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

[3.x] Add editor keyboard shortcut for Mono Build solution button #52595

Merged
merged 1 commit into from
Sep 14, 2021
Merged

[3.x] Add editor keyboard shortcut for Mono Build solution button #52595

merged 1 commit into from
Sep 14, 2021

Conversation

lewiji
Copy link
Contributor

@lewiji lewiji commented Sep 12, 2021

Related issue: #39633

This adds a keyboard shortcut for the Build button in Godot Mono 3.x. Default is Alt+B (since other suggestions from the issue were taken by the script editor bookmark shortcuts).

image

image

This exposes the ED_GET_SHORTCUT function from modules\mono\editor\editor_internal_calls.cpp to the GodotTools C# solution, via a new GodotTools.Internals.Globals function: GodotTools.Internals.Globals::internal_EditorShortcut - allowing the GodotSharpEditor.cs EditorPlugin to retrieve editor shortcuts, which it does to attach a new script_editor/build_mono_solution mono/build_solution shortcut definition to the mono Build button.

I couldn't get these changes to compile against master; firstly I got an error because ShortCut is now Shortcut, but then I got errors related to the mono/editor/code_completion.cpp file (unchanged):

modules\mono\editor\code_completion.cpp(126): error C3536: '<begin>$L1': cannot be used before it is initialized
modules\mono\editor\code_completion.cpp(126): error C3536: '<end>$L1': cannot be used before it is initialized
[Initial build] modules\mono\editor\code_completion.cpp(126): error C2100: illegal indirection
[Initial build] modules\mono\editor\code_completion.cpp(126): error C2440: 'initializing': cannot convert from 'int' to 'const KeyValue<StringName,ProjectSettings::AutoloadInfo> &'
[Initial build] modules\mono\editor\code_completion.cpp(126): note: Reason: cannot convert from 'int' to 'const KeyValue<StringName,ProjectSettings::AutoloadInfo>'
modules\mono\editor\code_completion.cpp(126): note: No constructor could take the source type, or constructor overload resolution was ambiguous

I'm unsure what the status of Mono is in Godot 4? So, I made these changes against 3.x since that's what I'm using, let me know if this needs to be moved to master, and if I need to change anything to do so.

Additionally, I've added the keyboard shortcut under the script_editor shortcut setting group, since that seemed most relevant, however this shortcut will be available even in non-Mono builds; is there a way to conditionally add the shortcut only in a Mono build? Shortcut is now under mono/build_solution settings key.

Cheers.

Bugsquad edit: This closes godotengine/godot-proposals#3197.

@lewiji lewiji requested review from a team as code owners September 12, 2021 10:02
@raulsntos
Copy link
Member

Additionally, I've added the keyboard shortcut under the script_editor shortcut setting group, since that seemed most relevant, however this shortcut will be available even in non-Mono builds; is there a way to conditionally add the shortcut only in a Mono build?

I think the ED_SHORTCUT should be inside the mono module but I'm not sure where exactly.

@lewiji
Copy link
Contributor Author

lewiji commented Sep 12, 2021

For the implementation of passing ED_SHORTCUT I followed the style of the other methods for getting editor settings, however I only wrote a getter. I assume the function can also set editor shortcuts, allowing the Mono project to add it from C#, though I haven't tried, so I'll look into that; that would indeed solve it.

@raulsntos
Copy link
Member

I didn't mean to set the editor shortcut from C#, just to have the ED_SHORTCUT macro inside the modules/mono directory since the shortcut only applies to the mono module.

@lewiji
Copy link
Contributor Author

lewiji commented Sep 12, 2021

I see, for clarity's sake, you mean to move the call to register the shortcut i.e.:

	ED_SHORTCUT("script_editor/build_mono_solution", TTR("Build Mono Solution"), KEY_MASK_ALT | KEY_B);

Into one of the cpp files in modules/mono? And probably it'd be good to change the script_editor category to something more appropriate like mono as well.

@raulsntos
Copy link
Member

I see, for clarity's sake, you mean to move the call to register the shortcut into one of the cpp files in modules/mono?

Yes, but I'm not sure where exactly.

And probably it'd be good to change the script_editor category to something more appropriate like mono as well.

I guess that would make sense.

@lewiji
Copy link
Contributor Author

lewiji commented Sep 12, 2021

After a bit of trial and error, I think this is about the right place for it:

https://github.com/godotengine/godot/blob/6841c89e1374c87e857a4da6820194f2ce803ffc/modules/mono/csharp_script.cpp#L1183

At this point the checks for Mono being enabled are done, and the cs editor plugin is about to be enabled. Any later than this in the process, and the plugin will try and lookup the shortcut before it's been registered. I have tested on a non-mono build, and the shortcut doesn't get registered as expected.

I fixed the shortcut hint label:

image

And the shortcut now sits in a new "mono" category of shortcuts:

image

I haven't got round to testing it with Godot 4 yet, but I will pull down your fix, @raulsntos, and see what happens, when I get a chance later. Thanks.

@lewiji
Copy link
Contributor Author

lewiji commented Sep 12, 2021

Btw, with my initial PR a review was auto requested from the @godotengine/script-editor group, as the original commit registered the editor shortcut in the script_editor_plugin.cpp file. I've since amended the PR so that the shortcut is added in the mono module - so I think that review request can be removed/dismissed.

@Calinou Calinou removed the request for review from a team September 12, 2021 22:15
@lewiji lewiji changed the title Add editor keyboard shortcut for Mono Build solution button [3.x] Add editor keyboard shortcut for Mono Build solution button Sep 13, 2021
Update GodotSharpEditor.cs & csharp_script.cpp with better casing and localisation for HintTooltip on Build button

Co-authored-by: Rémi Verschelde <[email protected]>
@akien-mga akien-mga merged commit 154cca3 into godotengine:3.x Sep 14, 2021
@akien-mga
Copy link
Member

Thanks!

@lewiji lewiji deleted the mono-build-solution-shortcut branch September 15, 2021 12:22
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.

5 participants