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.

Sign up for GitHub

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

Jump to bottom

Expose get_editor_toaster method to EditorInterface #98680

Merged
merged 1 commit into from
Nov 12, 2024
Merged

Expose get_editor_toaster method to EditorInterface #98680

merged 1 commit into from
Nov 12, 2024

Conversation

jaydensipe
Copy link
Contributor

@jaydensipe jaydensipe commented Oct 30, 2024 •
edited
Loading

Closes godotengine/godot-proposals#11051

Exposes get_editor_toaster to EditorInterface (w/ push_toast method); for use in the editor.

Preview (editedx3)

image

image

@jaydensipe jaydensipe requested review from a team as code owners October 30, 2024 19:43
@jaydensipe
Copy link
Contributor Author

This may close #77890 as well, as discussed in godotengine/godot-proposals#11051 that a separate API would be preferred.

@jaydensipe jaydensipe force-pushed the expose-toast-notification-methods branch 3 times, most recently from b77f746 to 47443ec Compare October 30, 2024 20:28
@KoBeWi KoBeWi added this to the 4.x milestone Oct 30, 2024
@KoBeWi
Copy link
Member

KoBeWi commented Oct 30, 2024

This should be in EditorInterface, not global scope. Also, instead of adding multiple methods, it's better to have one that takes enum argument for message type.

@jaydensipe jaydensipe force-pushed the expose-toast-notification-methods branch from 47443ec to fde85ae Compare October 31, 2024 01:20
@jaydensipe jaydensipe requested review from a team as code owners October 31, 2024 01:20
@jaydensipe jaydensipe force-pushed the expose-toast-notification-methods branch from fde85ae to 6b7ca2c Compare October 31, 2024 01:22
@jaydensipe jaydensipe changed the title Expose toast_error, toast_info, toast_warning methods to @GlobalScope Expose toast method to `EditorInterface Oct 31, 2024
@jaydensipe jaydensipe changed the title Expose toast method to `EditorInterface Expose toast method to EditorInterface Oct 31, 2024
@jaydensipe jaydensipe force-pushed the expose-toast-notification-methods branch from 6b7ca2c to b6db845 Compare October 31, 2024 01:30
@jaydensipe
Copy link
Contributor Author

This should be in EditorInterface, not global scope. Also, instead of adding multiple methods, it's better to have one that takes enum argument for message type.

Kinda figured once I saw all the tests blow up lolol, ty for the suggestions, updated! Just for discussion would we rather have the method name be toast or push_toast?

@jaydensipe jaydensipe force-pushed the expose-toast-notification-methods branch 3 times, most recently from 68d2d20 to b8b24c3 Compare October 31, 2024 02:35
@Mickeon
Copy link
Contributor

Mickeon commented Oct 31, 2024

push_toast

@ydeltastar
Copy link
Contributor

Missing tooltip parameter.

@jaydensipe jaydensipe force-pushed the expose-toast-notification-methods branch 2 times, most recently from 007ee15 to 3a23add Compare October 31, 2024 17:27
@jaydensipe jaydensipe changed the title Expose toast method to EditorInterface Expose push_toast method to EditorInterface Oct 31, 2024
@jaydensipe
Copy link
Contributor Author

@Mickeon @ydeltastar Fixed!

@jaydensipe jaydensipe force-pushed the expose-toast-notification-methods branch 3 times, most recently from 6bb9dc7 to b48fa93 Compare November 2, 2024 02:26
@jaydensipe jaydensipe changed the title Expose push_toast method to EditorInterface Expose get_editor_toaster method to EditorInterface Nov 2, 2024
@jaydensipe
Copy link
Contributor Author

@KoBeWi Let me know what ya think.

@jaydensipe jaydensipe requested a review from KoBeWi November 2, 2024 02:42
@jaydensipe jaydensipe mentioned this pull request Nov 2, 2024
Closed
@jaydensipe jaydensipe force-pushed the expose-toast-notification-methods branch from b48fa93 to 4f56883 Compare November 2, 2024 04:03
KoBeWi
KoBeWi approved these changes Nov 2, 2024
View reviewed changes
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.

I think it's fine like this.

@KoBeWi KoBeWi requested a review from groud November 2, 2024 10:39
AThousandShips
AThousandShips reviewed Nov 2, 2024
View reviewed changes
@jaydensipe jaydensipe force-pushed the expose-toast-notification-methods branch from 4f56883 to 4d2b701 Compare November 2, 2024 22:22
@jaydensipe jaydensipe force-pushed the expose-toast-notification-methods branch from 4d2b701 to 8bf800b Compare November 11, 2024 00:32
@jaydensipe
Copy link
Contributor Author

Rebased with master.

@jaydensipe jaydensipe force-pushed the expose-toast-notification-methods branch from 8bf800b to 722d932 Compare November 12, 2024 00:39
@KoBeWi KoBeWi modified the milestones: 4.x, 4.4 Nov 12, 2024
akien-mga
akien-mga reviewed Nov 12, 2024
View reviewed changes
doc/classes/EditorToaster.xml
<param index="1" name="severity" type="int" enum="EditorToaster.Severity" default="0" />
<param index="2" name="tooltip" type="String" default="&quot;&quot;" />
<description>
Pushes a toast notification to the editor for display.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Pushes a toast notification to the editor for display.
Pushes a toast notification to the editor for display.
Returns [constant ERR_TOASTER_ON_FIRE] if [param severity] is [constant SEVERITY_CRITICAL].

Copy link
Contributor

Choose a reason for hiding this comment

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

Wait hold on is this is actually real are you serious

Copy link
Member

Choose a reason for hiding this comment

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

Wait hold on is this is actually real are you serious

I checked, sadly it's not real. xD

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opened #444444 to fix this ;)

@Repiteo Repiteo merged commit 646cef2 into godotengine:master Nov 12, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 12, 2024

Thanks!

@jaydensipe jaydensipe deleted the expose-toast-notification-methods branch November 13, 2024 01:35
@KoBeWi KoBeWi mentioned this pull request Feb 17, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KoBeWi KoBeWi KoBeWi approved these changes

@groud groud groud approved these changes

@Mickeon Mickeon Mickeon left review comments

@akien-mga akien-mga akien-mga approved these changes

@AThousandShips AThousandShips AThousandShips approved these changes

Assignees
No one assigned
Labels
enhancement feature proposal topic:editor
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.

Expose EditorToaster interface
8 participants
@jaydensipe @KoBeWi @Mickeon @ydeltastar @Repiteo @akien-mga @groud @AThousandShips