Github action workflow for android nightly build

[krosk]

Hello,

Following up on topic at https://forum.simutrans.com/index.php/topic,21054.0.html.

I have setup a github action workflow at https://github.com/krosk/simutrans-android-docker-build/actions/workflows/nightly-android.yml
- On push, it builds the latest svn trunk; as of today it is hitting the r10015, which has a clang compilation error from code introduced days ago (https://github.com/krosk/simutrans-android-docker-build/runs/3345530926?check_suite_focus=true and the likely commit of interest https://github.com/aburch/simutrans/commit/446284d85e8df07fffc73bfb200a23086d562388), so it does not pass at the moment.
- As a result, I had to introduce a build on dispatch so as to target and release a given revision (example https://github.com/krosk/simutrans-android-docker-build/runs/3345540040?check_suite_focus=true). Useful for checking earlier versions since this android target is new, but ultimately this should be a temporary measure.

The workflow generates an APK, patches the configuration file with the version number reported by SVN revision, and overwrites the Nightly release APK, the same way as it is done today for the other builds. Note also that the manual dispatch will still overwrite the Nightly.

The attached patch covers the yaml of the workflow above + a patch on top of pelya repository that fix a video glitch fix and change default resolution.

[prissi]

Sorry, I am a little unsure, I correctly understood. This means this will not run out of a native repo, but needs to be run from pelya? I.e. putting this into the simutrans workflow it will not work?

On the other hand, I would not mind downloading the nightly from some other place, since the building server downloads and repack and pushed the mac also from github. Thus just using your github address would work for me too.

[krosk]

My apologies, the wording was probably confusing.

This is absolutely a workflow meant to run from github actions, on simutrans repo. The patch I provided is to be applied to simutrans trunk and will add a nightly build for android. I made a fork of the github simutrans repo to demonstrate.
https://github.com/krosk/simutrans/actions

About the sentence 'a patch on top of pelya': the patch I provided, aside the nightly build definition, also adds a file, that is a patch on top of pelya's repo; the workflow is, in addition to simutrans repo, pulling from their repository for building reasons but it needs a patch to work.

[prissi]

Not sure when and to what purpose a docker container is added here. Also I added your fluidsynth changes. But I do not understand the chnage to mx/cx. cx contains the first point when clicked and mx the current position when releasing. Why are they assigned?

Anyway I submitted it and I am waiting to see it build (or not). That will take a while, since I do not control the guthub repo (only my svn server).

[krosk]

prissi, sorry for the confusion.

1/
Only one commit is relevant for you:
https://github.com/krosk/simutrans/commit/06bdb66428312326f5ccf3b291cbf1f0e710658e
This is simply the patch I attached above.

Everything else is me playing around with the source on my terms, on this fork, and has nothing to do with the topic ! I sent a link to my fork as an example of adding the build pipeline, but not saying the whole fork is relevant. The stuff with cx/mx is indeed irrelevant.

2/
Not understanding your comment of the purpose of docker container. Anyway, no docker is involved in my fork of simutrans repo.
However, in the repo of the dockerfile I spoke about in the other topic, I made another github action pipeline that makes use of the dockerfile instead of putting every single step in the github action workflow. But this is not the approach used for this patch either.

3/
https://github.com/krosk/simutrans/commit/25c021e8aced153479407a1f767749734ac48fb9
To clarify, the change of fluidsynth is a revert of a previous commit because it makes clang not passing compilation for Android, so this arguably is required to make the Android build pass; I did mention this build problem at the very top, but this is not part of the patch I provided. In git it would have been more proper to revert the target commit in its own commit so we don't mix things up; I am not sure what is your workflow with svn.


Bottom line: there are two set of changes to do:
a/ this adds the nightly build https://github.com/krosk/simutrans/commit/06bdb66428312326f5ccf3b291cbf1f0e710658e
b/ this reverts a previous commit incompatible with clang used for Android https://github.com/krosk/simutrans/commit/25c021e8aced153479407a1f767749734ac48fb9

[prissi]

1) The patch file in a patch has my patch program spit out nonsense. I need to download this manually.

In the link given, you are forcing a revision number for the nightly. This is very wrong, it should use the current nightly revision number. So I removed these lines.

2) There were some lines

sudo chown -R runner /opt/android-sdk-linux
sudo chgrp -R docker /opt/android-sdk-linux
sudo chown -R runner /android-sdl
sudo chgrp -R docker /android-sdl

So I wondered about docker. But the github action systems have all kind of surprises ...

3) Since passing compilation is a very good reason to have this in fluidsynth (with was a recent addition I know next to nothing since I do not like the midi and disable it as default) it is better to use defines to succeed compiling with Android and leave the rest as it was. But I can revert it.

I thought I have put your yaml (minus forcing a revision). Also the current Simutrans version is in simversion.h, so I wonder if one can use this to make an correct patching. I then submit everything to the svn which get then pushed to github

[krosk]

prissi, thanks for the answers.

1/ Sorry for the akward 'patch in a patch' thing.

About the lines for the revision:

Code Select Expand

      revision:
        description: 'Target revision number (SVN), empty for latest'
Guthub actions allows running a build on-demand (so called workflow dispatch). So:
- if the build is executed as part of a new commit, these lines are ignored and the workflow checkouts the newest version of the repository <<< this is what is intended for a nightly build, and this is the default behavior
- if the build is executed manually via the github actions interface (there is a button 'run workflow'), these lines are taken into account; Now, the meaning of these lines is to checkout a particular target version of the svn repo; useful for debugging, or getting back an APK of a previous version. Arguably, this second mode is not what you expect of a "nightly" and would have been more appropriate for a different, build-on-demand workflow. So you will want to remove everything related to it. I attached a patch.


2/ Github actions is running a clean, ubuntu-based (or windows-based), docker container each time a build is performed.
The steps in the yaml file are executed within this container, as user 'runner' with the group 'docker'.
Those lines you quoted are to assign the correct permissions to the folders I create, so that following steps are permitted to work within them.

3/ The approach of using defines is appropriate, and you were right to do it this way. Reverting was never the true option, which is why I merely raised the issue without forcing this in the patch.
Now there is a compile error for android and macos, complaining about not matching #if and #endif. I believe the line 212 in the file music/fluidsynth.cc is an endif that breaks what you intended to do. Removing it would make it compile. Still, I suggest a simpler refactoring and attached a patch.

[prissi]

Thank you, I learned another new thing by this.

[Yona-TYT]

I wonder why there is a .patch file in the github repository?. 
https://github.com/aburch/simutrans/blob/master/.github/android/0001-ADD-patch-simutrans-122.0-compatibility.patch

[krosk]

Yona-TYT,

pelya repository for libsdl-android contains a set of configuration files and building scripts customized for simutrans. The configuration that pelya provided is today incorrect (it makes the screen flicker).

I submitted a PR on pelya repository to correct the issue, and until that PR gets accepted, I still have to make changes on top of pelya's repository so the APK built is functional.

The build pipeline clones pelya repo, and applies the patch you point out to correct the mentioned issues. It is awkward, but it is temporary (until pelya patches the issue) so we get a functional APK. An alternative would have been to fork pelya's repo, patch the fork, and clone from the fork.



prissi,

I have seen the line I mentioned being removed, but that is not the only compilation issue as some code was duplicated on the commit https://github.com/aburch/simutrans/commit/4a13eef290f9fd9c13b1432a1fa1465c5da31ed0.
Please consider the 2nd patch I provided; it is an simpler alternative to the linked commit, because it reduces the whole change in fluidsynth to only 2 lines of code.

[Andarix]

r10019

...
2021-08-18T15:26:38.8126422Z ===> HOSTCXX bauer/fabrikbauer.cc
2021-08-18T15:26:38.8576048Z /opt/android-sdk-linux/ndk/23.0.7599858/toolchains/llvm/prebuilt/linux-x86_64/bin/armv7a-linux-androideabi16-clang++  -g -ffunction-sections -fdata-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -mthumb -Wformat -Werror=format-security -Oz -DNDEBUG -fPIC  -isystem/android-sdl/project/jni/application/../sdl-1.2/include  -isystem/android-sdl/project/jni/application/../jpeg/include -isystem/android-sdl/project/jni/application/../png/include -isystem/android-sdl/project/jni/application/../ogg/include -isystem/android-sdl/project/jni/application/../flac/include -isystem/android-sdl/project/jni/application/../vorbis/include -isystem/android-sdl/project/jni/application/../freetype/include -isystem/android-sdl/project/jni/application/../jpeg/include -isystem/android-sdl/project/jni/application/../png/include -isystem/android-sdl/project/jni/application/../freetype/include -isystem/android-sdl/project/jni/application/../bzip2/include -isystem/android-sdl/project/jni/application/../fluidsynth/include  -fpermissive  -frtti -fexceptions -g -ffunction-sections -fdata-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -mthumb -Wformat -Werror=format-security -Oz -DNDEBUG -fPIC  -isystem/android-sdl/project/jni/application/../sdl-1.2/include  -isystem/android-sdl/project/jni/application/../jpeg/include -isystem/android-sdl/project/jni/application/../png/include -isystem/android-sdl/project/jni/application/../ogg/include -isystem/android-sdl/project/jni/application/../flac/include -isystem/android-sdl/project/jni/application/../vorbis/include -isystem/android-sdl/project/jni/application/../freetype/include -isystem/android-sdl/project/jni/application/../jpeg/include -isystem/android-sdl/project/jni/application/../png/include -isystem/android-sdl/project/jni/application/../freetype/include -isystem/android-sdl/project/jni/application/../bzip2/include -isystem/android-sdl/project/jni/application/../fluidsynth/include  -fpermissive  -O1 -DNDEBUG -DUSE_FREETYPE -I/android-sdl/project/jni/application/../freetype/include -DUSE_FLUIDSYNTH_MIDI -Wall -Wextra -Wcast-qual -Wpointer-arith -Wcast-align  -I/android-sdl/project/jni/application/../sdl-1.2/include -D_GNU_SOURCE=1 -D_REENTRANT -DCOLOUR_DEPTH=16 -c -MMD -o build/armeabi-v7a/bauer/fabrikbauer.o bauer/fabrikbauer.cc
2021-08-18T15:26:38.9478993Z music/fluidsynth.cc:199:2: error: unterminated conditional directive
2021-08-18T15:26:38.9481309Z #ifdef _WIN32
2021-08-18T15:26:38.9482549Z  ^
2021-08-18T15:26:38.9924298Z ===> HOSTCXX bauer/goods_manager.cc
2021-08-18T15:26:38.9990093Z 1 error generated.
2021-08-18T15:26:39.0482872Z make[1]: *** [common.mk:51: build/armeabi-v7a/music/fluidsynth.o] Error 1
2021-08-18T15:26:39.0495669Z /opt/android-sdk-linux/ndk/23.0.7599858/toolchains/llvm/prebuilt/linux-x86_64/bin/armv7a-linux-androideabi16-clang++  -g -ffunction-sections -fdata-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -mthumb -Wformat -Werror=format-security -Oz -DNDEBUG -fPIC  -isystem/android-sdl/project/jni/application/../sdl-1.2/include  -isystem/android-sdl/project/jni/application/../jpeg/include -isystem/android-sdl/project/jni/application/../png/include -isystem/android-sdl/project/jni/application/../ogg/include -isystem/android-sdl/project/jni/application/../flac/include -isystem/android-sdl/project/jni/application/../vorbis/include -isystem/android-sdl/project/jni/application/../freetype/include -isystem/android-sdl/project/jni/application/../jpeg/include -isystem/android-sdl/project/jni/application/../png/include -isystem/android-sdl/project/jni/application/../freetype/include -isystem/android-sdl/project/jni/application/../bzip2/include -isystem/android-sdl/project/jni/application/../fluidsynth/include  -fpermissive  -frtti -fexceptions -g -ffunction-sections -fdata-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -mthumb -Wformat -Werror=format-security -Oz -DNDEBUG -fPIC  -isystem/android-sdl/project/jni/application/../sdl-1.2/include  -isystem/android-sdl/project/jni/application/../jpeg/include -isystem/android-sdl/project/jni/application/../png/include -isystem/android-sdl/project/jni/application/../ogg/include -isystem/android-sdl/project/jni/application/../flac/include -isystem/android-sdl/project/jni/application/../vorbis/include -isystem/android-sdl/project/jni/application/../freetype/include -isystem/android-sdl/project/jni/application/../jpeg/include -isystem/android-sdl/project/jni/application/../png/include -isystem/android-sdl/project/jni/application/../freetype/include -isystem/android-sdl/project/jni/application/../bzip2/include -isystem/android-sdl/project/jni/application/../fluidsynth/include  -fpermissive  -O1 -DNDEBUG -DUSE_FREETYPE -I/android-sdl/project/jni/application/../freetype/include -DUSE_FLUIDSYNTH_MIDI -Wall -Wextra -Wcast-qual -Wpointer-arith -Wcast-align  -I/android-sdl/project/jni/application/../sdl-1.2/include -D_GNU_SOURCE=1 -D_REENTRANT -DCOLOUR_DEPTH=16 -c -MMD -o build/armeabi-v7a/bauer/goods_manager.o bauer/goods_manager.cc
2021-08-18T15:26:39.0506047Z make[1]: *** Waiting for unfinished jobs....
2021-08-18T15:26:40.5118481Z make[1]: Leaving directory '/android-sdl/project/jni/application/simutrans/simutrans'
2021-08-18T15:26:40.5128036Z make: *** [CustomBuildScript.mk:54: simutrans/libapplication-armeabi-v7a.so] Error 1
2021-08-18T15:26:40.5133744Z make: Leaving directory '/android-sdl/project/jni/application'
2021-08-18T15:26:40.5141465Z ##[error]Process completed with exit code 1.
2021-08-18T15:26:40.5252244Z Post job cleanup.
2021-08-18T15:26:40.7506555Z [command]/usr/bin/git version
2021-08-18T15:26:40.7561484Z git version 2.32.0
2021-08-18T15:26:40.7625083Z [command]/usr/bin/git config --local --name-only --get-regexp core\.sshCommand
2021-08-18T15:26:40.7658499Z [command]/usr/bin/git submodule foreach --recursive git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :
2021-08-18T15:26:40.7933682Z [command]/usr/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
2021-08-18T15:26:40.7955997Z http.https://github.com/.extraheader
2021-08-18T15:26:40.7966172Z [command]/usr/bin/git config --local --unset-all http.https://github.com/.extraheader
2021-08-18T15:26:40.8002461Z [command]/usr/bin/git submodule foreach --recursive git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader'; || :
2021-08-18T15:26:40.8316190Z Cleaning up orphan processes

[Roboron]

Talking about fluidsynth, does the music work on Android? I don't have an Android to test. The audio driver used is sdl2 (sdl1.2 is enough?) and a recent version of fluidsynth is needed (>=2.1.0, early 2020) for this driver. If such versions are not available, a different driver for Android may be used.

P.D.: Thank you very much for your work on this, krosk. I'm very happy to see Simutrans working on a mainstream mobile OS finally, even if there are still a lot of adjustments to be done to be playable.

[ceeac]

r10019

Fixed now (r10020).

[prissi]

@Krosp The patch for fluidsynth could not directly applied to the head. So I tried to extract that changes manually, an error prone process. Please provide a patch to either a certain revision (and state that revision) or to the head, because I cannot use github commit codes in my workflow. Sorry.

[krosk]

@Roboron, thanks for the kind words.  As of now, i hear no music. The fluidsynth version it is using is also too old I believe, and making the android version use SDL2 is not yet within reach, so switching to a different driver for android would be a good option? I will look into it.

@Andarix, thanks for pointing out the remaining compile error.
@ceeac, thanks for fixing it !
@prissi, I own the mistake and caused additional work for you guys, sorry. The patch I provided that was incomplete. This mistake is pointing out glaring flaws in my setup and development process (manual steps prone to mistakes), so I take measures to streamline this. Future patches will state which revision they apply to.

In any case, it is building now so here you have it, nightly for Android ! Thanks everyone.

[prissi]

I am very grateful for your work. I had Android on my radar for a long time (this is why I even asked Pelya in May again) and I am very happy that this was sucessful.

[Andarix]

please test the apk

https://github.com/Andarix/simutrans/releases

[krosk]

pelya has updated his repository and merged the video glitch patch, so we do not need to patch it ourselves.
Below is a patch applicable for r10031 that removes the android patch on /.github/android folder.

[prissi]

THanks, comitted in r10033

[Andarix]

r10033 not compile

2021-08-20T18:52:54.2602977Z ##[group]Run if [ -f $GITHUB_WORKSPACE/.github/android/*.patch ]; then cp -r $GITHUB_WORKSPACE/.github/android/*.patch .; fi;
2021-08-20T18:52:54.2604154Z if [ -f $GITHUB_WORKSPACE/.github/android/*.patch ]; then cp -r $GITHUB_WORKSPACE/.github/android/*.patch .; fi;
2021-08-20T18:52:54.2605020Z if [ -f *.patch ]; then git apply *.patch; fi;
2021-08-20T18:52:54.2606272Z sed -i "s/^AppVersionCode=[0-9]\+/AppVersionCode=$(svnversion project/jni/application/simutrans/simutrans)/" ./project/jni/application/simutrans/AndroidAppSettings.cfg
2021-08-20T18:52:54.2608436Z sed -i "s/^AppVersionName=\"[0-9]\+.[0-9]\+\"/AppVersionName=\"r$(svnversion project/jni/application/simutrans/simutrans)\"/" ./project/jni/application/simutrans/AndroidAppSettings.cfg
2021-08-20T18:52:54.2653884Z shell: /usr/bin/bash -e {0}
2021-08-20T18:52:54.2654285Z env:
2021-08-20T18:52:54.2655349Z   DEFAULT_PAKSETLINK: https://downloads.sourceforge.net/project/simutrans/pak64/122-0/simupak64-122-0.zip
2021-08-20T18:52:54.2656505Z   DEFAULT_PAKSETFILE: simupak64-122-0.zip
2021-08-20T18:52:54.2657172Z   ANDROID_HOME: /opt/android-sdk-linux
2021-08-20T18:52:54.2657805Z   ANDROID_SDK_HOME: /opt/android-sdk-linux
2021-08-20T18:52:54.2658458Z   ANDROID_SDK_ROOT: /opt/android-sdk-linux
2021-08-20T18:52:54.2659267Z   ANDROID_SDK: /opt/android-sdk-linux
2021-08-20T18:52:54.2659817Z ##[endgroup]
2021-08-20T18:52:54.2780634Z error: patch failed: project/jni/application/simutrans/AndroidAppSettings.cfg:74
2021-08-20T18:52:54.2781920Z error: project/jni/application/simutrans/AndroidAppSettings.cfg: patch does not apply
2021-08-20T18:52:54.2802840Z ##[error]Process completed with exit code 1.

[krosk]

prissi, the patch I provided removes the file in .github/android, but on the SVN trunk / git mirror, the file is still there. Maybe you have to remove it manually?

[prissi]

Yes, I have to. Thank you.

[Andarix]

r10034 works

[Roboron]

The fluidsynth version it is using is also too old I believe, and making the android version use SDL2 is not yet within reach, so switching to a different driver for android would be a good option? I will look into it.

It is just a matter of adding an extra ifdef for Android with the driver. Not sure if the attached patch works, since I can't test it. Please do so.

One note: I see Pelya added previously a soundfont for Android, but I don't think this comes bundled with Android OS. In such a case, the game will inform you that a soundfont is missing (and you'll have to bundle one).

[krosk]

Following @Roboron post, I investigated the issue of music, and manage to enable it for Android. Thanks Roboron for the direction.

This time instead of embedding a patch, I pointed to a fork of pelya repository for simutrans at https://github.com/krosk/commandergenius/commits/sdl_android_prebuilt. The important updates done on that forked repo are:
- embedding a prebuilt fluidsynth 2.2.2 library and dependencies (downloaded from https://github.com/FluidSynth/fluidsynth/releases/tag/v2.2.2)
- changing the location of the downloaded soundfont so it appears in the simutrans options (putting the sf2 file in the /music folder)

The patch below, applicable to r10048:
- change the build pipeline to use the forked repo
- remove the exception case in fluidsynth code (ifndef __ANDROID__) we introduced because the function we excluded is now available with the updated fluidsynth

Upon game start, there will be a warning that no soundfont was found; just go to the options/sound and select the included soundfont from there.


For the curious minds:
- pelya repository includes fluidsynth 1.1.3, a version too old to have any android support (in particular, the oboe driver)
- pelya repository includes libraries by source code to be compiled, but not prebuilt libraries
Compiling an updated fluidsynth source code was too complex due to the 3rd party dependencies (toolchain, libs, etc...). Fortunately, fluidsynth team provides prebuild libraries for Android, that I plugged into pelya repository. I had to replace a few libs too, so the changes in my fork of pelya probably won't go upstream. For now, the android build will depend on my fork.

[prissi]

Thank you very much. Now we are close to a next release with also Android on board!

[Roboron]

- changing the location of the downloaded soundfont so it appears in the simutrans options (putting the sf2 file in the /music folder)
Upon game start, there will be a warning that no soundfont was found; just go to the options/sound and select the included soundfont from there.

To make this more straightforward to the end user, we could split the default soundfonts we search for on paths and soundfonts names. This way we can look for those soundfonts names also on the music directory first (then repeat with system paths), and the user wouldn't need to set it manually if they are bundled with Simutrans instead of installed system-wide. Proposed patch attached.

[Flemmbrav]

Heyo, weird question, maybe I've missed it, but can you tell me where the Pakset is stored and how to swap it?

[krosk]

@Roboron, +1 for your patch.

@Flemmbrav, after install you should have a folder akin to /storage/emulated/0/Android/data/com.simutrans/files/ (the part not in bold will depend on your smartphone). The paks go into that folder. For instance, the default pak64 is Android/data/com.simutrans/files/pak. Upon running the next time, you will be prompted for choosing among the installed paks, like on PC.

EDIT:
@prissi, additional patch, that ties in-game logging to Android logcat. Android redirects stdout and stderr to /dev/null, and introduces different logging functions for logcat, The patch below applicable to r10049 makes use of them. For those who want to capture the logs, add the command line parameters -debug x at the beginning (SDL config), and capture logs via adb logcat -s com.simutrans.

[prissi]

If the Unix command line works in Android, one could install paks with the builtin dialog. To see this dialog, Simutrans nedds to be started without a pak, or if two paks are installed, Googling tells me that this could work, but it may choke on wget or curl trougles or write protected directories or no unzip etc. I will try to test this this evening.

@Flemmbrav
I would very much like to have a dumbed down starter pak like once discussed for steam in the pak192 forum. For small screens like a smartphone pak192 seems very good. pka64 is rather small.

[Flemmbrav]

@prissi agree! I got a bit more than just a bit irl in the past years, but I still do follow that thing. Lucky us, I'm sick these days so I could take a look at that again

[prissi]

Rather get well first ...

[Yona-TYT]

Rather get well first ...

I agree with @Prissi, I hope you recover soon. 

[Roboron]

Friendly reminder that there are two patches waiting on this thread, the log patch from krosk and a fluidsynth patch by myself.

@prissi

[Yona-TYT]

Friendly reminder that there are two patches waiting on this thread, the log patch from krosk and a fluidsynth patch by myself.

@prissi

Well, those of us who have spent a long time in the forum know that our dear @prissi is usually somewhat forgetful hehehe   ... but of course with so many things on his agenda it must be difficult for him to remember everything.

Greetings to all !.