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

Initial Linux support #32

Merged
merged 10 commits into from
Feb 7, 2021
Merged

Conversation

technobaboo
Copy link

All basic functions of Open Brush work in Linux now, so this PR is just to add support. Some credit goes to Babouchot for their contributions (see #4)

Menu.SetChecked(kMenuPlatformOsx, false);
Menu.SetChecked(kMenuPlatformAndroid, false);

switch (value) {
case BuildTarget.StandaloneWindows64:
Menu.SetChecked(kMenuPlatformWindows, true);
break;
case BuildTarget.StandaloneLinux64:
Copy link
Member

Choose a reason for hiding this comment

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

Are these tabs instead of spaces?

System.Diagnostics.Process.Start(url);
} else {
Application.OpenURL(url);
// Something about the url makes OpenURL() not work on OSX or Linux, so use a workaround
Copy link
Member

Choose a reason for hiding this comment

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

This comment doesn't match the update; did you mean to add the Linux RuntimePlatforms to the cases?

@@ -99,6 +99,10 @@ private class ParentNotFound : Exception {
case RuntimePlatform.OSXEditor:
Debug.LogError("Host id not implemented for macOS");
return "macOS-unknown";
case RuntimePlatform.LinuxPlayer:
Copy link
Member

Choose a reason for hiding this comment

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

tabs again?

@technobaboo
Copy link
Author

technobaboo commented Feb 7, 2021 via email

@mikeage
Copy link
Member

mikeage commented Feb 7, 2021

Thanks. One other thing, but possibly dependent on #24 -- the target in the python script is StandaloneLinuxUniversal, not StandaloneLinux64. That'll probably need to be updated too, but maybe best to do it there (or maybe it depends on which PR gets merged first)

@mikeskydev mikeskydev self-requested a review February 7, 2021 14:15
@mikeskydev mikeskydev added the enhancement Feature added label Feb 7, 2021
@mikeskydev
Copy link
Member

I was able to build and run the output, just noting that I had to follow this guide https://github.com/ValveSoftware/SteamVR-for-Linux#runtime-requirements to be able to run it. Is it worth including a Linux section in README to highlight this? I'm not sure if it was just specific to my setup (Ubuntu 20.04 LTS). Will now test from Windows side to make sure nothing breaks.

Copy link
Member

@mikeskydev mikeskydev left a comment

Choose a reason for hiding this comment

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

LGTM! No issues Windows side.

@mikeskydev mikeskydev merged commit 0673dd4 into icosa-foundation:main Feb 7, 2021
@mikeskydev
Copy link
Member

Thanks!

@mikeage
Copy link
Member

mikeage commented Apr 2, 2021

@technobaboo , I just wanted to let you and @Babouchot know that we now have Linux builds included as part of the automated pre-release builds!

#76

@technobaboo
Copy link
Author

technobaboo commented Apr 2, 2021 via email

@Babouchot
Copy link

@technobaboo , I just wanted to let you and @Babouchot know that we now have Linux builds included as part of the automated pre-release builds!

#76

Nice one !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature added
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants