search

diasurgical / devilutionX

Diablo build for modern operating systems
Other
7.98k stars 781 forks source link

[Issue Report]: import data breaks mpq files #6756

Closed Krzysiu closed 10 months ago

Krzysiu commented 10 months ago

Operating System

Android

DevilutionX version

1.5.1

Describe

I tried the "import data" function, which wasn't described. As the game couldn't find mpq files (they were there, tho), I thought maybe importing mpq will help. After selecting diabdat.mpq there weren't any error message, but next time I opened the game, game served error saying data is broken. Indeed after "import" diabdat was 0 byte long. I still don't know exactly what "import data" does, but I'm sure it's not supposed to nullify files.

To Reproduce

  1. Long tap on the devilution icon
  2. choose "import data"
  3. choose diabdat.mpg

Expected Behavior

show error message or info what "import data" really is

Additional context

No response

AJenbo commented 10 months ago

Where the files you select already in the application? If so you are transfering the files on top of them selfs.

Import data copies files in to the apps folder. You can find a description here: https://github.com/diasurgical/devilutionX/wiki/Extracting-the-.MPQs-from-the-GoG-installer#android

Krzysiu commented 10 months ago

Yes, they were. That sounds like a reason, but I think app should do a simple check to avoid essentially deleting them, even if it was clearly user's fault.

AJenbo commented 10 months ago

sure

StephenCWills commented 10 months ago

Hm, this sounds easier than it actually is. Seems we don't get the full path to the file we're importing. Here's what mine looked like: /document/primary:DevilutionX/DIABDAT.MPQ

The good(?) news is, Android 14 pretty much completely disallows access to Android/data folders so this shouldn't really be a problem if you upgrade. Perhaps I can do a little more testing with older versions of Android, but I'm not sure how much we can do to help prevent this type of mistake. Maybe a simple popup message would be the best approach. Something like...

On the next screen, select files that will be imported into <externalFilesDir>.

AJenbo commented 10 months ago

We could check for the base name and ask if they really want to overwrite, or just flat out reject matching base names.

StephenCWills commented 10 months ago

I think there are a couple problems with just checking matching filenames.

  1. Since the folder is flat-out inaccessible in Android 14, it is strictly necessary to allow the user to overwrite the file that's already there, if necessary. This doesn't do much to protect the user from making the same mistake because...
  2. If you don't mention what Import Data actually does, how is the user supposed to know what it is they're overwriting? There's no basis for determining whether to allow the overwrite or not.
AJenbo commented 10 months ago

Might still be good to notify them that they are about to overwrite a file (if we can do that).