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

Update to Android SDK 25 #5151

Merged
merged 1 commit into from Mar 31, 2017
Merged

Conversation

mahdihijazi
Copy link
Contributor

As a first step we are going to keep the write permission because we need to copy the files from the old location for those users who already have the emulator installed on Android to the new location that will not require the permission any more.

Later, once we feel most of users migrated their files to the new location, we can go on and remove the write permission.

if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
int hasWriteContactsPermission = ContextCompat.checkSelfPermission(activity, WRITE_EXTERNAL_STORAGE);
return hasWriteContactsPermission == PackageManager.PERMISSION_GRANTED;
} else {

This comment was marked as off-topic.

SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(parent);
boolean assetsCopied = preferences.getBoolean("assetsCopied", false);

if(!assetsCopied) {

This comment was marked as off-topic.

for (int platformIndex = 0; platformIndex <= Game.PLATFORM_ALL; ++platformIndex)
{
mPresenter.loadGames(platformIndex);
if(PermissionsHandler.hasWriteAccess(this)) {

This comment was marked as off-topic.

}
break;
default:
super.onRequestPermissionsResult(requestCode, permissions, grantResults);

This comment was marked as off-topic.

}
break;
default:
super.onRequestPermissionsResult(requestCode, permissions, grantResults);

This comment was marked as off-topic.

@@ -66,6 +66,13 @@ public void onClick(View view)
// TODO Split some of this stuff into Application.onCreate()
if (savedInstanceState == null)
StartupHandler.HandleInit(this);

if(PermissionsHandler.hasWriteAccess(this)) {

This comment was marked as off-topic.

Copy link
Member

@JosJuice JosJuice left a comment

Choose a reason for hiding this comment

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

Just to make sure I'm understanding the code correctly: This PR isn't changing what directory we write to, right? It just adds support for the new way of requesting the write permission?

return true;
}

int hasWriteContactsPermission = ContextCompat.checkSelfPermission(activity, WRITE_EXTERNAL_STORAGE);

This comment was marked as off-topic.

This comment was marked as off-topic.

@mahdihijazi
Copy link
Contributor Author

yes, you are right. I noticed that I need the handle the write permissions anyway since the first step would be to copy the files from the old location to the new location in the sdcard for those who have dolphin installed on their devices to avoid duplicating them. And copy from assets for fresh installs.

After a while we can drop this, remove the permission and all its related code.

@mahdihijazi
Copy link
Contributor Author

if everything looks good to you, I can squash the commits?

@mbc07
Copy link
Contributor

mbc07 commented Mar 24, 2017

While we're on this subject, is there any reason why Dolphin doesn't store its files in the app-exclusive shared storage folder (/storage/emulated/0/Android/data/org.dolphinemu.dolphinemu)? AFAIK apps don't require write permission to store data on that specific location and it's still user acessible...

@mahdihijazi
Copy link
Contributor Author

@mbc07 that's the next step after this PR get merged.

This handles the new permission system in Android M.
@mahdihijazi
Copy link
Contributor Author

@lioncash @JosJuice please let me know if I need to change anything else. Comments changes squashed into the original commit.

@lioncash
Copy link
Member

@mahdihijazi Sorry for taking this long to get back to you. As far as I can tell this looks fine.

@lioncash lioncash merged commit f5dcfb1 into dolphin-emu:master Mar 31, 2017
@mahdihijazi
Copy link
Contributor Author

@lioncash it would be awesome if you also can take a look at PR 5160

@lioncash
Copy link
Member

@mahdihijazi done 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants