-
Notifications
You must be signed in to change notification settings - Fork 27
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
Example for running on MacOs #30
Conversation
// if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) | ||
// { | ||
// Console.WriteLine("Running LocalMultiplayerAgent on Linux is not supported yet."); | ||
// return false; | ||
// } | ||
// | ||
// if (Globals.GameServerEnvironment == GameServerEnvironment.Linux && !_settings.RunContainer) | ||
// { | ||
// Console.WriteLine("The specified settings are invalid. Using Linux Game Servers requires running in a container."); | ||
// return false; | ||
// } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for contributing! It seems that we should have some conditional logic in place as well to see if we are running in macos to prevent the checks, rather than commenting it out. Similarly for the ProcessRunner start command. @dgkanatsios thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to push another tidier commit. I wasn't sure if you'd be interested in MacOS support!
I can also add an example config
hey @crazedpeanut, wow! thanks a lot for this, we'd love to have MacOS support!!! We'd be more than happy to work with you on this PR :) May I ask, did you try running container mode as well? Any luck? |
@dgkanatsios I didn't try running it in a container yet. Thinking about it now I probably should have tried running a Linux container first. I probably wouldn't have had to change anything! I'll have a look |
@crazedpeanut thank you! So my current thinking is that developers using MacOS would probably target Linux as the operating system of choice of PlayFab. In this context, I think that LocalMultiplayerAgent should support containers on Linux when running on MacOS. Let me know what you think, thanks again for working on this! |
Yes that sounds right! I’ll test out running in a Linux container
Sent from ProtonMail for iOS
…On Tue, Jul 13, 2021 at 8:39 am, Dimitris-Ilias Gkanatsios ***@***.***> wrote:
***@***.***(https://github.com/crazedpeanut) thank you! So my current thinking is that developers using MacOS would probably target Linux as the operating system of choice of PlayFab. In this context, I think that LocalMultiplayerAgent should support containers on Linux when running on MacOS. Let me know what you think, thanks again for working on this!
—
You are receiving this because you were mentioned.
Reply to this email directly, [view it on GitHub](#30 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AB2YDCVI45H3Z7MS72JA2OLTXNVLJANCNFSM5AB3RGRA).
|
@dgkanatsios it turns out the project I am working on can't target Linux because of some dependency issues. So I am stuck with the MacOS builds. If this is not the intended workflow for the local agent feel free to close this issue, i'll just keep my own fork :) |
@crazedpeanut so the way it works right now is launching MacOS processes? |
@dgkanatsios correct! |
Great! So we could work and eventually merge this, by having a check in the beginning like “if OS==MacOS and runContainers==true then exit”. What do you think? We’d be more than happy to accept it! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johnkendall-MK and @crazedpeanut , thanks for your contribution!
It's good to understand the problem areas for expanding support onto a Mac, and you've provided that, thanks. In the case of this PR, we can't accept it as is, because we can't remove support for Windows/Linux in order to add Mac support. If you would update this PR to fix Mac without altering Windows/Linux, we would appreciate it. In the meantime, I don't want to leave this PR open forever. I think we'll close it for now, and transform it into an issue linking the closed PR.
@@ -92,7 +92,11 @@ public Task CollectLogs(string id, string logsFolder, ISessionHostManager sessio | |||
|
|||
private (string, string) GetExecutableAndArguments(SessionHostsStartInfo sessionHostsStartInfo, int instanceNumber) | |||
{ | |||
string[] parts = sessionHostsStartInfo.StartGameCommand.Split(' ', StringSplitOptions.RemoveEmptyEntries); | |||
var command = sessionHostsStartInfo.StartGameCommand.Replace("\\ ", "\\SPACE"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C# generally accepts / in Windows paths, and Linux-based systems naturally want / as well. I'd say a conversion of "\" -> "/" would be more universal and safe. Additionally, when I think of what would typically replace a space in a path, I think of "_" or sometimes "-". I don't really think of "SPACE".
Hi! I understand MacOS isn't officially supported but I thought i'd share the changes I needed to make in order to run the agent on my MacBook.
I had to make the following changes to the configuration file:
For example:
"ProcessStartParameters": {
"StartGameCommand": "/Users/example/Example.app/Contents/MacOS/Example -nographics -batchmode"
},
Our Unity Game's product name includes spaces, so I needed to escape these in the ProcessRunner. Otherwise, the StartGameCommand would be interpreted incorrectly.
It works great, thanks for providing the local agent!