Jump to content
  • Advertisement
Sign in to follow this  
Nexusfactor

When one object can be used multiple ways

This topic is 436 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I've asked this question 3 different times to varying results, and I'm not entirely sure why. I'll present my problem, solution, and hopefully someone can explain if it's or isn't code smell. 

First

Second

Third

I have the following Character Interface:

interface Character{
  void attack(Weapon toAttackwith, Enemy enemyObj);
  void reload(Reloadable weapon, Bullet bullets);
  void drinkHealthPotion(Potion health);
  void drinkManaPotion(Potion mana);
  void spendMoney(Money toSpend);
}
interface Reloadable {
    void reloadWeapon(Bullet bulletstoReload);
}

interface Weapon {
    void attack();
} 
public class ReloadableWeapon implements Weapon, Reloadable {
    @Override
    //logic to fire bullet
    public class attack();
    
    @Override
    //logic to reload Gun
    public class reload(Bullet bullet);
}
public class MainCharacter implements Character{
    //left out other method implementations
    void attack(Weapon to attackWith, Enemy enemy){
    }
    
    void Reload(ReloadableWeapon weapon, Bullet bullets){
        //call weapon.reloadWeapon(bullets);
    }
}

A character can do a lot of things. Here is why I placed Attack/Reload in the character interface:

A character can:

  • attack an enemy object with a Weapon.
  • reload a weapon with bullets.

To me it makes sense to place those two methods in the character interface, pass the required parameters, and call the necessary methods to complete the operations.

The first link suggests that I use the Observable pattern to listen for when the reload method has been called. To me this seems complex and unnecessary.  The suggested implementation just uses the weapon to attack. The provided code works, but seems like a lot for one action. For example, In Gun you have fire, but in MeleeWeapon you have hit, both are just attacks.

The second link, one of the commenters agreed that it should be part of the character, however another suggests a separate implementation for actions.

The third, considers it code smell, because I would have to keep updating to to accommodate any actions a character can take with an Object.

However, the problem I see is, regardless of where I place my actions, as I'm creating my game, I'll have to keep updating my interface/whatever container hold my actions. Now I understand, some game objects can only be used in one way, so rather than have a method called spend money I can have a method that takes the objects abstract type(in my game, GameItem), and call the use method, for example, spendMoney could be:

void useGameItem(GameItem item);

Drink potion can't be condensed into one drink method as both alter a character state in different ways(one will affect player health, the other the mana).

Some times an Object can be used in more than one way, for example my Gun. At this point you will have to determine who/what is using the object, and how.

For me placing attack/reload in the character interface makes sense, it's an action the character can do given certain situations/parameters.

If it's too much to read, my main idea is this. My character could use a GameItem, but certain GameItems could be used in more than one way. How can I handle this?

Can some explain why this is code smell/not ideal?

Share this post


Link to post
Share on other sites
Advertisement
All interface members should be part of a useful abstraction.
Is this:
character.attack(weapon, enemy)
Any different from this:
weapon.attack(enemy)
?
If not, then the character.attack function is useless and should be deleted.

I would imagine that part of the character abstraction though would be to hide the weapon interface from the character's user, so it would look more like:
character.attack(enemy) // this part of the code doesn't even know what a weapon is.

As well as interfaces providing meaningful abstraction that enforce invariants and present a simplified viee of the internal workings, they should also strive to reduce the dependencies between each software component.

As for your drinkHealth/ManaPotion functions - these seem inside out. If the character has a drink action with many polymorphic behaviours, those behaviours should be hidden away behind the abstract 'drink' interface function.

Maybe, for example, health/mana attributes range from 0-100, but can temporarily pushed up to 200 though they'll slowly self drain back down to 100.
You could implement both as some kind of CharacterAttribute class, with min/max/uberMax/selfDrainRate members, and Add/Subtract/Set/Update(deltaTime) functions.

A mana potion acting upon a character could call character.mana.Add(amount). The player just has to update its health/mana members each frame to implement the 200->100 "overcharge"/self-discharge feature. The Character class doesn't need to know about potions whatsoever, or how the mechanics of its health/mana attributes actually work.

Share this post


Link to post
Share on other sites

I would argue that the fundamental problem you have here is that you want to give your game entities an arbitrary number of mix-and-match units of functionality, which you implement with interfaces - but interfaces are a compile-time concept, designed to help you bind code together, not a run-time concept to help you examine objects for their capabilities. Therefore the place to mark a weapon as reloadable is not in the class definition - as they do not vary at run time - but in the object data somehow, including the potential choice of subclass.

In the weapon case, this could simply be done via changing the interface:

public interface Weapon {
    void attack();
    bool needsReloading();
    void replenish(); // does nothing if needsReloading returns false
}

But this does not necessarily extend to your general case, where you want individual items to be able to support a whole bunch of potential behaviours. The traditional way to do this would need to have all the various methods in the base GameItem object, and each subclass would implement them accordingly, including functions similar to needsReloading() to show the presence or absence of capabilities. It's a bit unwieldy, but it's been used in many shipping games.

A more adaptable approach comes in the form of components, where the presence or absence of a component tells you whether a certain interface is adhered to. For example, to represent a health potion, your GameItem might contain a CarryableComponent and a DrinkableComponent. A fountain might contain just the DrinkableComponent. A gun contains a AttackComponent, a ReloadableComponent, and a CarryableComponent; a turret only contains the first two, and an energy weapon or sword might not have the ReloadableComponent.

The downsides of components is that they can complicate the code - you need to check for component existence at run time (do you have something like 'GetComponent' or do you try to structure code so that you can be sure about which objects contain which components), you need to consider how to divide up the functionality (where does the ammo count live? which component checks for it?). Worse still, there are a lot of different philosophies on how components should be implemented, many coming from people who are concerned primarily about performance rather than ease of use.

In your case, I would recommend the chapter on components over at Game Programming Patterns.The subtitle there is "Allow a single entity to span multiple domains without coupling the domains to each other.", which is pretty much exactly what you want, and it doesn't get bogged down in some of the esoteric stuff that other component-aficionados sometimes get into.

Share this post


Link to post
Share on other sites

As for your drinkHealth/ManaPotion functions - these seem inside out. If the character has a drink action with many polymorphic behaviours, those behaviours should be hidden away behind the abstract 'drink' interface function.

Maybe, for example, health/mana attributes range from 0-100, but can temporarily pushed up to 200 though they'll slowly self drain back down to 100.
You could implement both as some kind of CharacterAttribute class, with min/max/uberMax/selfDrainRate members, and Add/Subtract/Set/Update(deltaTime) functions.

A mana potion acting upon a character could call character.mana.Add(amount). The player just has to update its health/mana members each frame to implement the 200->100 "overcharge"/self-discharge feature. The Character class doesn't need to know about potions whatsoever, or how the mechanics of its health/mana attributes actually work.

Bare with me here, cause I'm still learning :D

The problem I see is, the potions are updating two different attributes of the Character. If I hide the drinking of potions behind one method, how would I determine what attribute the potion was meant for? Health or Mana. Let's keep it simple, and say Health and Mana are int variables. How would I write my function to determine which attribute to update based on Potion, and then update it?

I would imagine that part of the character abstraction though would be to hide the weapon interface from the character's user, so it would look more like:
character.attack(enemy) // this part of the code doesn't even know what a weapon is.

Shouldn't the character be aware of what weapon it's using? and who it's using it on?

Edited by Nexusfactor

Share this post


Link to post
Share on other sites

In your case, I would recommend the chapter on components over at Game Programming Patterns.The subtitle there is "Allow a single entity to span multiple domains without coupling the domains to each other.", which is pretty much exactly what you want, and it doesn't get bogged down in some of the esoteric stuff that other component-aficionados sometimes get into.

If I understand the pattern correctly, it's composition? i.e Like Composition over Inheritance? Only in this case, instead of turning a "is-a" relationship into a "has-a", they are breaking down into smaller manageable pieces. it seems they are delegating certain tasks to other components.

I placed the reloadable in it's own interface because, not every Weapon is reloadable, for example Swords.

Share this post


Link to post
Share on other sites

Yes, it is composition. And I already explained why implementing reloadability via interfaces is arguably the wrong approach - it's not (practically) queryable at runtime.

Edit: let me try and word this another way. The purpose of an interface, in a typical programming language, is to say: "In this part of the program code, you are only allowed to use objects that conform to this interface". It ensures that objects are wired together, at compile time, with a predictable set of functionality. A function that requires an object implementing the Comparable interface always has, at compile time, the compareTo method. This is all decided before the program is ever run, to help you build maintainable software.

What you're looking for is a system that, at run time, can examine 2 existing objects and see what functionality they provide. That is a property of the objects, not of their classes. It is possible to return object properties via methods hard-coded into the classes - e.g. different return values from `needsReloading` depending on the object type - but that means all these objects must implement `needsReloading` which means they share the same (or at least one) interface in code. The interface(s) can do the work of telling you about the objects, but not the presence or absence of those interfaces.

Edited by Kylotan

Share this post


Link to post
Share on other sites

Yes, it is composition. And I already explained why implementing reloadability via interfaces is arguably the wrong approach - it's not (practically) queryable at runtime.

Let's say if in my PC Game, I hit the key to reload. On the technical side, what's happening?  I check if the Weapon contains a reloadable component? and if so, pass the reloadable, let the reloadable component to do it's job, and if not, pass null? isn't that bad practice?

Take for example a Sword class:

public class Sword implements Weapon {
    private int damage;
    //what should I return here, given that Sword isn't reloadable.
    private Reloadable reload;
}
Edited by Nexusfactor

Share this post


Link to post
Share on other sites

I updated my post above, so take a look at that. But no, I wouldn't implement Sword like that. Please re-read my first post, and especially the article that I linked as it would explain that (a) there would be no Sword class, and (b) there would be no Reloadable in that object as the whole point is that the absence of the component means it doesn't support whatever that component does. (You'd have to check for that either in the general Weapon class, or in the Player class when it uses a weapon, etc.)

Share this post


Link to post
Share on other sites

I hit the key to reload. On the technical side, what's happening?  I check if the Weapon contains a reloadable component? and if so, pass the reloadable, let the reloadable component to do it's job, and if not, pass null? isn't that bad practice?
Take for example a Sword class:

public class Sword implements Weapon {
    private int damage;
    //what should I return here, given that Sword isn't reloadable.
    private Reloadable reload;
}

All three (now all four) of your questions describe the same problem. This is yet another example of the problem.

Interfaces describe how a thing is to work, everything must be completely interchangeable. It IS-A thing. In all ways it is interchangeable with any other thing.  For example, a 13/16 spark plug must be interchangeable with all other 13/16 spark plugs, even though they may vary in their implementation details. Some may be iridium, some may have forked tips, but all of them can be used in an engine that accepts 13/16 spark plugs.

You need to be careful about building your interfaces.  Whatever it implements must be COMPLETELY replaceable with the parent.  If you have an interface that says an object has a function, perhaps a function named Reload(), then the function must behave interchangeably with all other objects based on the interface.

Even if the behavior is to do nothing and return true, the behavior must exist and must follow the same rules as its siblings.

Share this post


Link to post
Share on other sites

[Interfaces describe how a thing is to work, everything must be completely interchangeable. It IS-A thing. In all ways it is interchangeable with any other thing. For example, a 13/16 spark plug must be interchangeable with all other 13/16 spark plugs, even though they may vary in their implementation details.

This is a summary of the L in SOLID, the 5 core rules of OO.
Upon first reading these don't really give any practical advise as to how to fix your code... But it's worth keeping them in the back of your mind at all times so that eventually with experience you'll come to grok them.

If I hide the drinking of potions behind one method, how would I determine what attribute the potion was meant for? Health or Mana. Let's keep it simple, and say Health and Mana are int variables. How would I write my function to determine which attribute to update based on Potion, and then update it?

There's lots of ways. Perhaps Potions don't need to act upon the Character interface at all; instead they could act upon an Attributecollection interface which the Character implements. This is the D in SOLID - remove dependencies between components by creating a common interface instead.
The potion could have a name (enum, int, string, etc) specifying which attribute it wants to act on. It could ask the AttributeCollection to retrieve an attribute with that name, or null if it doesn't exist,and then the potion can then apply a change to that attribute. This would also allow potions to be used on any other game object that implements the AttributeCollection interface and contains a "mana" attribute.

Shouldn't the character be aware of what weapon it's using? and who it's using it on?

In your example, this is true but it's also true that the code that is controlling the character is also aware of these details... Which is a hint that the character abstraction is pretty weak.
If the character is fully aware of its weapon and its enemy, then the controlling code could just write:
character.attack(); // it knows which weapon and who

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!