Test

Test post to see if everything works as expected.

Fix fix fix

Another week pass, with a lot of breaking and fixing. The good part is, I am really getting used to this.

The first one was a obvious mistake, I forgot to increase the iterator I use. When you try to exit, the removeAllScripts function is called, and it tries to free the first element a second time.

Second one was a save load problem. I was using iterators, and checking if the iterator equals to end(), but loading creates and adds a new node in every loop, so it never ends. Since the number of elements is written in the save file, and read before the loop, I just changed the check in the for body.

I was having random segmentation faults that I did not get what triggers them. While testing the save-load, I realise that they are happening a lot more now. So I checked if save load has something to do with it. The problem was, currentScriptPtr was not null, but it should be. There are two assignments to it in the engine. One before starting a script, that assigns it the address of the script to run, and runs it in a while, then at the end of the while, another assignment that makes currentScriptPtr's value NULL. How a variable, that only function that assigns a value to it also assigns null to it before ending, can have non null value? The answer is loading. If you load a save before the running script finishes, the pointer is not null, and that causes a segmentation fault (or worse). Just added another assignment to be sure the value is null.

The last problem was another segmentation fault, that happens at start of the game. I was trying to move persoTable to vars.h/cpp when this problem emerged, so I checked if it is a initialization problem, and found out, even after assigning null to all elements, the first element is not null, but 0xFFFF0000. Then I debugged with gdb, and watched the address. Then I realise, before I modified it, there was always a first element in the list, but now, there might not be, and since I did not add a check for that, a assignment to first element was overwriting to the persoTable.

As you can see, there were a lot of problems, but I do not feel down when I found an error any more. This is a part of the job I guess, nothing to worry about.

The First Bug

I have worked on ctp.h/cpp this week. Player walk logic is in it. There were too many global variables used, so I decided to form them into classes. I have created two classes, one for walkbox and one for routes. I manage to move most of the globals, and move to Scripts.h/cpp unaware of the lurking problems.

When I was testing the changes in the script files, I found some inventory items causes segmentation faults. The problem was that I used a pointer as parameter, and it was passed as value, not referance. It was fairly easy to fix since I can pinpoint which commit broke it, and see the difference I made. But while I test it, I forgot to take an item from the first room, and when I came back, I found out, the character can not move.

First find the commit that broke it, that is the easiest part. Okey, I found the commit that the problem emerged, but the changes are small and simple. I could not found anything that may cause that problem, or any problem. So I start to watch the values that assigned, but the did not mean anything to me. Then I though to compare them with the real ones, and that give me some bizarre differences.

The first loop was fine, but in the second one, one value was wrong. than it gets completely different. The weird part was that the loop counts was different too, that was my clue. There was a assignment out of index in a matrix. That was not a problem before since the assigned value will be overwritten in next loop, but since I removed the matrix and moved the variable in to a class, where the assignment actually write the value is unpredictable, and might broke all the walking logic. I talked with my mentor about this, and consulted dreammaster too( thank you again). I have added a little code for not changing the behaviour, but there is room for improvement.

This was the first bug that I did not cause, and I am happy to be able to work on it. But I have to say, knowing how c++ works and being able to think like it is two very different things.

Using Common::

This was a smooth week for me. I had no big glitches, no unpretictable segmantation faults etc. I worked on different parts, but the most important class I created is the Stack class.

There was a implementation of stack in stack.h/cpp, but fingolfin leave a TODO comment that suggest replacing with Common::Stack, so I did that. I had seen code parts using Common but this was the first time I did. The stack was used for both int16 and void*. There is a union of those two, and a enum for type, capsulated as stackElementStruck, so it uses spesific pop and push functions. first I made it a class, and added constructors for both possible data type, then created a new Stack class, inherited Common:Stack, and added pop & push methods to it. It is pretty straight forward, but it feels better integrated.

I am thinking about using Common::List for some of the parts I already worked on, but it has to wait after midterms, which is this week.

if you can not see anything, take a step back

This week was a little sad for me. I have started working to make use of Cell lists head, but in the end, I have to backup, and postpone it until GSoC midterms.

I started with modifying the add and remove. But of course it did not work out as expected. I had a lot of faulty assumptions. First of all, add does not always adds to end of the list, and it was obvious actually. I knew Remove does not have to remove from last, what I did not realise was, removing head is not an option. After every modification another problem emerged, and of course a lot of segmantation faults. After days of working, I manage to fix nearly all of the problems, except unable to save or load, and an odd rendering issue. Some of the objects were rendering in wrong order. I tried everything I can think of on sorting, but I could not found the origin of the problem, or anything wrong in the code. On that point I feel I do not have a choice, but surrender.

I may be lost some days, but I start to get how the authors of Cruise were thinking while coding it. There were things that I found strange, like a pointer named plLast, that keeps first node for sorting, but after working on same code parts so long, I get why it was named like that.

I have committed the changes to a new branch, so I can resume working on it anytime I want, and I hope working on other parts will give me a clue about how to fix the issues..

Best Way Of Exception Handling: SegFault

This week I worked on some list strucks. First one was backgroundIncrustStruct, that holds previous and next pointers, and the variables that composes backgroundincrust. I thought having the list and the class together complicates code too much, so I decided to split it in two. I had some concerns but everything worked out, and I decided to go on, trust the original engine developers.

After trying to do the same to cellStruct, I had a segmentation fault. Splitting a Struct this way creates a huge diff since you have to edit every line that has any use of it to get it to compile, so I had no idea what got wrong.After reading the code several times did not help, I started with running with debug on, but it gave me nothing. Then I tried gdb, to debug the code, and it showed me a line of if statement (if (si->_cell->type == -1)). At first, it seemed odd, since it did not assign anything, but after giving some though, I realise that there was a CellListNode that do not has a Cell, so any read attempt to its members causes segmentation fault. I have go over and over on the code but addNode function creates and assigns Cells for every node. I added some debug messages,that confirmed every addNode run assigning a cell. I read backtraces, that did not help a bit. The cause was so obvious, it was too hard to see.

The only node that is not created by addNode is Head. In both lists (backgroundIncrust and Cell), head created manually, and used for previous and next pointers only so it do not have a object assigned to it. In backgroundIncrustList, methods directly pass head and start working with next node, which was my concern at the first step, but in CellList, methods does not pass head, although it never passes conditional statements that causing an empty first round in the loop. Since I split the CellStruct, the conditional statement tries to read a value from a null pointer, that causing segmentation fault at start of the first loop.

I can fix this with a few lines, just assign an empty Cell to head, but changing this behaviour, and making use of the head nodes is a better solution, so instead of adding and empty object just to prevent segmentation faults, I will alter the method to add and use actual objects to head.

In this few days a lot has happened in ScummVM front. Tiqpit, the other GSoC student has stopped working on is project, and also ScummVM's co leader fingolfin has retired. I wish both of them good luck in their lives..

generally, you push and pull, but sometimes, git fights back!

This was my last week of finals, so I expected to speed up, but things does not always work out as you expect, are they?

The problem started when I need to merge the upstream(main ScummVM repository) changes. Updating the master branch was pretty easy since I did not do any changes, only a fast-forward was enough but when it come to rebasing my working branch, things get complicated.

I did not realize that until my mentor Johannes Schickel warned me about it. There was a merge commit that merges my working branch with itself. It took some time before I find out what got wrong, and it was indeed the rebase. I am still not sure how it happened, but when I rebased, git didn't remove the old commits, yet still added new rebased commits to history. There was double commits that were identical, except parent commits, and git merged them on the branch they are.

I used interactive rebase and manually removed old commits, also used a force push to fix history on github. I know force push is unpleasant but in the end, everything sorted out as they should.