yairm210 / Unciv

Open-source Android/Desktop remake of Civ V
Mozilla Public License 2.0
8.13k stars 1.52k forks source link

Mods that add civilizations don't load (Android - Unciv 3.17.0) #5309

Closed osjasmine closed 2 years ago

osjasmine commented 2 years ago

Platform Android 10 (32 bit). Device: Samsung A10

Version Unciv 3.17.0, Github (the bug was also present in F-Droid version - 3.16.15-patch2)

Describe the bug I can download mods but in the new game settings screen, when I check/select mods that add new civilizations, they don't load. For example when I check Ukraine Mod, I don't see Ukraine in the Civilizations list.

To Reproduce Steps to reproduce the behavior:

  1. Download any of the many civilizations mods.
  2. Start new game > Select the mod
  3. Then check the civilization list. You will find that no new civilizations have been added.

Additional context This bug was present in all mods that add new civilizations except in Kakaoalan. Kakaoalan is the only one that actually loads. Also the mod Difficulty_Sandbox doesn't work. Other mods like DecivRedux work.

I have already tried clearing data and reinstalling the app but no change.

osjasmine commented 2 years ago

The last version where civilization mods work is 3.16.0. In 3.16.0-patch1 (and all subsequent versions) the mods stop working.

SomeTroglodyte commented 2 years ago

That is very likely because many mods don't follow the rules. Which is because we're changing the rules - to make following the rules simpler in the future.

You should get explicit reasons what's wrong on the "mod check" tab of the options dialog.

SomeTroglodyte commented 2 years ago

Ukraine works perfectly fine for me. Difficulty_Sandbox works. Latin american civs work.

OK, some mods stumble over a regression due to Civilopedia text accessing a current player - which shouldn't be, Civilopedia is about globally valid info, not player-specific. But if that one hits, you'll crash, not search in vain for the nation in the picker. (fix underway)

osjasmine commented 2 years ago

Thanks for trying to help me out. I tried reproducing this another phone (Android 9) but I couldn't so its probably something wrong with my phone. I will try reproducing this issue on another phone running Android 10 and if Im not able to do so, you can close this issue.

Other info: I found out that if I install a civilisation mod (like the Ukraine mod) in Unciv 3.16.0 and then update Unciv to the latest version on Fdroid (3.17.0), the mod still works (when it wouldnt work on fresh install of Unciv 3.17.0).

Plus this showed that the issue isnt with the mods. On a fresh install of Unciv 3.17.0 the "Ancient Civilizations" mod wouldn't load but it showed no errors. But when I installed it on Unciv 3.16.0 then updated to Unciv 3.17.0, I couldn't select the mod at all as I got an error telling me it wasnt compatible when I tried selecting it. Its almost like in fresh install of Unciv 3.17.0, the game doesnt even try to load the mod.

This bug is with all mods that are supposed to work before the game loads.

Comparing Unciv 3.16.0 and 3.16.0-patch1, there is only one commit that relates to mods. I wonder if reverting that commit and then compiling Unciv 3.16.0-patch1 will fix this issue but Im not a dev and dont know how to compile.

SomeTroglodyte commented 2 years ago

.... nnnn...no. That commit changed display-only stuff, optimized a rename, and its core feature was to use a generated, unique name for the temporary download file and extract folder instead of a constant one - basically allowing you to download mods in parallel if you're quick enough and don't mind that the lower right button sometimes says success but still initiates the next DL on click.

So unless that rename thingy fails on some context - and it should catch a fail and revert to copy-then-delete - no that commit can't be "it".

Still curious. You clean up, reinstall 3.17.x, then launch right away, then open mod manager without detours, download a mod and wait for confirmation, so it appears in the left column, then leave mod manager but do not quit the game, go to new game, activate mod, check player picker. Right? OK, does something change when you quit Unciv after using the mod manager and relaunch before going to new game?

Also - do you have an SD card in the slot and if so, have you set it to portable or to "integrate" with internal (dunno what it's really called)? Anything special about Unciv moved to SD or not?

osjasmine commented 2 years ago

I agree with you but Unciv 3.16.0 is the last version where all mods work for me while from 3.16.0-patch1 the mods no longer work. So there has to be something between these two versions thats causing the issue.

I followed what you said but the mods still didnt work. I didnt notice any changes in either of the scenarios.

I do have an sdcard mounted, I think its in the portable state (iirc samsung doesnt support integrate into internal storage option). I also unmounted the sdcard and then installed 3.17.0 but there was no change.

SomeTroglodyte commented 2 years ago

Oh, my. On an Android Q AVD the additional atlas/texture files do not even load, so something is definitely not right. But loading Medieval Civs and immediately staring as Hungarians does work.

SomeTroglodyte commented 2 years ago

That's two bugs fixed thanks to your input (#5313), but neither have anything to do with what you describe. That I still cannot reproduce on any environment I have available, and that includes an Android 9 hardware (actually Lineage 16), and Android 10 AVD (meaning running an actual 'roid system image in an emulator - but that's close to AOSP, not to Samsung-mangled 'roid).

What you describe sounds like incomplete mod files - folder exists, but at least one json missing. You wouldn't happen to have root and a file manager to check what exactly happened in /data/data/com.unciv.app/files/mods? Or logcat access?

Another thing: The screen right after a download should look like this image (Plus there's a toast for 2 seconds near the top) - You getting the mod stats down left and the button saying "downloaded" lower right like that or is something missing?

osjasmine commented 2 years ago

Oh I figured a workaround. I went to the data directory that you mentioned and there I could see the Latin Civilisation mod folder. I looked through the folder but didnt notice anything out of ordinary.

Then I remembered that the Kakaoalan civ mod worked so I installed that in Unciv and looked through the files. I noticed that while in the Latin civ mod folder some jsons files were in the main mod folder, in the Kakaoalan mod all the jsons files + translations were in the jsons folder. So I copied all the jsons to jsons folder (in the Latin civ mod) and the mod was working no problem at all 😁.

Now I was curious why the mods work in 3.16.0 and especially even after updating Unciv. So I installed Unciv 3.16.0 and then installed the Latin civ mod. And to my suprise the jsons were correctly placed in the jsons folder.

So for some reason in Unciv 3.17.0 the jsons are not placed in the jsons folder for all civ mods except for Kakaoalan mod.

As for the mod screen, its the same as yours.

Screenshots of folders (3.17.0): latin civ mod files kakaoalan mod files

SomeTroglodyte commented 2 years ago

So the unpacking does misplace files... Extra strange then that the mod screen seems to manage the mod load once, or you wouldn't have nation/building/units etc counts in the details pane where it says "installed: X Nations..." next to the close button.

SomeTroglodyte commented 2 years ago

... And those 2 mods have no significant differences in their file structure at the source ...

So I'd say file system level operations, namely java.io.File.renameTo, are broken. It must be reporting success through its return value without having actually done its job. And then you were right about that commit provoking this.

SomeTroglodyte commented 2 years ago

If you're willing to experiment, then...

It's essentially "if Java rename reports success, check if it lies. If the move hasn't happened, revert to Gdx copy+delete" - so if that works, it proves your OS-level libraries are the cause. If so, I'm not sure this workaround is the best answer for the master branch, but I have a basis to operate from.

osjasmine commented 2 years ago

Extra strange then that the mod screen seems to manage the mod load once, or you wouldn't have nation/building/units etc counts in the details pane where it says "installed: X Nations..." next to the close button.

Oh sorry didnt realise that there were some stats down there (even though you mentioned them before, I guess I wasnt paying attention). Now that I look at it again, it only says installed for Latin civ mod but for Kakaoalan, the complete stats are shown, with the no. of nations and stuff. So the feature is working fine (also thats a pretty cool feature).

Im unable to download the debug as the link ends up giving me an http 406 error. (And also thank you so much for helping me so far.)

SomeTroglodyte commented 2 years ago

Link fixed. Sorry I didn't check it - dumb copy&paste mistake.

it only says installed for Latin civ mod but for Kakaoalan, the complete stats are shown

Observation supports analysis so far.

osjasmine commented 2 years ago

The debug apk didnt work, with files still being misplaced. I didnt notice any improvments.

SomeTroglodyte commented 2 years ago

Should have marked it, so we could be 100% sure you're running the right code...

osjasmine commented 2 years ago

Should have marked it, so we could be 100% sure you're running the right code.

Sorry, I didnt get what you meant by marked. I guess you are saying that you should have changed the app ver or something to indicate that Im running the right code. I did make sure that I had the debug apk installed.

(PS: I feel like this issue is probably pretty rare, and doesnt seem to have any obvious fixes, so might not be worth fixing. Copying the jsons files to the right folders is a good enough workaround for me. I wouldnt mind if you closed this issue.)

SomeTroglodyte commented 2 years ago

One last test if you still got the patience? this fresh apk basically reverts the rename/move changes from that commit - and it has its own version number you can check if you want.

rare

I'd say so, yes, but investigating has already led to very important improvements, even if those were not directly related. Worth it.

Final verdict depends on your answer (don't wanna anymore / that apk loads mods better / no change) - middle option hardest to answer, because that "fix" not only costs performance, but some non-trivial unnecessary SSD write cycles.

osjasmine commented 2 years ago

It works OMG! All mods are working, even the Sandbox mod. Also thank you so much! I was thinking I was giving you too much trouble but Im glad you find it worthwhile. As for whether to implement the fix, its upto you. Probably better to leave it unfixed.

SomeTroglodyte commented 2 years ago

I'll Let it ripen in the brainbox for a while. Maybe a "quirks mode" switch.

For the time being, I'll close this. Remember: You can't update that version from any store - different signatures. You have read the doc. Pity there's no LOS build for the A10, that would likely help, too.