Banana Rogue

[Krice]

Refactoring save and load game routines. Taking a small break from this project did help, the source code doesn't seem that hard to work. The save/load routines are just ancient, but I think easier to refactor than "daemon" function pointers. Even if the old list routines would stay, saving the data in them is just that. A problem I have with this project is that I don't really have a good understanding how everything is stored, but examining the source code will most likely reveal it.

 

[Krice]

I think there is either non-persistent levels (I think Rogue had this, but can't remember) or each level is saved on disk which I doubt. I started the save game routine with first item on the list, traps. They are stored in a fixed global array in level.cpp. Same with rooms, but the room struct has two linked lists. When I began to look at the code for saving and loading linked lists I realized they aren't going to work in C++. The list has char* pointer for data which can be anything in C, but not in C++. It's a lot of work to rewrite all list routines, but I think the game doesn't really have that many lists. Also, I think creating specific container classes for items and creatures will probably make some routines much better than what they are now.

 

[Hag]

I've spend some time adapting C++ code to C lately, so I like your musings on doing the opposite. Learning a few things along the way and giving me insight on how other developers manage their shit. You must really come through some deep "what the hell this guy was thinking about" moments though.

 

[Krice]

This source code is quite nice if you think about how old it is. It's consistent in its own way which I find is a sign of good programming.

 

[Krice]

Slowly moving list routines to container classes. It's not clean, but it should work. There are "only" 74 linked_list references found, but still plenty of work to do. I've chosen the "easy" way to implement list routines which is moving some list operation to a container class with everything it had so for example when monsters look for gold items in a room those items are not cleanly returned from the list, they are processed inside the container class just as they would have been in the procedural implementation.

void Object_List::Run_To_The_Gold(thing *tp, room *trp)
{
   //check the level for gold and see if tp in room trp wants to make a run for it
   for (oitr i = objects.begin() ; i != objects.end() ; ++i)
   {
       object *cur = (*i);
       if ((cur->o_type == GOLD) &&
           (roomin(&cur->o_pos) == trp))
       {
           /* Run to the gold */
           tp->t_dest = &cur->o_pos;
           turn_on(*tp, ISRUN);
           turn_off(*tp, ISDISGUISE);

           /* Make it worth protecting */
           cur->o_count += GOLDCALC + GOLDCALC;
           break;
       }
   }
}

 

[Krice]

I'm wondering what this code is doing. I think it's searching a trap from the array and probably removes it by performing a strange copying in that while loop. 'ntraps' is the number of traps in the level and 'traps' is an array of trap struct. 'ce' is a macro to check if the trap location is same as player's location. This code happens when you read petrify scroll (S_PETRIFY) so it's even more confusing, I guess. Thinking to replace the traps array with a list container for traps, it makes everything easier.

           int i;

           /* Find the right trap */
           for (i=0; i<ntraps && !ce(traps[i].tr_pos, hero); i++);
           ntraps--;

           if (!ce(traps[i].tr_pos, hero))
               msg("What a strange trap!");
           else {
               while (i < ntraps) {
               traps[i] = traps[i + 1];
               i++;
               }
           }

 

[Krice]

Now traps also have their container class. One thing is interesting though, I can't see when a trap is destroyed. It's probably a flag in trap data, but in list it's probably better to remove the trap object completely.

Save and load routines are a big issue, because they use some kind of really weird way to save and restore a "reference" to the object. This is needed in places like rooms which has a list of monsters for reasons I still don't understand. Something to do with lighting the room if there is a monster that can do it. This list of course saves a reference to a monster in the main level list. What I plan to do is save the index to the list, it should work...

Monsters have doorplace and "dest" coordinates which are pointers and when you look at the save routine it's hilarious, because with these it's saving some kind of meta data in x, y of coordinates which then restore coordinates by searching stuff like doors. I think it would be easier to save those coordinates, but before that you need to check if some weird flags are set with them.

With monsters I also have to use dead state rather than removing the monster from the list during a turn. The game itself knows this problem by doing some list magic when actions of monsters are performed, so they knew about it, but didn't know how to handle it properly.

 

[Krice]

It's getting really close to compile, but it's only the beginning... What is still left are couple of annoying list routines. Like when you pick up something the original routine moves items at x, y in the front of the list which is actually a clever trick to get away from creating another list. In fact it should work with std::list, so maybe it's the easiest way to refactor it. The only other problem is the save/load game as usual, mostly those coord pointers I was talking about.

 

[Krice]

I made a manual sorting for pick up routine, hope it works. Another thing I forgot is how to save and load 'daemons' or delayed actions to items and monsters. They do have object handle stored in them so... if nothing else works the game objects need an id which then could be passed to daemons so they know who has the item etc.

 

[Krice]

The structure of the source is now better I think. It's easier to find stuff when you look at a file name. With structs I've noticed that in procedural code it's better leave them simple and not try to make them like classes with lots of member functions. Some things like save/load for struct data works better in the struct itself however. I'm going to leave the save/load problems later and focus on getting this thing to compile. There are two files that need lots of fixes: rip and new_level. Rip handles score which is quite complex, because it's using some kind of network code to read from a network. I think at that time it wasn't internet, because it wasn't yet invented, but a local network of computers. New_level is creating a new level, but it's weirdly moving some stuff from the current level to a new level. This game doesn't have permanent levels so it might be the reason for that.

 

[Krice]

New_level was not that difficult to fix after all. Some monsters are saved to the next level depending on strange 'terrasave' routine, but this happens only in outside levels. I had to write a special routine to save and restore monsters, because afterwards they are located in new rooms and there was not really a way to make difference between saved and new monsters other than to use list copying. I have also worked on rip.cpp and it's possibly also easy, because I can remove network code and find a better way to display scores and add score to the high score list. It's now kind of in the same routine. The age of this game shows in the way you display highscores from console with -s added as an argument. This game needs a title screen anyway so it's going to have a highscore view routine added to main menu.

 

[Krice]

I could make this compile and run, but I want to figure out how to save function pointers and also 'sticks' or magic wands, because they have a special thing going on. Other than save+state only rip unit is unfinished from compile perspective at least, so it's getting close which I have to admit is exciting. I was wondering if it was a good call to replace curses with SDL2, but it's too late now. You could change it back to curses, because I've tried to keep most of the original functions, it just uses SDL2 in the end. Now that I think of it, probably could have keep everything the same and make it possible to switch to curses with #ifdef block...

 

[Krice]

Rip compiles now, but in all honesty I mainly removed a lot of code from it, and replaced an array of highscores with std::list. For some reason you could edit the highscore list if you were a "wizard" and since I didn't get that function I removed it. Also removed all network code, because it wouldn't work in today's environment I guess. The reason such an old game had that is probably the fact that computers had that local network thing before internet was invented, at least in places like schools.

 

[Krice]

The save routine has only function pointers saving missing so I commented them out and tried to compile and run this for the first time. But for some reason Visual Studio doesn't want it to happen. The linker is complaining about missing "fatal()" function, but it's there. And it's only giving this error in one of the files that's using fatal(). I think as a next step I'm creating a GCC version with Code::Blocks to see what happens.

 

[Krice]

It was an actual linker error and because I don't get those often I was confused for a while. The fatal() had const char parameter in declaration but only char in definition. After that it did run and didn't even crash, but the screen was empty (white) which in SDL2 means nothing is output (I had forgot to blit the font data to temporary 32 bit surface before creating a final texture). Then it just began to work. Well, at least to the gameview where I noticed you can't move. The reason for that is probably a global no_command variable which determines if you can do something and it doesn't seem to get out of that loop. Also, as a first bug the game shows the player is a "veteran" which probably isn't true in the beginning of the game.

 

[Russia is over. The end.]

Wanna show a screenshot?

 

[Krice]

It always shows the same thing, so for some reason the randomizer doesn't work. The rng is a bit weird and also I think it could be using some kind of seed generation to keep level structures but I'm not sure about it.

 

[Krice]

After some debugging I found out the game is working properly, when you press some command it's actually called. The problem however is how the game is using four different screens and my guess is that in SDL2 implementation it doesn't work as it should during the gameview screen (which is 'cw' I think). When the game starts you enter the name, select a class and roll values (re-roll actually changes the random seed and level structure!) and this works fine. Also, some routines work, like inventory (although it crashes, because the list routines are buggy). These work, because they are not in the gameloop.

The reason there are four screens is that 'cw' IS the level structure. Monsters (and items I think) save and restore the tile when they move to next location. There is also 'mw' for monsters, 'hw' for help and 'stdscr'. It's quite confusing and overly complex. They didn't yet have a concept of level map or a display map, they used the curses window for storing data also. There are couple of ways to make this work. The first is trying to mimic the curses display and update in SDL2 and the second one is create a level map which is much more work, but would be much better solution that removes a lot of complexity from display routines and elsewhere.

 

[Krice]

The reason however was none of those, but a buffer overflow in the Window class... I did write it fast to attach it to the routines of io.h so it would be ready, but forgot to protect the buffer from invalid index which was bound to happen. When I fixed that it started to work just like that. Of course inventory routines still don't work and there are some other strange things like each room has a dark spot for some reason and I can't see monsters anywhere, at least in the first level. Also, when you pick up an item other than money it doesn't erase it from the map. But it's nice to know it runs and it's now possible to start to fix the game in action.

 

[Krice]

The plan is to replace curses 'screens' with a proper gameview/level array. I've put too much work on this just to leave it like this. The 'mw' alone has over 250 references so it's not going to be easy fix, but once done it's going to be much simpler and better than ascii layers, because you can use object pointers in the level data. Things like terrain doesn't have to share the data array with creatures etc.

 

[Krice]

Working on 'mw' references, still 40 left. The 'mw' window is used when monsters move or they are put on the map. Movement routines have been easy to fix, but there are some things that are harder to implement like "detect monsters" spell which overlays 'mw' to 'cw' or player's window. It's not that difficult to make, but requires a special routine in gameview. When I was fixing these I noticed how ridiculous the room list for "fire" creatures is. It's just way too complicated and prone to bugs. It's used to light the room when some creature has light and those creatures are in a separate list for each room. I think an easier solution for that is to use a value for each tile which tells what room it belongs to. It's maybe more data, because each tile needs that value, but it doesn't matter in modern times when memory limit is not a concern.

 

[Krice]

I feel like this is a beginning of a long rewrite, because layered windows were supposed to work in a specific way when thinking about invisible monsters and also monsters that can go through walls. It's not easy to figure out when invisible monsters should be a part of a routine, but I think it doesn't even matter, because I'm not planning to preserve the original gameplay. I wonder if there is a plugin for Visual Studio to find repeating code, because this has it quite a lot.

 

[vlzvl]

I wonder if there is a plugin for Visual Studio to find repeating code, because this has it quite a lot.

Not a Visual Studio plugin but it supports C++ and its pretty good at detecting duplicated code.
I am using it myself to reduce technical debt on my projects and it found a lot of it:
https://www.harukizaemon.com/simian

 

[Krice]

Well maybe it's too much in a project like this. And searching gives good results, because the duplicate code is quite similar in many places.

 

[Krice]

I've done some rewrite on structs to change them to classes by using member functions. It's much more fun than trying to figure out how those window layers work. There is also a practical reason for this, it's making the source code easier to work on when stuff like item code is restricted in the item class itself and it works through the public interface. And it's actually relatively easy, just find member variables and see where they are used, and move those functions or parts of code as member functions of the class.