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

C#: Move build button to EditorRunBar #79357

Merged
merged 1 commit into from
Aug 3, 2023

Conversation

raulsntos
Copy link
Member

  • Move C# build button to EditorRunBar.
  • Add C# build icon.
  • Add shortcut macros to GodotTools.
  • Move C# build shortcuts to C#.

The motivation is to make the build button look more integrated with the rest of the Godot editor. Before it looked very out of place.

4.1.stable This PR
BuildButtonBefore BuildButtonAfter

editor/gui/editor_run_bar.h Outdated Show resolved Hide resolved
editor/gui/editor_run_bar.h Outdated Show resolved Hide resolved
Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

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

Aside from the discussed API changes, this makes sense to me. The editor part and the icon look good. The C# part looks fine too as far as I can tell.

@YuriSizov YuriSizov modified the milestones: 4.x, 4.2 Aug 3, 2023
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks great to me. Makes a lot of sense.

We could consider naming the icon just Build since it's just a hammer, as it might be useful for other use cases in the future (e.g. a plugin to compile C++ GDExtension code from the editor). But that's not very important and could be changed in the future (unless we consider that EditorIcons names are part of the API and that renaming them breaks compat).

- Move C# build button to `EditorRunBar`.
- Add C# build icon.
- Add shortcut macros to `GodotTools`.
- Move C# build shortcuts to C#.
@raulsntos
Copy link
Member Author

We could consider naming the icon just Build since it's just a hammer, as it might be useful for other use cases in the future.

Keep in mind the icon is inside the .NET module so it's only included in the .NET-version of Godot. If we want the icon to be used for other cases we'll have to move it to editor.

@akien-mga
Copy link
Member

We could consider naming the icon just Build since it's just a hammer, as it might be useful for other use cases in the future.

Keep in mind the icon is inside the .NET module so it's only included in the .NET-version of Godot. If we want the icon to be used for other cases we'll have to move it to editor.

Right, let's not future proof then and just wait for the need to arise.

@akien-mga akien-mga merged commit bf185e4 into godotengine:master Aug 3, 2023
14 checks passed
@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.

3 participants