search

Akylas / conty

Interactive stories for kids. Download & play offline, anytime!
MIT License
9 stars 0 forks source link

IzzyOnDroid #2

Open farfromrefug opened 6 days ago

farfromrefug commented 6 days ago

@IzzySoft here is a new app of mine. Would love for it to be on IzzyOnDroid ;) Thanks!

IzzySoft commented 6 days ago 
! repo/com.akylas.conty_13.apk declares sensitive permission(s):
  android.permission.READ_EXTERNAL_STORAGE android.permission.READ_PHONE_STATE
  android.permission.MANAGE_EXTERNAL_STORAGE

READ_EXTERNAL_STORAGE I can understand. But MANAGE? Why would you need that? And why READ_PHONE_STATE (for incoming-call-detection, just register to the onAudioFocusChanged broadcast).

Another question: where are the stories downloaded from?

Apart from that: it will go online with the next sync around 6 pm UTC :smiley:

farfromrefug commented 6 days ago

@IzzySoft i removed the READ_PHONE_STATE which was coming from a dependency. Now about MANAGE_EXTERNAL_STORAGE. I use it because the app heavily uses file read, unzip, copy... (to import packaged stories in zip). SAF is a nightmare and is amazingly slow. Using MANAGE_EXTERNAL_STORAGE allows me to work directly with java.io.File and makes the app package processing 10 times faster. I will add a first dialog prompt to explain why i ask for that permission. BTW the app will still work if you refuse it.

EDIT: and i forgot to say thank you!

IzzySoft commented 6 days ago

Thank you! Yes, SAF is a pain especially with Android 5-11 (and the permission mess there). And for heavy file operations pretty slow, agreed. But why MANAGE and not just READ/WRITE? Ah, legacy and "ignored" with Android 12+ or something, right?

farfromrefug commented 6 days ago

@IzzySoft because READ/WRITE does not work on android >= 11. It is either SAF or MANAGE And btw it is even worth on android 10 where even with READ/WRITE there are some operations which wont work without SAF (like creating dirs on sdcard)

IzzySoft commented 6 days ago

because READ/WRITE does not work on android >= 11. It is either SAF or MANAGE

Thanks for confirming! I wasn't 100% sure. And now wonder how Goog then deals with it in their toy shop if they limit MANAGE to "well proven cases, mostly only file managers and such"… :man_shrugging:

on android 10

On 10 (and only there) you can opt into legacy, though. But that would make it complicated if you want to cover > 10 as well. Crazy stuff that…

farfromrefug commented 6 days ago

@IzzySoft i do opt in on android 10 for legacy storage but still does not work. Dont rrmeber were but it is written that some File operations might fail.... And indeed create dirs, delete them, fails even with the legacy storage flag...

IzzySoft commented 4 days ago

Wow. That's weird. But well, I believe you. Makes it even more crazy how they deal with it concerning PlayStore admission…

That leaves the two questions:

farfromrefug commented 4 days ago

@IzzySoft books can be either downloaded from the web or imported from a zip. I am 100% but then I think I need read storage for pre SAF devices ?

IzzySoft commented 4 days ago

books can be either downloaded from the web or imported from a zip.

Ah, that implicitly answers the reason of my question. So nothing reaching out to some non-free/fixed server by default – and the app would even work without any server as one can import from ZIP. Very good, so we don't need some NonFreeNet or such :wink:

I am 100% but then I think I need read storage for pre SAF devices ?

If there were any affected, yes. But which should that be? Conty requires at least Android-5, which was when SAF was introduced. Would you support devices on Android < 5, then yes. But you don't. So if you consequently use SAF for all file operations outside your app's dedicated storage (i.e. outside /data/data/com.akylas.conty/), you should need neither READ nor WRITE permissions. On first file access you'd need to pop up the file picker so one can choose a location – and as long as you operate inside that, all should be fine (to my understanding – I'm not an Android dev, so some things are "theory" to me and not first-hand experience).

IzzySoft commented 4 days ago

Oh, PS: I miss the IzzyOnDroid badge in the Readme (I just wanted to use that to quickly check your min Android version on the repo browser, but only found the PlayStore and Github badges there) :relaxed:

farfromrefug commented 4 days ago

I am 100% but then I think I need read storage for pre SAF devices ?

If there were any affected, yes. But which should that be? Conty requires at least Android-5, which was when SAF was introduced. Would you support devices on Android < 5, then yes. But you don't. So if you consequently use SAF for all file operations outside your app's dedicated storage (i.e. outside /data/data/com.akylas.conty/), you should need neither READ nor WRITE permissions. On first file access you'd need to pop up the file picker so one can choose a location – and as long as you operate inside that, all should be fine (to my understanding – I'm not an Android dev, so some things are "theory" to me and not first-hand experience).

Indeed i could use SAF bu i am thinking to not use SAF for devices < 10 since it is much faster (10 is sadly the one version where everything is messed up without real solution). So i guess i prefer READ/WRITE over SAF. Is that ok?

And I brought back the badge! Thank you

IzzySoft commented 4 days ago

Sure. SAF would be more granular and mostly guaranteeing which directories the app can access – but my question is mostly about transparency. So I've added the reasoning now (putting all storage permissions to your app's "green list" with an explanation). Thanks!

IzzySoft commented 3 days ago

I see you added Sentry. Is it strictly opt-in? Or is something of it enabled by default?

farfromrefug commented 2 days ago

@IzzySoft Damn that s a mistake should only be in playstore builds :s Will be removed in next ones... Really sorry

farfromrefug commented 2 days ago

@IzzySoft it is published https://github.com/Akylas/conty/releases/tag/android%2Fgithub%2F1.5.5%2F18

IzzySoft commented 2 days ago

Thanks for the swift action! Just manually triggered an update now to verify… Oops, now android.permission.MANAGE_EXTERNAL_STORAGE is back. And looks like you didn't increase versionCode (it's still 17 despite the tag indicating 18, which will break RB if set up – ah, wasn't (checking) ah, Typescript – well, no idea how to do RB with that). But Sentry is gone now, thanks!

farfromrefug commented 2 days ago

Managé permission is normal ! As we explained before in this issue. But indeed I forgot to increase the build number :( will.make a new one tomorrow morning Sorry

IzzySoft commented 2 days ago

No probs, and no need to hurry it. In this specific case it's even somehow useful it overwrites the existing APK – but only for new installs, and for those who did not yet update. So a timely update would be great indeed, but tomorrow is totally sufficient, thanks!

farfromrefug commented 2 days ago

@IzzySoft the new build is up https://github.com/Akylas/conty/releases/tag/android%2Fgithub%2F1.5.6%2F19. Sorry for all the trouble and thank your for your great support as usual!

IzzySoft commented 1 day ago

Thanks – and it just went live with today's sync :star_struck:

farfromrefug commented 11 hours ago

@IzzySoft i have a little question for you. iOS users are asking me to release ipa file through github (thanks to the new european law). So to do that i would need to create new releases on github (all my apps) specially for iOS. My question is would it be a problem for izzydroid if there was releases with only ipa and not apk?:

IzzySoft commented 6 hours ago

would your process ignore releases without apk?

It would log an error on each try – but yeah, it wouldn't "break" anything.

do you need a special rule to ignore those releases tags?

Well, that would be nice to avoid the error, yes. My guess is it won't even come to that as you most likely already plan what I suggest now:

AutoUpdateMode: Version ^android/github/[^/]+/%c$

is what's set up for Conty here currently. Your IPAs will most likely come with tags named matching the RegEx ^ios/github/.*, right? Those tags would be entirely ignored by the IoD updater (which only considers those matching the above RegEx). And before you ask: the %c stands for versionCode. So if the check finds a tag pointing to a versionCode not higher than what's already here, that would be ignored as well :stuck_out_tongue_winking_eye:

farfromrefug commented 6 hours ago

Well, that would be nice to avoid the error, yes. My guess is it won't even come to that as you most likely already plan what I suggest now:

AutoUpdateMode: Version ^android/github/[^/]+/%c$

is what's set up for Conty here currently. Your IPAs will most likely come with tags named matching the RegEx ^ios/github/.*, right? Those tags would be entirely ignored by the IoD updater (which only considers those matching the above RegEx). And before you ask: the %c stands for versionCode. So if the check finds a tag pointing to a versionCode not higher than what's already here, that would be ignored as well 😜

Yes that s the case i use ios in the tags and not android! So awesome! BTW will be the same for : https://github.com/Akylas/OSS-DocumentScanner https://github.com/Akylas/alpimaps/ https://github.com/Akylas/oss-weather/

IzzySoft commented 3 hours ago

Ah, while we're here… fastlane. Could you maybe adjust the full_description.txt a little? At least using bullet-points (asterisks, i.e. *, no graphical dots) in "how it works")? Then it could be rendered as Markdown – which would look much better than as-is. Be welcome to the IzzyOnDroid Fastlane Documentation for further hints :wink:

farfromrefug commented 3 hours ago

@IzzySoft should be better now.