Bay 12 Games Forum
Other Projects => Curses => Topic started by: zaroth on May 09, 2014, 01:10:29 am
-
While in the Github thread (http://www.bay12forums.com/smf/index.php?topic=138426.0) I posted about the more general stuff concerning further LCS development, here I'd like to keep things pretty specific. I'm talking about refactoring the source code.
Me and Lasica, while surfing through LCS code, found some pretty good reasons for some refactoring work to be done:
- all of the function declarations are lumped into HUGE includes.h, making dependencies between files not clear (and being a pain while making changes, having to recompile a lot all the time)
- despite being a really nice fit for an OOP approach, much of this potential is wasted right now, using global variables and huge functions all over the place
- talking about huge functions, there are some offenders which could really use delegating some of their functionality to smaller, specialized ones (my favorite is attack(), which has 1500 lines of code and takes over one third of src/combat/fight.cpp)
- display and game logic are mixed all over the place, which results in a game where it's really hard to make a translation / port to another platform / display form (such as some tiles / SDL windows, possibly)
In order to learn what was done in this direction, we've skimmed the forum, commit logs, code etc. and found what follows:
- dev/ directory in the source code, containing quite a few interesting classes and ideas - need to look into it more
- Jonathan S Fox talking about his adventures with using Boost in LCS (http://www.bay12forums.com/smf/index.php?topic=2398.0)
- Jonathan S Fox talking about why LCS has this non-OOP approach with some history (http://www.bay12forums.com/smf/index.php?topic=2405.0)
- Toady One explains why Wisdom and Heart are the way they are (http://www.bay12forums.com/smf/index.php?topic=2418.msg39438#msg39438) (not code related)
- Grundee asking for help with portability work (http://www.bay12forums.com/smf/index.php?topic=2481.0)
- "Future of the Crime Squad" topic (http://www.bay12forums.com/smf/index.php?topic=2392.0) - it mentions huge refactoring changes were done, so I guess things used to be even worse ;)
- Gatleos quite recently starts thread on future of LCS UI (http://www.bay12forums.com/smf/index.php?topic=136238.0)
- LCS on Android, where a lot of work had to be done on the UI (http://sourceforge.net/p/lcsagame/git/commit_browser) - it would be nice if the author could comment on that, need to look into it more
- Android fork discussion thread (http://www.bay12forums.com/smf/index.php?topic=127236.0)
If you know about other important architecture discussions or documents that I've missed, please link them here, as we certainly do not wish to reinvent the wheel. Of course, it would be best if the developers themselves could comment on these issues (especially Toady One, Jonathan S. Fox, Addie MacGruer, blomkvist, grundee and yetisyny - who seem to be veterans, having fought many battles in code for the liberal cause), but we'll take what we can :) Also, on a bit unrelated note, we haven't looked at the portability / pdcurses etc. part of code, if someone could shed some light on how/why things are done there, it'd be great.
Since refactoring isn't easy work, we'll probably mostly plan it / do it in small steps for now, but here are some possibilities that we are considering:
- splitting the huge functions/files into more easily consumable blocks
- grouping some of the functionality into classes, encapsulate and all the good stuff
- writing some unit tests to make sure nothing breaks during our refactoring and to be useful later on
- abstract the curses calls to allow for a different UI (e.g. SDL) in the future - super long term and work consuming, probably won't be done, but it's nice to dream :)
-
Hi Zaroth
Thanks very much to the invite to the forums. I hadn't realised you guys had paid so much attention to my android port of the game; it's made my weekend and will be raising a glass to you all later.
I'd been going through a quiet period at work -- having just got my first android phone, an S2 -- and was looking for a project to occupy my time. The android store certainly doesn't need another fart app, so was looking for something reasonably chunky and open-source that I could play with. I'm a massive DF fan, and had been looking through the rest of Toady's work. I'd played a few games of Liberal Crime Squad in DosBox, and loved the concept, despite being rubbish at it. (I still am. Bug reports from people who are very much better at it than me and who have found a crash further than I can get still cause me much grief.)
Some of the primary issues encountered when translating anything to Android are:
- Android is Java. Liberal Crime Squad is an exciting mix of old-school C and new-school C++, almost as if it has been written by several different people over the course of many years. As it has.
- Android requires multi-threading, really. The UI thread needs to remain responsive at all times, whereas the logic thread needs to run in the background. This is probably the easiest problem to solve when translating something like DF: set up a 'synchronized' object that the LCS logic thread can wait on whenever it's waiting for the keyboard. Any button that gets displayed on the UI thread can have a character value attached to it as a tag, and whenever the a button gets pressed, send its value to the sync object and wake any threads listening to it.
- The Android on-screen display is, er, interesting. It's not curses, being somewhat more like the display system the Amiga used to have, where you'd send text boxes and UI widgets with layout information attached, to be displayed by the UI manager. It can only be altered by the UI thread, requiring a system of message passing from the logic thread, and the whole thread gets destroyed and recreated whenever you rotate the screen or another app gets time on the front, needing anything that's been displayed on the screen to be cached in some static memory. The preferred method of "displaying" a screen on Android is to 'inflate' it from an XML description. Android LCS has a few 'customised' views for such things as the base screen and the on-site view, where updated text is sent to a textview with an id code, but mostly just some empty scrollviews where texts and buttons are slung in in order.
Being a bit of a porting novice, my translation of LCS to android went as follows:
I wrote a python script that reads a class file, identifies all of the class variables and method names, and spits out a java file with an appropriate class name, instance variables left at the top, and method names with the correct names and all of the code included but commented out, and with a 'throw new UnsupportedOperationException();' at the top.
Starting with the main() function, look through the code a line at a time, and change it to Java. Changing the C code to Java mostly involves removing the comments; int x=3 is the same in both languages, for instance. Changing the C++ idioms (for instance, loops), required a bit of rewriting.
Once I've completed a function or two, run it, and see where it crashes. If it's on a UOE, then great, work on that function next. If not, fix the code and try again. Repeat repeat repeat.
The curses routines are already factorised out in LCS. Writing a few stub functions for java that send it to logfile lets you play through LCS/Android under the debugger, and you can fix up output to the screen using the message passing functions a bit at a time. Repeat till done.
There were two big issues I had with the Android port:
- Performance.
- The PC version saves every day, as you go out to do something Liberal. It takes milliseconds to do a memory dump in C on a PC. It takes quite a noticeable bit of time to do an object tree serialisation in Java. Custom readObject / writeObject routines for the main offenders was in order, which as anyone that does Java knows, means lots of hard-to-find bugs.
- Creating a new location to visit allocates a whole pile of structures (width × length × height), which only takes a second on PC, but introduces quite a noticeable lag on a phone. About 30 seconds on a galaxy S2, not acceptable. I altered that code to make it only allocate a floor at a time, if / when needed.
- Parsing all of the text configuration files. A native version of this code in Android is not fast; string manipulation isn't really one of Java's core strengths anyway, and doing it on a phone takes an age. On the other hand, xml files are compiled into an intermediate binary form when you produce an apk, and these are interpreted extremely fast. A little bit of horrible reflection-based code later, and voila, the game starts in a reasonable amount of time, and is still configurable.
- Stability
- native LCS stores (many of) its list structures as doubly-linked lists. This isn't really a java-native style of list access, so they're mostly changed into ArrayList in this port. However, a very common activity is to iterate over a whole list (eg. all liberals) and remove any that don't meet some condition (eg. are dead). Removing items from a list that you're iterating over is a ConcurrentModificationException on Java: instant crash. This is aggravated by the fact that there's a lot of very intricate calling structures in LCS, and it's not always obvious which paths will lead to a modification downstream.
- NullPointerExceptions. Aargh! Mostly I hadn't appreciated that in C++, allocating a variable name for an object also calls its no-argument constructor. Bit of a shock, and I only really found this out when I was nearly done. It's not how things work in Java. Also, there are many (many) paths through the code, and there's a few sneaky ones where I'd only allocated objects if I thought they'd be needed -- because it's slow, as above -- and boom, out you come in a crash.
However, I did have a lot of fun putting it all together, and am glad that so many people have enjoyed it. I hadn't really been intending to release it at all -- hence the lack of forum posting, or anything similar -- but when it was done I thought it might be a chuckle. Sent Toady an email asking if that would be okay; he said it would, as long as the code stayed freely available, and here we are today. Hope Mr Fox isn't too upset about me playing with his baby; I'd not appreciated the work he'd put into it, not having much experience of forums.
Regarding Zaroth's goals:
splitting the huge functions/files into more easily consumable blocks
grouping some of the functionality into classes, encapsulate and all the good stuff
I can't speak for how easy this is in C / C++ (not my area of expertise), but this is fairly straightforward in Eclipse / Java. I've done quite a lot of splitting things out and rearranging for the Java port, mostly so that I could get my head round it. On the other hand, Java is very easy to refactor (as these things go), and gives you plenty of warnings about things that are inaccessible, uninitialised local variables and so on. Also, plenty of static code analysis tools (FindBugs, Unnecessary Code Detector) to pick out the 'obvious' mistakes. I'd begun commenting on all the functions as well, but it was taking ages, and perfect being the enemy of good enough, I decided to just get it out there.
writing some unit tests to make sure nothing breaks during our refactoring and to be useful later on
I can only suggest that this would be an excellent idea. Assuming of course you understand how the damage calculation code is supposed to work :-)
abstract the curses calls to allow for a different UI (e.g. SDL) in the future - super long term and work consuming, probably won't be done, but it's nice to dream :)
Actually, this is okay, depending on what your goals are. The curses code is already factorised out and in one place. If you're wanting to create an OpenGl port that just draws the characters on-screen where they would be in DOS mode (like, for instance, Dwarf Fortress does) then it shouldn't take more than a few hours. Caveat: says me, that doesn't know a great deal about SDL programming.
Any questions on anything, please feel free to ask, will do my best. Otherwise, cheers everyone, and keep pursuing that Liberal agenda,
Addiexxx
-
Last I saw Fox was thrilled to see LCS expanded onto another platform. LCS is perfect for Android. You can stop at any time and pick up at almost any time. It's largely just a better game than 99% of what's on the google marketplace. I look forward to the next update.
-
Oh, it's great to see this game's not dead. I thought nobody would read my crash reports (which are happening less often now that I avoid crashy things). Really, there's only two big bugs I've noticed: One of my Liberals lost his head and was pretty much fine after a two month clinic visit, and if somehow you send Liberals to a safe house that is not seen by the game as a safe house, you don't see any sieges. 0% heat, and none is generated. There are smaller bugs with stats too, one the health bug where heavily injured Liberals heal and get way too much health (not sure if it affects gameplay, would it change defense or blood levels?) and beating Conservatives raises Wisdom- a lot. (This one has crashed my phone before, but only because I'm curious. I had to let the hostage escape after like 2600 Wisdom.)
-
Hi, I'm blomkvist on Sourceforge and, yes, as you've pointed out, the source code could certainly be improved by any brave enough to endure.
-
Hi I'm yetisyny on SourceForge and also pleased to see these efforts. I hope to work together with you on this. I've made a number of changes to the code on SourceForge recently... I hope we can merge our work together. I like your ideas on making the code more object-oriented and portable and not having ridiculously long functions, and making it easier for it to use something other than curses for display. Curses, and console mode, have plenty of limitations. Also I think the Android port is really awesome, and also quite an effort since it's a different language. I've been working on getting more concise code, among other things... as far as the really long length of includes.h goes, that's both a blessing and a curse... it's kinda nice being able to look up almost any function declaration all in one place, but the file is definitely too long. But if you increase the number of header or code files, you need to add them to the makefiles and the project files for the various IDEs. I've done plenty of code cleanup... before I started on the game, it had a very long list of compiler warnings... I fixed all of that and now it never generates any warnings, that's just 1 example. Recently I changed the compiler options for MinGW-GCC in the Code::Blocks project to use the C++98 standard and be "pedantic", and had to change a bunch of things that were, well, non-standard. This ensures that all the code is now standard code that ought to work on any compiler on any platform as long as it's a C++ compiler. I've also been changing from C-strings to std::strings gradually, since std::strings are safer and don't have the buffer overflow problems and are more object oriented, not based on that old-fashioned pointer/array code. I also hope to change the savegame format to be pure XML that is fully editable in any text editor, eventually. This would have the advantage that small changes to the game that happen periodically would no longer break savegame compatibility, and it would be great for debugging too.