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

Launcher: keep alive #1894

Merged
merged 3 commits into from
Jun 27, 2023
Merged

Launcher: keep alive #1894

merged 3 commits into from
Jun 27, 2023

Conversation

Berserker66
Copy link
Member

What is this fixing or adding?

Alternative to #1892, that makes each window closable independently.

How was this tested?

Only with TextClient on Windows

If this makes graphical changes, please attach screenshots.

@ThePhar ThePhar added the is: enhancement Issues requesting new features or pull requests implementing new features. label Jun 26, 2023
Copy link
Member

@black-sliver black-sliver left a comment

Choose a reason for hiding this comment

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

Seems to work for me.

The only gripe I have with our multiprocessing solution now is that it won't join/cleanup the sub-processes until you either spawn another or exit the main application. I'm not sure how this is supposed to be handled with multiprocessing.

e.g.

  1. open TextClient -> process for TextClient
  2. close TextClient -> shows process as <defunct> (because it's not joined)
  3. open TextClient -> joins old process, runs new process
  4. close TextClient -> shows process as <defunct>
  5. close Launcher -> joins the defunct process

@Berserker66
Copy link
Member Author

Interesting. That mid-step is not something I had occur, or just not seen it. In my testing I opened Text Client once, closed it, then launcher and then the whole thing exited.

@Berserker66 Berserker66 merged commit a4e485e into main Jun 27, 2023
19 checks passed
@Berserker66 Berserker66 deleted the launcher_keep_alive branch June 27, 2023 07:31
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: enhancement Issues requesting new features or pull requests implementing new features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants