1
\$\begingroup\$

Hey there my fellow programmers. Looking for a little input into a potentially smelly code problem (I was about to post this on the Code Review SE, let me know if that's the more proper place).

After looking at a few of the answers here on setting up an rpg like inventory and using a component based system, I've come up with a format to creating items, here are a couple of examples.

Entity potion = new Entity("Potion", new Item(100,50), new Healing(75,0));
Entity elixer = new Entity("Elixer", new Item(50, 25), new TempStatIncrease(0,5,...));

Here are the classes involved.

Entity Class
IComponent[] components
string name

IComponent Interface
ComponentType componentType

Item Class : IComponent
int purchasePrice
int resaleValue

TempStatIncrease Class : IComponent
int strength;
int agility;
...

Healing Class : IComponent
int hpAmount;
int mpAmount;
Heal(Character character);

Now where I run into a bit of design trouble is how to handle interacting with these items. If I have "Potion" selected, and hit the use key in my inventory, it brings up the character list for me to select a character. When I select a character this is the code that would run:

Character Class
UseItem(Entity entity)
{
    foreach(IComponent component in entity.components)
    {
      switch(component.Type)
      {
         case Healing: { ((Healing)component).Heal(this); } break;
         case TempStatIncrease: { ((TempStatIncrease)component).Increase(this); } break;
         case ... //This could get ugly quick.
      }
    }
}

I feel like the above approach is prone to disaster. It's a switch for one, I have to modify this every time I want a character to be able to use a new component. Two, I have to do a not-so-nice cast to the concrete component. Add to the fact I have to create a new ComponentType enumeration for each new component, another piece of maintenance.

Any thoughts on how I might be able to re-factor my implementation to remove the potentially disastrous switch statement and that complex component casting?

\$\endgroup\$
  • \$\begingroup\$ Are you planning to have consumable/usable effects that aren't simple scalar adjustments? Healing looks like a permanent "currentHealth" stat increase, to me. So from what you describe .Heal() and .Increase() are both just .Adjust(character,stat,value,duration), thus collapsing your Component classes into one. I doubt that's what you have in mind, but without a sense of other expected effects I'm not sure we can actually suggest complexity-/maintenance-alleviating refactors. \$\endgroup\$ – Chris Mills-Price Oct 8 '14 at 23:20
  • \$\begingroup\$ There aren't any complex item uses right now. Just standard RPG type items. Healing potions of various strengths, items that give temporary/per meant stat increases, ailment relief, etc. \$\endgroup\$ – steve Oct 9 '14 at 2:32
  • \$\begingroup\$ If you had the effect implementation in systems instead of the components themselves this wouldnt be an issue. \$\endgroup\$ – user40079 Oct 9 '14 at 6:46
2
\$\begingroup\$

If Component had a method ActOn(Character), you could simply call component.ActOn(this). Each subclass could implement appropriately. For example:

Healing.ActOn(Character character) {
    Heal(character);
}
\$\endgroup\$
  • \$\begingroup\$ Hey Carl, thanks for the response! I should have mentioned that I had thought about that exact method, but I didn't go that route because I figure not all components will have to interact with the characters. I would be forced, via the interface, to add that method into classes that would never use it. \$\endgroup\$ – steve Oct 8 '14 at 23:09
  • \$\begingroup\$ Then create a sub-interface, CharacterInteractingComponent, which adds that method, and let the relevant classes extend that interface instead. Or an unrelated interface, CharacterInteracter, and let the relevant classes implement both interfaces. In either case, though, you'll need a type check. Probably simpler to let the non-interacters just no-op. \$\endgroup\$ – Carl Manaster Oct 8 '14 at 23:13
  • \$\begingroup\$ I didn't realize I could create sub interfaces. I may go that route and see how well it works. What's the No-op you mention about being simpler? \$\endgroup\$ – steve Oct 8 '14 at 23:53
  • \$\begingroup\$ Just, for any Component which doesn't need to interact with the Character, implement ActOn() to do nothing. \$\endgroup\$ – Carl Manaster Oct 8 '14 at 23:54
  • \$\begingroup\$ Rather than trying to mold a specific solution around a problem it might be better to focus on the problem itself. Not everything in your game has to follow and fit in the same architecture, if it makes sense do it. \$\endgroup\$ – user40079 Oct 9 '14 at 6:35
1
\$\begingroup\$

There are often lots of items in a game that can be used in varying ways. One generic way to do this is to implement an interface such as the following:

 class IUsable {
 public:
   virtual void Use() { /* does nothing */ }
 }

For items which are usable on the current player such as potions, their Use method send an event to the current player with the heal amount or power amount to be gained followed by a signal to remove the entity (used item) from the player's inventory.

For items that are usable only on targets, the Use method might obtain the current target of the owner and if a target could be obtained and is valid, then uses the item on that target; such as a costume or perhaps an aura buffer, etc.

The same concept could be used for IEquippable, IToggleable, and so forth where the former is for putting on a piece of armor and the latter might be for opening a treasure chest, door, or some other object that essentially has a true/false, on/off, 0/1 type concept. These interfaces can be linked to a component and system or scripts as needed depending on your design.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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