Thursday 17 September 2015

Kingdom of Refactoria

So I had an idea to inherit container functionality to GO_Container which confusingly is a game object type (such as boxes you can move), not the container class itself. We have been told that you should use composite type classes which are instances of classes inside another class. However sometimes it creates a number of wrapper functions, especially when you use the composite class from outside the host class. One way to reduce that is expose the composite to outside, so you can use notation class->composite->Do_Something(); but then again, why not use inheritance?

I started by looking at the container class. In Teemu it's called T_Bag. I know, I know... It's an "abstract" class which is just a list of items which then can be accessed, removed and put into the container. I found something fishy about Get_Slot() function that took the item type as parameter. The function itself wasn't a problem but the way it was used in Player class (I think player's class was Player in Teemu, not sure).

There was a variable called 'weapon' in the Player class. I noticed that it was the type of the item and that item was retrieved from the container with Get_Slot(). I think my original reasoning was that items are always stacked so there will always be only one item type in a slot position. But I think it has changed or will change in the near future. It means that when you try to access the weapon item it can be another item with same item type!

I'm embarrassed to mention, but first I made a mistake and changed the 'weapon' to weapon_slot which pointed to a slot in the inventory. It's perfectly valid way to access the inventory and the only way to access items when you select an item from inventory. Then again if you use the slot to store the current weapon it will be a disaster, since you can throw items from the inventory and change the internal slot positions. I wasn't really thinking this, because I only remembered you can't drop items, although it also will change soon.

After realizing this I changed weapon_slot to weapon_item which is simply a handle or pointer to the item in the inventory. The solution was simple in theory, but required changes in Wield_Item and Wear_Item routines among other routines. This time I also wrote routines to handle simple operations with member functions and not just directly using weapon_item variable as I had done with 'weapon'. Even when you are using member variables it's sometimes good to limit the places where the variable is set or changed and "hide" it behind a member function. Not only it's easier to refactor but also less likely to create bugs.

As a refactoring experience this one was actually not that difficult. It seems like I didn't make much progress, but in fact now T_Bag is ready for experimenting as a part of inheritance structure of GO_Container and I have fixed that dubious weapon handling. To make things bit more complex T_Bag will also be a composite class in Player, because the player has two inventories, one for regular items and one for tools, but I think it should work.

No comments: