Take the 2-minute tour ×
Game Development Stack Exchange is a question and answer site for professional and independent game developers. It's 100% free, no registration required.

I'm trying to think of the best way to handle player inventory following an object oriented approach.

For example, sword and axe are two different classes, both inheriting from weapon. Weapon and potion are also different classes both inheriting from item.

Ideally, the inventory would be implemented as a container of items, but that can't be done for several reasons. So it has to be a container of item (smart) pointers. Since I don't want to liter my code with calls to new and delete, I'll wrap it in an ItemWrapper class which news/deletes on ctor/dtor.

So the code will look something like this

class Sword;
class ItemWrapper;

//Prepare the long sword.
const Sword longSword(...params...);

//Declare the inv
vector<ItemWrapper> inv;

 //Add it to inventory
inv.push_back(longSword);  //compiles if ItemWraper(const Sword&) is defined.

This approach was the best I could come up with in terms of code clarity and ease of writing. The problem is that ItemWrapper will need one constructor for each leaf class that inherits from Item (which is a lot).

This is my first attempt at this, so I'd like to know: am I going the right way? or if there's a better way of doing it (perhaps one that avoids so many constructors)?

share|improve this question
    
I'm really interested in how this is best implemented. –  Tili Mar 6 '12 at 17:03

2 Answers 2

up vote 9 down vote accepted

There is something you are doing right now that you may find that you regret later, even though it seems like a logical thing to do.

You are likely to find that making each type of item its own class hard to manage once you have more than about twelve of them.

My own inventory management systems don't have different types for items. Instead, I use a "Descriptor" class, which has a set of properties (Name-Value pairs, stored in an associative container [in C++ you would likely use std::map])

The property getter is a template function that casts the property from the map into the appropriate type.

The way I differentiate between item types is to have an ItemType property, usually some sort of enum with entries like WEAPON, ARMOR, etc. Other properties contain damage ranges, references to images to use to render it, name description, etc.

Now, these descriptors aren't for item instances. They describe the item type as a whole. Item instances contains some sort of reference to the descriptor as well as a similar map of properties for values needed for an item instance (like individual durability, for example). In this way, one avoids repeating the same information over and over again. After all, one sword is about the same as another sword, it just might be closer to wearing out.

As far as avoiding constructor calls, I recommend a factory method.

share|improve this answer
4  
I think you might be a bit further than I would. I think there's no reason to make a VorpalBladeOfPower class and a PunyIronHandaxe class; but having separate types for weapons, armor, and so forth might save you grief. Let the compiler prevent a player from dual-wielding a health potion and a helm. –  dhasenan Mar 6 '12 at 22:29
    
I understand your point on consolidating items into one class. I don't really get why store the properties inside an associative container. If I understand correctly (and do tell if I got it wrong), instead of a float weight variable inside the Item class you keep the number in a container (perhaps associated to label "weight"). It seems you take a tiny performance hit (reading from the container) and write more code, in exchange for a bit of memory. –  Bruce Connor Mar 6 '12 at 23:37
    
I am assuming textures and sounds are already memory-managed, of course (as they should always be). In that case, I believe the advantage of memory-managing a few floats, enums, and ints per Item instance (even if there are hundreds of Item instances) wouldn't be very significant. –  Bruce Connor Mar 6 '12 at 23:39
    
It would be a performance hit if you had thousands of properties in each descriptor. Generally, there are less than 20 properties in most of these. –  PlayDeezGames Mar 7 '12 at 0:52
    
Sorry, the "performance hit" wasn't the focus, I know it's tiny or even nonexistent. My point was that it seems like you're writing more code and saving 1 or 2MB of memory (assuming hundreds of items). –  Bruce Connor Mar 7 '12 at 14:07

For example, sword and axe are two different classes, both inheriting from weapon. Weapon and potion are also different classes both inheriting from item.

I wouldn't do things this way; a sword and an axe differ mostly in their data, not their fundamental behaviors as weapons. Likewise with items. I would avoid this overuse of inheritance, starting by restricting yourself to just Weapon and Item (you could even fold these into a single class, but that's a different discussion). If nothing else this will alleviate the constructor explosion you're seeing in ItemWrapper.

So it has to be a container of item (smart) pointers. Since I don't want to liter my code with calls to new and delete, I'll wrap it in an ItemWrapper class which news/deletes on ctor/dtor.

Consider the Boost smart pointers instead of rolling your own. They're fairly isolated from the rest of Boost (so you don't have a ton of dependencies to pull in if, for some reason, you don't want to take advantage of the rest of Boost's neat stuff), and many of them have been adopted to be part of the new standard library in C++11, so you shouldn't have issues upgrading.

This is my first attempt at this, so I'd like to know: am I going the right way? or if there's a better way of doing it (perhaps one that avoids so many constructors)?

As above, I would strive to avoid inheritance for this, and consider using established third-party solutions instead of rolling your own. Those steps alone should remove much of the maintenance complexity you're discovering (or will shortly discover).

That said, another approach is to look at it this way: ItemWrapper is a band-aid for an implementation detail problem. It itself doesn't represent a real "object" in any typical sense. So think about what might be a better analogy that also solves your problem of having a place to put the automatic new and delete calls.

Most inventories allow (some) items to stack, do they not?

The "stackability" of an item is a property of that item, but the item itself should not be responsible for maintaining the stack. If you have a class representing an ItemStack with methods to query the size of the stack, split the stack, et cetera then you can put your management of the lifetime of the item pointer (which, I should note, may become slightly more complicated with stacks, thus providing another argument to use something like Boost) in the stack class along with the implementation of the rest of its behavior. Items that cannot stack are represented by a stack that always contains a single item, and your inventory itself still deals with homogeneous instances of ItemStack.

share|improve this answer
    
+1 Combining Weapon and Item would be the last nail in the coffin for ItemWrapper. Weapon could extend Item or they could both extend a common base class. Then you could just have something like vector<shared_ptr<Item> > inv; –  Gary Buyn Mar 6 '12 at 21:00
    
Using an ItemStack does seem like a good idea. –  Bruce Connor Mar 6 '12 at 23:20

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.