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

Fix multiple instances of a game being allowed to start concurrently #6452

Merged
merged 3 commits into from
Dec 12, 2024

Conversation

peppy
Copy link
Member

@peppy peppy commented Dec 11, 2024

Regressed with #6442. Closes ppy/osu#31072.

Was not visible in test coverage because as per the inline comment, this is an exceptional case where two different processes bind.

I'm not sure how to add test coverage for this, but I have tested manually on both windows and macOS.

Can test using this:

diff --git a/SampleGame.Desktop/Program.cs b/SampleGame.Desktop/Program.cs
index 4756edbaa..ae5075535 100644
--- a/SampleGame.Desktop/Program.cs
+++ b/SampleGame.Desktop/Program.cs
@@ -12,9 +12,17 @@ public static class Program
         [STAThread]
         public static void Main(string[] args)
         {
-            using (GameHost host = Host.GetSuitableDesktopHost(@"sample-game"))
+            using (GameHost host = Host.GetSuitableDesktopHost(@"sample-game", new HostOptions
+                   {
+                       IPCPipeName = "wang"
+                   }))
             using (Game game = new SampleGameGame())
+            {
+                if (host.IsPrimaryInstance)
+                    return;
+
                 host.Run(game);
+            }
         }
     }
 }

Regressed with ppy#6442.

Was not visible in test coverage because as per the inline comment, this
is an exceptional case where two different processes bind.

I'm not sure how to add test coverage for this, but I have tested
manually on both windows and macOS.
@bdach
Copy link
Collaborator

bdach commented Dec 12, 2024

Not sure how it happened but your test diff is a little backwards (the IsPrimaryInstance check should be negated shouldn't it?)

Other than that tested on linux (including testing with a local game checkout) and things seem to be working fine, so 👍 Not checking windows (or mac, obviously 😅)

@peppy
Copy link
Member Author

peppy commented Dec 12, 2024

Not sure how it happened but your test diff is a little backwards (the IsPrimaryInstance check should be negated shouldn't it?)

Yeah I rewrote it locally for the sake of the PR description because copy-paste to my windows testing machine was not working. Safe to say it was the correct negation when I actually tested.

@bdach bdach merged commit 69a5a39 into ppy:master Dec 12, 2024
11 of 14 checks passed
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.

Can open multiple instances on Linux
2 participants