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

Lock screen rotation while in gameplay on android #10516

Merged
merged 5 commits into from
Oct 19, 2020

Conversation

Game4all
Copy link
Contributor

Summary

This adds a GameplayScreenRotationLocker component which locks screen rotation on android devices while in gameplay.

Unfortunately, android (same for Xamarin) doesn't expose a nice, non-hackish way to get the current activity from anywhere (which is required to lock the screen rotation) so I had to resort to storing the game activity inside a static field upon startup.

Testing

No automated testing for this altough from the testing I've done on 3 devices I own, this seems to work fine.

@bdach
Copy link
Collaborator

bdach commented Oct 15, 2020

I'm not sure we should be doing this. We already changed the activity to use FullUser so that OS-level screen rotation lock can be preserved in-game. And even if we were to do this forcing landscape would probably be saner.

@Joehuu
Copy link
Member

Joehuu commented Oct 15, 2020

For reference, this will fix #5331 for android.

private void load(OsuGame game)
{
localUserPlaying = game.LocalUserPlaying.GetBoundCopy();
localUserPlaying.BindValueChanged(userPlaying => updateLock(userPlaying));
Copy link
Contributor

Choose a reason for hiding this comment

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

Change to localUserPlaying.ValueChanged += updateLock;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Method group would also work. I'd slightly prefer that to an event subscription actually but unsure if it matters.

Copy link
Contributor

Choose a reason for hiding this comment

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

BindValueChanged exists mostly to allow passing true as the second argument to run it immediately.
The first line is literally ValueChanged += onChange;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I went with BindValueChanged as I thought we might want to run callback immediately in this scenario but I'm not sure it is actually necessary anymore.

@swoolcock
Copy link
Contributor

Would be nice to implement this for iOS too, even if it's a separate PR.

@Game4all
Copy link
Contributor Author

As an android user, I'll have to delegate implementation of this for iDevices as I don't own any of these (and lack Xamarin.iOS knowledge as well)

Copy link
Contributor

@smoogipoo smoogipoo left a comment

Choose a reason for hiding this comment

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

Looks fine to me. @peppy could you test on your devices?

@peppy
Copy link
Member

peppy commented Oct 19, 2020

Works as expected

@peppy peppy merged commit 0e9b28d into ppy:master Oct 19, 2020
@Game4all Game4all deleted the android-gameplay-locked-screen-orientation branch October 23, 2020 08:01
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.

6 participants