Author Topic: Let's build for Android!  (Read 20416 times)

eri0o

Re: Let's build for Android!
« Reply #100 on: 25 Sep 2021, 00:36 »
@CrimsonWizard, I found the bug, it's here: https://github.com/adventuregamestudio/ags/blob/c3b286539df21180300c282c415c15f56d444be4/Common/core/assetmanager.cpp#L301

In the whole assetmanager resolution of things which I don't understand, after IsAssetLibDir returned true, it goes to GetAssetFromDir, and then instead of simply Opening the File Stream and being happy with it, it instead checks the file with Path::IsFile(found_file), but since it's just a pile of bytes inside of the AndroidAsset, it doesn't find anything in the directory with is file and returns false. Not sure how to solve this without throwing more ifdefs... It also will later try to get the file size for some reason and this will obviously fail too. Ideally, it would simply try to use File::OpenFile and if it fails then it would try the other things to see what's wrong...

This is why all packages works (.ags, .vox, ...) but not normal files in the directory.

But in short, there's a bug which I am not sure how to report or how to fix properly...

Edit: here's a fix that works https://github.com/adventuregamestudio/ags/compare/master...ericoporto:fix-maybe-assetmanager-android
« Last Edit: 25 Sep 2021, 01:22 by eri0o »

Crimson Wizard

  • Local Moderator
    • Lifetime Achievement Award Winner
    • Best Innovation Award Winner 2013, for spearheading the AGS 3.3.0 project
    • Crimson Wizard worked on one or more games that won an AGS Award!
    •  
    • Crimson Wizard worked on one or more games that was nominated for an AGS Award!
Re: Let's build for Android!
« Reply #101 on: 25 Sep 2021, 01:25 »
Well, the solution is then to either provide Android implementations for these two functions (IsFile and GetFileSize) which also use AAssetManager to check inside list of their assets, or refactor our AssetManager again to work by opening file stream right away. There has been time when AssetLocation was used outside of AssetManager, but not anymore.

I guess that in the end there have to be "filesystem" functions that cover all cases depending on the platform. Right now they are just spread around too much (File, Directory, and some in Path).

EDIT: how does it work with File::FindFileCI, should not it also fail in this case?

In the whole assetmanager resolution of things which I don't understand

Please tell what is causing difficulties to understand how it works, then I will add more comments.
« Last Edit: 25 Sep 2021, 01:43 by Crimson Wizard »

Crimson Wizard

  • Local Moderator
    • Lifetime Achievement Award Winner
    • Best Innovation Award Winner 2013, for spearheading the AGS 3.3.0 project
    • Crimson Wizard worked on one or more games that won an AGS Award!
    •  
    • Crimson Wizard worked on one or more games that was nominated for an AGS Award!
Re: Let's build for Android!
« Reply #102 on: 26 Sep 2021, 05:54 »
I will probably look if it's possible to simplify the asset manager later today. In general, i think, there has to be at least 2 file functions that provide android-specific handling: "open file" and "find file ci", because existing "open file" function is meant to open a file of precise name, while "find file ci" checks for case insensitive variants too.
« Last Edit: 26 Sep 2021, 07:09 by Crimson Wizard »

eri0o

Re: Let's build for Android!
« Reply #103 on: 26 Sep 2021, 10:32 »
ok, did you see my quick fix above?

I am a bit without free time right now, so I couldn't spend more time. :/

Crimson Wizard

  • Local Moderator
    • Lifetime Achievement Award Winner
    • Best Innovation Award Winner 2013, for spearheading the AGS 3.3.0 project
    • Crimson Wizard worked on one or more games that won an AGS Award!
    •  
    • Crimson Wizard worked on one or more games that was nominated for an AGS Award!
Re: Let's build for Android!
« Reply #104 on: 26 Sep 2021, 15:06 »
ok, did you see my quick fix above?

Yes I've seen it. Problem is this function GetAsset* is used for two purposes: opening an asset and simply checking that one exists. I don't think that it's a good thing to open a file and a stream when you only need to test that a file is present, so either these methods have to be refactored, or there has to be an android-specific implementation for the function that tests the file (asset) presence. EDIT: OTOH, since there's a case when you need to only check the file presence, such implementation is necessary anyway.

But also my question was, what does File::FindFileCI return in this case? Because it seems like it's searching only on disk, so I'm wondering how it works on Android. Does it use case-sensitive or case-insensitive method? EDIT: How is AGS_CASE_SENSITIVE_FILESYSTEM flag is defined? I can't seem to find it anywhere, except the Makefile-defs.linux, but what about CMake scripts?...

PS. I might make a small test pr later today, containing android implementations for few more file functions, if I learn how to do them with this AAssetManager thing.
« Last Edit: 27 Sep 2021, 10:09 by Crimson Wizard »

eri0o

Re: Let's build for Android!
« Reply #105 on: 27 Sep 2021, 10:58 »
CMake sets case insensitiveness forced here for Linux (and this is the only place):

https://github.com/adventuregamestudio/ags/blob/9887c180e10175bf8d67908d219d711f16088924/Common/CMakeLists.txt#L198

This is not currently set for Android. Is this flag needed beyond backward compatibility?

About AAssetManager, the only functions are those linked below. Some are marked with a note they don't work if the file is compressed, but I haven't tested those to confirm, I avoided those at the time of making AAssetStream because for that it felt it wasn't necessary. AAssetDir_getNextFileName is the way to list "files in dir" in Android asset manager.

https://developer.android.com/ndk/reference/group/asset

Crimson Wizard

  • Local Moderator
    • Lifetime Achievement Award Winner
    • Best Innovation Award Winner 2013, for spearheading the AGS 3.3.0 project
    • Crimson Wizard worked on one or more games that won an AGS Award!
    •  
    • Crimson Wizard worked on one or more games that was nominated for an AGS Award!
Re: Let's build for Android!
« Reply #106 on: 27 Sep 2021, 23:04 »
Ah. I also found that the AAssetManager reference is used incorrectly; it's being queried and added a reference on each open stream, but these references are never deleted.
According to this page you got to call DeleteGlobalRef on objects that were received with the NewGlobalRef.
This may also be seen in the SDL2 sources, where it initializes AAssetManager only once and also has a function that releases its reference.

I don't know which yet, but we need to either have similar global create/delete functions in the Common, and explicitly call them on engine start/quit, or alternatively make AAssetManager owned by the platform driver and somehow call it from the common lib.

EDIT: I did this for now: https://github.com/ivan-mogilko/ags-refactoring/commits/android--filefuncs2
where platform driver calls initialization in Common. Alternatively, can make platform driver contain AAssetmanager's reference, and only pass it into the Common.
Will investigate the AAsset functions you linked now, hopefully it's not too complicated to make it work consistently with file funcs.

... although i keep wondering why we are not using SDL2 directly, as it seems to be doing all this already. Maybe this requires moving to use its RW Ops? In which case I'd delay such transition until next version.

EDIT 2: After more thinking, I realized there's a way and sense to also simplify AGS AssetManager further, and do something similar to your proposal, where it opens a stream right away, so to avoid extra steps.


CMake sets case insensitiveness forced here for Linux (and this is the only place):

https://github.com/adventuregamestudio/ags/blob/9887c180e10175bf8d67908d219d711f16088924/Common/CMakeLists.txt#L198

This is not currently set for Android. Is this flag needed beyond backward compatibility?

This is a good question. Yes, if Android is case sensitive system then it should also use case insensitive file handling, at least in 3.* branch, which is supposed to be a backward compatible, otherwise some existing games may fail on Android with the AGS player/launcher.

Also, in theory, because the engine suppose to work same way on case sensitive and insensitive systems, it may be necessary in general; for example someone has a file with lowercase name in the game, but calls it uppercase in script. That script will work on Windows, but not on Linux.
That is, unless we explicitly state that we hold no responsibility for such mistakes.

On the other hand, this case-insensitive handling currently only works when searching for existing files, but not when e.g. writing a file. If the game script writes two files which differ only by letter case, both of these may be opened on Linux, but not on Windows, where you won't be even able to create second one without overwriting first.

But this is also a question of how engine is supposed to work by design. I recall there have been already a small discussion some time in the past, but I don't remember in which forum topic. I believe for ags4 it's important to make a definitive decision on this.
« Last Edit: 29 Sep 2021, 10:22 by Crimson Wizard »

Crimson Wizard

  • Local Moderator
    • Lifetime Achievement Award Winner
    • Best Innovation Award Winner 2013, for spearheading the AGS 3.3.0 project
    • Crimson Wizard worked on one or more games that won an AGS Award!
    •  
    • Crimson Wizard worked on one or more games that was nominated for an AGS Award!
Re: Let's build for Android!
« Reply #107 on: 30 Sep 2021, 10:46 »
@eri0o, well in the end I did both, changed engine's AssetManager to not do unnecessary steps when opening data for reading (this might also be enough to fix this particular problem with translations), and opened a PR for additions to Android file handing (includes "findfile" implementation too): https://github.com/adventuregamestudio/ags/pull/1421

Crimson Wizard

  • Local Moderator
    • Lifetime Achievement Award Winner
    • Best Innovation Award Winner 2013, for spearheading the AGS 3.3.0 project
    • Crimson Wizard worked on one or more games that won an AGS Award!
    •  
    • Crimson Wizard worked on one or more games that was nominated for an AGS Award!
Re: Let's build for Android!
« Reply #108 on: 30 Sep 2021, 23:06 »
@Amir, the modified native libraries for Android may be downloaded here:
https://cirrus-ci.com/task/6279218209226752
where it sais "Artifacts" -> "libs"
if you've got time to make another test.
« Last Edit: 01 Oct 2021, 01:03 by Crimson Wizard »

Re: Let's build for Android!
« Reply #109 on: 01 Oct 2021, 12:29 »
@Amir, the modified native libraries for Android may be downloaded here:
https://cirrus-ci.com/task/6279218209226752
where it sais "Artifacts" -> "libs"
if you've got time to make another test.

I tested (the game from the 3.6.0.9 version), cool, translations work :) thank u. Sound cutting is still there. eri0o is still working on it I guess?
But it's still running very slowly. For example the sounds are out of sync with the animations, the sounds are 2 seconds faster than the animations becoz it runs slow. Is that related to Android Studio or libs(release)? 
Truly, truly, i say to you, blessed are those who play adventure games, for theirs is the kingdom of heaven.