Let's build for Android!

Started by eri0o, Mon 08/01/2018 00:35:15

Previous topic - Next topic

eri0o

#100
@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

Crimson Wizard

#101
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?

Quote from: eri0o on Sat 25/09/2021 00:36:20
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.

Crimson Wizard

#102
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.

eri0o

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

#104
Quote from: eri0o on Sun 26/09/2021 10:32:21
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.

eri0o

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

#106
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.


Quote from: eri0o on Mon 27/09/2021 10:58: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?

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.

Crimson Wizard

@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

#108
@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.

Amir

Quote from: Crimson Wizard on Thu 30/09/2021 23:06:15
@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.

FanOfHumor

Is anyone planning on making this able to create AGS apps on android easily?

Crimson Wizard

#111
Quote from: Pajama Sam on Thu 30/12/2021 06:31:41
Is anyone planning on making this able to create AGS apps on android easily?

There has been a plan for 3.6.0 to setup a game for Android in the Editor, so that it creates a package, similar to how it does for Linux now.

If I understand correctly, we have a working method now, but it still must be run by hand. So the remaining tasks are:
1) Add a Android setup menu into the Editor (like a panel where you enter necessary options for your game);
2) Make Editor run APK building.

Presumably a user will still have to install some tools on their own (Android SDK, etc); as we do not want to distribute these big programs with AGS (and maybe that is also illegal, idk).

Now, the biggest question is who is going to do this and when. I've been more focused on the engine lately, and the person who updated Android port (eri0o) is currently away busy with something. OK, he's back now.
So unless someone else does this, maybe I will be able to look into this after 3.6.0 gets into the "Beta" stage at least.

Amir

I hope u can do it. I think that's the only weakness of the AGS engine. I know some people who decided to switch to a different engine just because of the lack of Android  :(
Truly, truly, I say to you, blessed are those who play adventure games, for theirs is the kingdom of heaven.

Crimson Wizard

Ok, so eri0o is back now, and we continue to update Android tasks, so there's some hope :).

Amir

Truly, truly, I say to you, blessed are those who play adventure games, for theirs is the kingdom of heaven.

SMF spam blocked by CleanTalk