Android port

[gerstrong]

How does one compile VCMI for Android? I’m willing to tinking with my Nexus 7, but I have never done Android development before.

You need a framework that has some libs compiled, as well as the ndk and sdk of android.

Anyways here some code I found in the sources where the android version crashes: File: client/CMT.cpp at Line 312

 
if (!testFile("DATA/HELP.TXT", "Heroes III data") || !testFile("MODS/VCMI/MOD.JSON", "VCMI mod") || !testFile("DATA/StackQueueBgBig.PCX", "VCMI data")) 

I only have that StackQueueBgBig.PCX File in mods/vcmi/data not under data directly. I can remove that file but I guess something else will fail. Should that work? Is that normal? It says “VCMI Data” I think the mod directory is merged, but somehow this seems to be wrong. I there another version of the StackQueueBgBig.PCX file or of other resources?

[gerstrong]

Btw. are data paths case sensitive? How important is that…?

[Ivan]

Placing that file in mods/vcmi/data should work - VCMI should load that directory as part of “VCMI mod”.

Perhaps VCMI does not loads this mod? Mod will be loaded if:

• file Mods//mod.json is present. And code checks for presence of this file just before stack queue image test.
• mod was not explicitly disabled in ~/.vcmi/config/modSettings.json. By default mods are enabled so this shouldn’t happen normally.

You may try to copy all files from mods/vcmi to root directory but fact that vcmi fails to find this file as part of mod may indicate another problem.

I there another version of the StackQueueBgBig.PCX file or of other resources?

Only one - archive core.zip on our server:
download.vcmi.eu/core.zip

Btw. are data paths case sensitive? How important is that…?

Paths from VCMIDirs are case-sensitive but virtual file system generated by vcmi is not.

BTW - one more thing that you should check - file access permissions. VCMI must be able to list files in all directories (+x flag on all directories). Normally it does but I remember receiving weird reports due to this.

[gerstrong]

Well, I did an reupload to my galaxy S1 and the problem went away, now I’m stuck at this line when I try to start a scenario:

CServerHandler::CServerHandler(bool runServer /*= false*/)
{
	serverThread = nullptr;
	shared = nullptr;
	port = boost::lexical_cast<std::string>(settings"server"]"port"].Float());
	verbose = true;

	boost::interprocess::shared_memory_object::remove("vcmi_memory"); //if the application has previously crashed, the memory may not have been removed. to avoid problems - try to destroy it
	try
	{
		shared = new SharedMem();
    } HANDLE_EXCEPTIONC(logNetwork->errorStream() << "Cannot open interprocess memory: ";)
}

At least, start screen works and game can chosen. I’m going to look deeper into that…

The exception it throws is:

"Cannot open interprocess memory: "

[Ivan]

AFAIK this is one of Android-specific problems - lack of proper support of shared memory. Check original port to see how pelya handled this situation.

[gerstrong]

Yeah I know, we write very often. I have his patches and he did it through just creating files.

I will take a deeper look on it. I also found some issues in the code. Is there a way to contribute through patches or svn commits?

I know some ways to make it more portable and a bit easier to handle, also for better performance. I hope nobody is against it. I want to help a bit…

[Ivan]

Is there a way to contribute through patches or svn commits?

Of course. Submit some patches to show you code and if they’re OK you can ask Tow for commit access to svn.

[stopiccot]

I’ll remind that really good solution for that is still making a single process version of vcmi. If on android you can make some hacks to get multiprocess version working on iOS i’ll never achieve it. Also SDL2 adoption would be very good for mobile version. So I encourage you not to waste your time on hacks but make something good from long-term perspective.

[Ivan]

I’ll remind that really good solution for that is still making a single process version of vcmi.

Decisions like this are up to Tow. Besides - even if answer will be positive - who will implement this? Such changes in architecture are far from trivial.

Also SDL2 adoption would be very good for mobile version.

Yeah I know. We may get some improvements for desktop version too. But for now we need to keep 1.2 supported at least till release of next Ubuntu LTS (April 2014). Once it will be out I’ll start checking other major Linux distros and once all of them will have SDL2 we can adopt it.

But if SDL2 can be used without dropping 1.2 support - then I’m OK with that.

[gerstrong]

I’ll remind that really good solution for that is still making a single process version of vcmi.

Decisions like this are up to Tow. Besides - even if answer will be positive - who will implement this? Such changes in architecture are far from trivial.

Also SDL2 adoption would be very good for mobile version.

Yeah I know. We may get some improvements for desktop version too. But for now we need to keep 1.2 supported at least till release of next Ubuntu LTS (April 2014). Once it will be out I’ll start checking other major Linux distros and once all of them will have SDL2 we can adopt it.

But if SDL2 can be used without dropping 1.2 support - then I’m OK with that.

Well, I have one question here and might send a patch if it is okay:

line 68 lib/filesystem/CInputStream.h

	std::pair<std::unique_ptr<ui8]>, size_t> readAll()
	{
		std::unique_ptr<ui8]> data(new ui8[getSize()]);

		size_t readSize = read(data.get(), getSize());
		assert(readSize == getSize());

		return std::make_pair(std::move(data), getSize());
	}

wouldn’t it easier to understand if you use a std::vector instead of that construct? Shared pointers and especially unique_ptr are a nice feature in boost and C++11 but I think for these types of data not really required at that point. A vector or STL array will do just fine if not better, and you get a simpler and cleaner code.

I observed that Pelya replaced that because I guess he got alignment issues on Android. He has a huge patch but I hopefully won’t need it. As I read from the fans here, the code got much better since 0.86. I haven’t compared it.

I also saw some passed std::strings through parameters which could be references. For that type of data I always would use references. It makes the code faster and less memory consuming.

BTW. if you don’t know I’m the main developer of Commander Genius:

[Commander Genius/URL]

It is a clone for the Commander Keen games and I can tell you a lot about shared_ptrs and so on, we also use them there at some places where it really helps. We also have successfully ported CG to Android, you also find it on the Google Play store.

Getting back to the topic… Let me know if it is okay to change that part and adapt to the code. I would test it and send you the patch so you don’t need to do anything on it. I assure you it will be better for your engine.

Getting to SDL2. I ported CG to get working on SDL2 and I can tell you that about 45% of SDL functions needed to be adapted in our case. It now works with both versions of 1.2 and 2.0 but I control some routines through preprocessor macros and wrappers so keep that in mind if you try to do those changes. It might take a bit of time, but it’s worth. I really like the update. Ubuntu 13.10 also has SDL2 in the apt sources, but it’s also optional and I think it can be installed aside, I hope so. SDL2 does not talk SDL 1.2.

What the SDL guys changed on the other side is pretty cool. A lot of stuff is easier to get done and better to understand, finally you have a correct way to accelerate your graphics without using OpenGL. You can still use it though.

On Android it runs pretty well through OGLes2 as well. I have ported some of the functions to Pelyas libSDL repo as well.

I know, there is another official SDL2 implementation for Android on which we also base our stuff but SDL2 for android doesn’t bring you boost, ffmpeg and all that stuff we already have. Also the Java he wrote like the automatic downloader for resources or the touch overlay are missing in the official implementation. Well, that’s is because it is SDL only while Pelyas repo is a lot more than the name says.

The port for Pelyas libSDL regarding version 2.0 is not done, but when I get time I will work on that again, so, well, we might port VCMI as one of the first games to it as well if you want, but first let me make that engine solid for SDL 1.2 and get some of the memory issue we still encounter on Android fixed at first glance.

And yeah about the shared memory system, we another project where we got a better release of boost::interprocess but we have to put more effort on that one.

I’m thinking about replacing this by a file,where client and server have access, not sure. This change of course would be android specific only and I would hold that patch in our repo or use the other lib…

What do you think? Hope I don’t bug too much. As tole before I like your project and I’m willing to help.

CG can rest a bit, it is pretty much done…](http://http://clonekeenplus.sourceforge.net/)

[Ivan]

wouldn’t it easier to understand if you use a std::vector instead of that construct? Shared pointers and especially unique_ptr are a nice feature in boost and C++11 but I think for these types of data not really required at that point. A vector or STL array will do just fine if not better, and you get a simpler and cleaner code.

This is pretty much leftover from old API - before we had method with signature like this:
char * openFile(std::string name, size_t * size);

I’m not opposed replacing it with std::vector but there is one difference between them: you can’t release data from vector. You need to either copy it (possible speed issues) or swap into another vector (you’ll have to replace destination storage with vector).

BTW - regarding alignment issues - some players managed to install VCMI on Android using “Linux deploy” tool which allows running Linux distributions on Android in chroot. As result - I got stacktraces for several bugs caused by alignment. All of them should be fixed now.

I also saw some passed std::strings through parameters which could be references. For that type of data I always would use references. It makes the code faster and less memory consuming.

I don’t think that there will be any change in speed - most of our strings are really small. I think any situations where (according to profiler) copying was actually slow are already fixed.

If you really want to fix this - then go ahead. Sounds OK for me.

I really like the update. Ubuntu 13.10 also has SDL2 in the apt sources, but it’s also optional and I think it can be installed aside, I hope so.

Yeah - SDL2 can be installed along with 1.2. Reasons why I waiting for Ubuntu LTS are:

• fixed release data as opposed to rolling release distros. I don’t need to constantly check SDL2 status.
• according to ppa statistics around half of players use last LTS (12.04) - they still don’t have SDL2.

If you or someone else want SDL2 now then I’m OK with that as long as backwards compatibility remains. Othervice - we will switch to SDL2 once Linux distributions will be ready.

One SDL2 issue that needs investigation - CMake support. Right now CMake comes without FindSDL2 modules. We’ll need to either wait till CMake fixes this or provide our own modules.

[gerstrong]

wouldn’t it easier to understand if you use a std::vector instead of that construct? Shared pointers and especially unique_ptr are a nice feature in boost and C++11 but I think for these types of data not really required at that point. A vector or STL array will do just fine if not better, and you get a simpler and cleaner code.

This is pretty much leftover from old API - before we had method with signature like this:
char * openFile(std::string name, size_t * size);

I’m not opposed replacing it with std::vector but there is one difference between them: you can’t release data from vector. You need to either copy it (possible speed issues) or swap into another vector (you’ll have to replace destination storage with vector).

BTW - regarding alignment issues - some players managed to install VCMI on Android using “Linux deploy” tool which allows running Linux distributions on Android in chroot. As result - I got stacktraces for several bugs caused by alignment. All of them should be fixed now.

I also saw some passed std::strings through parameters which could be references. For that type of data I always would use references. It makes the code faster and less memory consuming.

I don’t think that there will be any change in speed - most of our strings are really small. I think any situations where (according to profiler) copying was actually slow are already fixed.

If you really want to fix this - then go ahead. Sounds OK for me.

I really like the update. Ubuntu 13.10 also has SDL2 in the apt sources, but it’s also optional and I think it can be installed aside, I hope so.

Yeah - SDL2 can be installed along with 1.2. Reasons why I waiting for Ubuntu LTS are:

• fixed release data as opposed to rolling release distros. I don’t need to constantly check SDL2 status.
• according to ppa statistics around half of players use last LTS (12.04) - they still don’t have SDL2.

If you or someone else want SDL2 now then I’m OK with that as long as backwards compatibility remains. Othervice - we will switch to SDL2 once Linux distributions will be ready.

One SDL2 issue that needs investigation - CMake support. Right now CMake comes without FindSDL2 modules. We’ll need to either wait till CMake fixes this or provide our own modules.

Sounds good to me. I will take a look on that interprocess stuff, maybe it’s the only stuff that needs a workaround. We’ll see.

FindSDL2. I have a module for cmake. Don’t where it came, but I can give it to you. SDL2 support in CG is optional as well and I also will wait so more distro get it as default…

[beegee]

I also saw some passed std::strings through parameters which could be references. For that type of data I always would use references. It makes the code faster and less memory consuming.

Use const& if you need to read from the string only, pass per value if you need a copy of the string and then use the move-ctor if needed.

[Tow]

Hi gerstrong, it’s really great to see the Android port development resurrected.

Is there a way to contribute through patches or svn commits?

Of course, we will happily accept your contributions.
At first, please post a patches here (or by bugtracker, if you prefer). After a few of them are committed, I’ll give you a direct SVN access. (send me PM if I don’t get back to it)

I’ll remind that really good solution for that is still making a single process version of vcmi. If on android you can make some hacks to get multiprocess version working on iOS i’ll never achieve it.

Single-process version of VCMI won’t happen in predictable future (if ever).

The reason for the current model was mostly to prepare ground for network MP game. Since the server-client architecture is fast enough, it works for single-player mode good enough. That way we save on code and its maintenance. Plus the client-server mode is really well tested and GUI/game logic code separation is strictly enforced.

Making a single-process VCMI is possible (well, once VCMI was a single-process app), however this would require much work (both client and server parts assume they have their own copies of VCMI_lib). There are many missing features and bugs that need our attention much more.

As for the inteprocess communication issue, I don’t think it justifies such rewrite. Basically the whole boost::interprocess thing is only for server to notify the client that it is ready to accept connection.
It can be substitued on Android with something simpler (server creating a file) or even ripped out altogether. Client could just attempt connecting until it succeeds. Ugly but should work as a workaround. [Or client, after making all the preparations, could spawn server and then wait for server to initiate connection. Though this would make the start time significantly longer.]

What is exactly the issue with iOS you are referring to? If it allows a process to spawn another process and use TCP sockets, it will suffice.

I also saw some passed std::strings through parameters which could be references. For that type of data I always would use references. It makes the code faster and less memory consuming.

Right, strings that are read-only should be passed as a const reference. The speed difference is negligible (rendering eats a big majority of our CPU time) and we were likely too lazy to type the whole type.
Feel free to correct this.

wouldn’t it easier to understand if you use a std::vector instead of that construct? Shared pointers and especially unique_ptr are a nice feature in boost and C++11 but I think for these types of data not really required at that point. A vector or STL array will do just fine if not better, and you get a simpler and cleaner code.

I’ll disagree on this.
I believe typical use-case for this function is to get the memory and use it to construct some other structure (sound chunk, image, and so on). These structures often use raw memory (void*, size) to be constructed from.
With vector we would end up building such pair from it anyway — it would just bring us more boilerplate (moving memory from vector is not its typical use-case).

pair<unique_ptr<ui8]>, size_t> is IMHO both descriptive and clean enough as a interface. Well, if anything, it could be wrapped into some kind of structure, I don’t like pairs. (first/second member names don’t tell anything in places of usage). AFAIR there were also memory buffer wrappers in Boost.Asio but that doesn’t sound easier.

I can tell you a lot about shared_ptrs and so on, we also use them there at some places where it really helps.

We know about shared_ptr. And many other C++11 features. There are many, many places where they could be applied for a cleaner code. VCMI was initiated in 2005 when there was no C++11 (and I was still a C++ newbie) and that’s why many things look as they look. Our codebase is continually getting better, yet there still remains much to be done. Any help is welcomed.

[stopiccot]

Single-process version of VCMI won’t happen in predictable future (if ever).

I do not understand why it couldn’t be rewritten using multiple threads instead of multiple processes. For example quake3 - an example of heavily client-server oriented game. They have only one binary (standalone server binary needed for dedicated servers is optional). And there is no benefits from multiprocess model for us - if server crashes, there is no point of client still working. It’s not a Chrome with process-per-tab approach/

[gerstrong]

The reason for the current model was mostly to prepare ground for network MP game. Since the server-client architecture is fast enough, it works for single-player mode good enough. That way we save on code and its maintenance. Plus the client-server mode is really well tested and GUI/game logic code separation is strictly enforced.

Making a single-process VCMI is possible (well, once VCMI was a single-process app), however this would require much work (both client and server parts assume they have their own copies of VCMI_lib). There are many missing features and bugs that need our attention much more.

As for the inteprocess communication issue, I don’t think it justifies such rewrite. Basically the whole boost::interprocess thing is only for server to notify the client that it is ready to accept connection.
It can be substitued on Android with something simpler (server creating a file) or even ripped out altogether. Client could just attempt connecting until it succeeds. Ugly but should work as a workaround. [Or client, after making all the preparations, could spawn server and then wait for server to initiate connection. Though this would make the start time significantly longer.]

wouldn’t it easier to understand if you use a std::vector instead of that construct? Shared pointers and especially unique_ptr are a nice feature in boost and C++11 but I think for these types of data not really required at that point. A vector or STL array will do just fine if not better, and you get a simpler and cleaner code.

I’ll disagree on this.
I believe typical use-case for this function is to get the memory and use it to construct some other structure (sound chunk, image, and so on). These structures often use raw memory (void*, size) to be constructed from.
With vector we would end up building such pair from it anyway — it would just bring us more boilerplate (moving memory from vector is not its typical use-case).

pair<unique_ptr<ui8]>, size_t> is IMHO both descriptive and clean enough as a interface. Well, if anything, it could be wrapped into some kind of structure, I don’t like pairs. (first/second member names don’t tell anything in places of usage). AFAIR there were also memory buffer wrappers in Boost.Asio but that doesn’t sound easier.

I can tell you a lot about shared_ptrs and so on, we also use them there at some places where it really helps.

We know about shared_ptr. And many other C++11 features. There are many, many places where they could be applied for a cleaner code. VCMI was initiated in 2005 when there was no C++11 (and I was still a C++ newbie) and that’s why many things look as they look. Our codebase is continually getting better, yet there still remains much to be done. Any help is welcomed.

Well, I all understand this and respect it as well. I would have used singleton at this place, but well I think it is more about everone’s style.

So far I have fused the interprocess stuff through a file as suggested, but now I get other crashes.

My priority is to get it running and look later for improvements. Since I’m a bit busy right now here, I think this makes the most sense, hence I’ll send you the patches later.

The issues I now run into is that vcmi_server doesn’t want to be executed at all. Log file is opened but stays empty. I need to take a deeper look into this. I’ll do that later. Just be patient. I’m getting through

[gerstrong]

Hi guys,

my apologies for being away for a while, but I have other stuff to get done. I managed to get part of VCMI ported to android and working, but there needs to be done more.

I’ll be back in a while… be patient. I try my best

[noname]

gerstrong,
you really porting VCMI for android?
I am glad that this still working

sorry for my english

[anon]

Hi guys,

my apologies for being away for a while, but I have other stuff to get done. I managed to get part of VCMI ported to android and working, but there needs to be done more.

I’ll be back in a while… be patient. I try my best

Please provide your patches for VCMI, I’m sure they’ll be helpful to others.

[Random_dude]

I love to read updates on the develpoment of vcmi on android
Keep us in touch with your work.Post something like a screenshoot or a video to make us happy.I would love to see your work so far.Dont let us down man. With respect an anon.