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

Support for events when the application terminates #170

Closed
sumibi-yakitori opened this issue Jan 26, 2020 · 9 comments · Fixed by #804
Closed

Support for events when the application terminates #170

sumibi-yakitori opened this issue Jan 26, 2020 · 9 comments · Fixed by #804
Assignees
Labels
feature New feature or request question Further information is requested
Milestone

Comments

@sumibi-yakitori
Copy link
Contributor

sumibi-yakitori commented Jan 26, 2020

When I close the window on MacOS,
Since the current version of winit::event_loop seems to be causing a panic without returning control flow,
Drop traits for variables allocated by the my application are not executed.

I want something like this,
Is it better to cast a window event and handle it immediately?

@hecrj
Copy link
Member

hecrj commented Jan 27, 2020

Since the current version of winit::event_loop seems to be causing a panic without returning control flow,

This sounds like a winit issue. The application shouldn't panic once you close the window.

I want something like this

That's one way to approach it. However, closing applications gracefully is sometimes more complicated than that. Users may need to run commands and wait until completion, for instance.

Because of this, I think it's a good idea to differentiate between a close request and the application deciding it should exit. We could add a new event CloseRequested, which could in turn be captured by a Subscription, and a method Application::should_exit or similar, which can be used by the runtime to know when to finish the event loop.

Sounds good?

@hecrj hecrj added this to the 0.1.0 milestone Jan 27, 2020
@hecrj hecrj added feature New feature or request question Further information is requested labels Jan 27, 2020
@sumibi-yakitori
Copy link
Contributor Author

sumibi-yakitori commented Jan 29, 2020

This sounds like a winit issue. The application shouldn't panic once you close the window.

Sorry for not being able to tell you exactly what happened.
That's right. I think this is a winit issue. No explicit panic message is displayed, but anyway, closing the window terminates the process immediately.
There are no bugs in iced for this issue.

Because of this, I think it's a good idea to differentiate between a close request and the application deciding it should exit. We could add a new event CloseRequested, which could in turn be captured by a Subscription, and a method Application::should_exit or similar, which can be used by the runtime to know when to finish the event loop.

I think it is very good.

Since the subscription argument is &self,
If you include &mut self as an argument of Application::should_exit, my wish will come true.

@hecrj hecrj self-assigned this Feb 19, 2020
@hecrj hecrj mentioned this issue Mar 26, 2020
@hecrj hecrj modified the milestones: 0.1.0, 0.2.0 Apr 2, 2020
@hecrj hecrj modified the milestones: 0.2.0, 0.3.0 Nov 26, 2020
@hecrj hecrj mentioned this issue Dec 1, 2020
@twitchyliquid64
Copy link

I hit this today, and am wondering if your suggestion is sufficiently non-controversial for me to implement. Specifically:

Because of this, I think it's a good idea to differentiate between a close request and the application deciding it should exit. We could add a new event CloseRequested, which could in turn be captured by a Subscription, and a method Application::should_exit or similar, which can be used by the runtime to know when to finish the event loop.

Sounds good?

Which I interpret as:

  1. Converting ControlFlow::Exit into an event widgets/subscriptions can match on, maybe we can call the invariant iced_native::event::Event::Window::ExitPressed

  2. Removing https://github.com/hecrj/iced/blob/d16b9cf7cd98a3d65ea5408ac9b72298cb267e85/winit/src/application.rs#L163-L165

  3. Adding a new method to Application: fn should_exit(&self) -> bool' which aborts the event loop if true is returned

@zenzoa
Copy link

zenzoa commented Dec 29, 2022

The CloseRequested event doesn't seem to let me interrupt the window closing for me, on PopOS (Linux). I'm able to act on other window events, so I know I've got the basic event subscription and update loop set up correctly. But no event is triggered at all for me when I click the window's close button in the title bar. (I'm using Iced and Rust for the first time, so apologies if I'm just missing something obvious. Unable to test on another OS at the moment.)

@13r0ck
Copy link
Member

13r0ck commented Dec 29, 2022

@zenzoa
Copy link

zenzoa commented Dec 29, 2022

@13r0ck Ah! Thank you!

@sowbug
Copy link

sowbug commented Feb 3, 2023

In case anyone is wondering why this isn't working anymore in 0.7.0, check out commit b5ab50b and do this to fix:

First, remove your should_exit() implementation in Application, which no longer compiled when you updated to 0.7.0. I'm assuming you had something like a self.should_exit boolean in your app from when you first implemented the override. Next, at the end of your Application's update() method, you most likely are returning Command::none(). Replace that with code like this:

use iced_native::command;

[...]

        if self.should_exit {
            Command::single(command::Action::Window(window::Action::Close))
        } else {
            Command::none()
        }

(#1606 says to return window::close(), which as far as I can tell is shorthand, rather than literal code, for command::Action::Window(window::Action::Close)

Hope this helps.

@hecrj
Copy link
Member

hecrj commented Feb 4, 2023

@sowbug That works. But it's probably a better idea to return window::close() wherever you set self.should_exit to true in your update logic. You may be able to get rid of the should_exit flag altogether that way.

@sowbug
Copy link

sowbug commented Feb 4, 2023

I see. I tried again and came up with this:

use iced::window;

return window::close::<Self::Message>();

As usual, there's something unique and mysterious about my code that prevented the compiler from inferring the Message type.

Apparently I had imported iced_native::window elsewhere, and that was why rust_analyzer said there was no such function as window::close(), which is what first sent me off in the wrong direction. I agree that I can remove myapp.should_exit now that I can take care of the closing right away. Thanks for the help.

sowbug added a commit to sowbug/groove that referenced this issue Feb 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants