-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Bring osu.Android to .NET 6 #17462
Bring osu.Android to .NET 6 #17462
Conversation
The XA4301 warning is about picking linux assets for android from NativeLibs, since android is considered a child of linux. Should be resolved at framework side. |
The mono debugger issue can be workarounded by removing Xamarn.Essentials and reimplement it. There's no blocking issue I can find now. |
Co-authored-by: Susko3 <Susko3@protonmail.com>
Briefly tested this branch on my two devices, and got a pretty worrying exception stack trace on one of them:
As you could guess, the above causes everything online to not work. I've checked and this does not happen on the latest release. Device in question is a Samsung device, API 24. #17499 possibly related? |
Did you test if it reproduces on that device with old xamarin? If so, I think it could not block this PR as it's not a new issue. You may change NET6_0 to use HttpClientHandler here to see if there's difference. |
I haven't found a way to disable LLVM, and the issue may potentially be resolved (doesn't occur for me), so we might as well just remove the key for now.
global.json
Outdated
@@ -0,0 +1,6 @@ | |||
{ | |||
"sdk": { |
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.
Not sure we want this? @smoogipoo
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.
Theoretically, there are environments that can compile net6.0
but have unfixed issues in the SDK.
If we introduce net7.0
in any project, I think it can serve as a soft requirement for SDK version.
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.
@smoogipoo can you make a call on this one? from our brief discussion last week i think you'd want to see this removed?
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 think I would prefer this being removed, since it's one of those things that silently changes compilation in the background that isn't easily noticeable via any tooling. That being said, it is not a strong preference.
<AndroidSupportedAbis>armeabi-v7a;x86;arm64-v8a</AndroidSupportedAbis> | ||
<EnableLLVM>false</EnableLLVM> <!-- This currently causes random lockups during gameplay. https://github.com/mono/mono/issues/18973 --> | ||
|
||
<!-- This currently causes random lockups during gameplay. https://github.com/mono/mono/issues/18973 --> |
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 wonder if this is still relevant
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.
Testing with the same reproduction steps in the issue thread, I can't reproduce this at all with LLVM enabled (assuming it's enabled by default given that clang
is used during building).
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 think it's probably not, but we can test this in a separated PR.
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.
As long as it isn't forgotten.
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.
Other than what's mentioned above, this looks pretty sane to me and matches iOS (minus the workarounds).
Have removed for now. Have also tested this branch deploying to device and all seems good. I'll cherry-pick the changes across to the framework update branch in their current state. |
Development requirement
Run
dotnet workload install
android to install the necessary SDK components (may requiresudo
).Can be changed to
dotnet workload restore
once there's no old projects (Xamarin) remaining.The default Android SDK version changed from 29 to 31.
Xamarin toolchain in Visual Studio should be still required. I haven't tested if the managed components can be compiled without Xamarin installed.
Known issue
Realm currently only works in Release configuration. A hard crash will happen in Debug configuration.
Compatibility check list
Audio, network and video all work fine together with framework side change.
Moq compatibility not tested.
Fastlane deployment not tested.