4
\$\begingroup\$

When a LivingBeing attacks another LivingBeing, an attack roll is made to determine if a hit occurs. If a hit occurs, a damage roll is made to determine the amount of damage.

The task is to find a design so what the attack roll and damage can be adjusted no matter how many exterior objects affect them and no matter how bizarre their corresponding rules are.

Here are some example exterior objects to work with to test the design:

  • MagicalSword (derived from DamageInflictor): Increases the attack roll by 2, but only against a Troll. If the original roll is 20 (i.e. the maximum roll), then add 15 extra acid damage (normally a sword does slashing damage, not acid damage).

  • MagicalArmor<LeatherArmor> (derived from Armor) : Lowers the attack roll by 2 against sword attacks (against other weapons, nothing special happens). Reduces the damage by 10 if the damage is acid damage (against other types of damage, nothing special happens).

  • SwordSpell (derived from Spell): Increases the attack roll by 3 if the weapon is a sword, by 4 if the user is a Fighter, and by 5 if the user is a Fighter and his race is Elf. There will be extra fire damage of 10 if the original roll is 20 and the target is a Troll.

  • ArmorSpell (derived from Spell): Lowers the attack roll by 4 if the being under the spell is a Troll. Lowers damage by half if it is fire damage.

  • In the test client code, we shall have a Fighter (of race Elf), using a MagicalSword and under the SwordSpell, hit a Troll that is wearing a MagicalArmor<LeatherArmor> and is under the ArmorSpell, and all the above rules must be followed without using any if-statements in the client code. All methods are to be called from the base classes only so as to be usable for all other situations.

The test client code I'm using is:

LivingBeing *fighter = new Fighter ("Fighter", new Elf),  *troll = new Troll ("Troll");

fighter->setWeaponInHand (new MagicalSword);
fighter->isUnderSpell (new SwordSpell);
troll->setArmor (new MagicalArmor<LeatherArmor>);
troll->isUnderSpell (new ArmorSpell);

fighter->attacks(troll);  // And that's it.  No if-statements in client code.

My idea is to have a class handle the interactions between these objects so that they need not know about each other. I'm calling this class AttackMediator, though I'm not sure if this is truly the mediator pattern or not.

class AttackMediator {
    public:
        struct Data {
            LivingBeing *attacker, *target;
            DamageInflictor* damageInflictor;
            std::reference_wrapper<int> value;
            int originalRoll;
            DamageType damageType;
            Data (LivingBeing* a, LivingBeing* t, DamageInflictor* d, int& v) :
                attacker(a), target(t), damageInflictor(d), value(v), originalRoll(v) {}        
        };
    private:
        Data data;
    public:
        AttackMediator (LivingBeing* attacker, LivingBeing* target, DamageInflictor* damageInflictor, int& value) :
            data (Data(attacker, target, damageInflictor, value)) {}
        void setValue (int& v) {data.value = v;}
        inline bool announceHit();
        inline void determineSpecialDamageChange();
};

Attacking then invokes an AttackMediator to handle all the necessary iteractions:

inline void LivingBeing::attacks (LivingBeing* target) {
    std::cout << name << " attacks " << target->getName() << ".\n";
    int roll = std::rand() % 20 + 1;  // Not const because it may be changed.
    AttackMediator mediator (this, target, getDamageInflictor(), roll);  
    if (!mediator.announceHit())
        return;  // Since the result is a miss.
    std::cout << name << " hits " << target->getName() << "!\n";
    int damage = getDamageInflictor()->getDamage();  // Not const because it may be changed.
    mediator.setValue(damage);
    mediator.determineSpecialDamageChange();
    target->losesHitPoints(damage);
}

where we have:

inline bool AttackMediator::announceHit() {
    data.damageInflictor->adjustAttackRoll(data);
    for (const Spell* spell : data.attacker->getSpellsInEffect())
        spell->adjustAttackRoll(data);
    for (const Ring* ring : data.attacker->getRingsWorn())
        ring->adjustAttackRoll(data);  // etc... for other objects that can increase data.value.
    if (data.target->isWearingArmor())
        data.target->getArmor()->adjustAttackRoll(data);
    for (const Spell* spell : data.target->getSpellsInEffect())
        spell->adjustAttackRoll(data);
    for (const Ring* ring : data.target->getRingsWorn())
        ring->adjustAttackRoll(data);  // etc... for other objects that can decrease data.value.
    return data.value >= data.target->getArmorClass();
}

and

inline void AttackMediator::determineSpecialDamageChange() {
    data.damageInflictor->specialDamageAdjustment(data);
    for (const Spell* spell : data.attacker->getSpellsInEffect())
        spell->specialDamageAdjustment(data);
    if (data.target->isWearingArmor())  // Minor flaw.  This is the second isWearingArmor() call, when it should only be checked once.
        data.target->getArmor()->specialDamageAdjustment(data);
    for (const Spell* spell : data.target->getSpellsInEffect())
        spell->specialDamageAdjustment(data);
}

All the appropriate virtual overrides of the various objects involved are then defined to satisfy the rules mentioned. For example,

// Increases attack roll by 3 if weapon is a sword, by 4 if user is a Fighter, by 5 if user is a Fighter and his race is Elf.
inline void SwordSpell::adjustAttackRoll (AttackMediator::Data& data) const {  
    if (data.damageInflictor->isSword()) {
        data.value += 3;
        if (data.attacker->isFighter()) {
            data.value += 1;
            if (data.attacker->isElf()) {
                data.value += 1;
                std::cout << "attack roll increased by 5 because damageInflictor is a Sword and attacker is a Fighter and of race Elf.\n";
            }
            else
                std::cout << "attack roll increased by 4 because damageInflictor is a Sword and attacker is a Fighter.\n";
        }
        else
            std::cout << "attack roll increased by 3 because damageInflictor is a Sword.\n";
    }
}

// Reduces damage by 10 if the damage is acid damage.
template <typename T>
inline void MagicalArmor<T>::specialDamageAdjustment (AttackMediator::Data& data) {
    if (data.damageType == Acid) {
        std::cout << "Total acid damage reduced by 10 because of MagicalArmor<T> worn.\n";
        data.value -= 10;
    }
}

The output I get when running the test client code is:

Fighter attacks Troll.

AttackMediator::announceHit() called.
data.originalRoll = 20
data.value = 20
attack roll increased by 2 because target is a Troll.
data.value = 22
attack roll increased by 5 because damageInflictor is a Sword and attacker is a Fighter and of race Elf.
data.value = 27
attack roll decreased by 2 because damageInflictor is a Sword.
data.value = 25
attack roll decreased by 4 because the target is a Troll.
data.value = 21
Fighter hits Troll!

AttackMediator::determineSpecialDamageChange() called.
data.originalRoll = 20
data.value = 8
15 hit points extra (acid) damage because the original roll is 20.
data.value = 23
10 hit points extra (fire) damage because the original roll is 20.
data.value = 33
Despite the MagicalArmor<T> worn, no damage reduction because damage is not acid.  [BUG!  15 hit points is acid damage.]
Total fire damage reduced by half because of ArmorSpell in effect [BUG!  Only 10 hit points, not 33, is fire damage.]
data.value = 16

Troll loses 16 hit points.

You can tell by the comments in my output that there still needs to be some tweaking the get the damaging correct, but that simply involves changing the data members of AttackMediator. I just wonder if this design is good or not before I try to perfect it.

One flaw I can see right away is to 99% of all DamageInflictor, Armor, Spell, Ring derived types will do absolutely nothing in the virtual methods adjustAttackRoll and specialDamageAdjustment, and so calling these virtual methods will do absolutely nothing 99% of the time. But 1% of the time, they will do something vital under special circumstances, as shown in the client code here. I don't see how to avoid this because it cannot be known ahead of time which objects will have an effect and which ones won't based on upcoming circumstances (and most won't).

Another flaw is that the system currently does not allow an automatic miss or an automatic hit, or cancelling out the total damage completely after a hit. These outcomes should be possible by special spells or whatever under special circumstances. So far, all AttackMediator is doing is adjusting the attack roll and the damage values based on current situations and whatever objects are in the picture, although I think I could add the above extra features in AttackMediator once I'm convinced that this is the design to keep.

In case you want to read it or run it, here is my entire code so far:

#include <iostream>
#include <string>
#include <list>
#include <functional> 

#define show(variable) std::cout << #variable << " = " << variable << std::endl;

class LivingBeingProxy;  class DamageInflictor;

enum DamageType {Slashing, Fire, Acid};

class AttackMediator {
    public:
        struct Data {
            LivingBeingProxy *attacker, *target;
            DamageInflictor* damageInflictor;
            std::reference_wrapper<int> value;
            int originalRoll;
            DamageType damageType;
            Data (LivingBeingProxy* a, LivingBeingProxy* t, DamageInflictor* d, int& v) :
                attacker(a), target(t), damageInflictor(d), value(v), originalRoll(v) {}        
        };
    private:
        Data data;
    public:
        AttackMediator (LivingBeingProxy* attacker, LivingBeingProxy* target, DamageInflictor* damageInflictor, int& value) :
            data (Data(attacker, target, damageInflictor, value)) {}
        void setValue (int& v) {data.value = v;}
        inline bool announceHit();
        inline void determineSpecialDamageChange();
};

class DamageInflictor {
    private:
        int damage;
        DamageType damageType;
    public:
        DamageInflictor (int d, DamageType dt) : damage(d), damageType(dt) {}
        int getDamage() const {return damage;}
        DamageType getDamageType() const {return damageType;}
        virtual void adjustAttackRoll (AttackMediator::Data&) const {}  // Only special magical weapons will override this.
        virtual void specialDamageAdjustment (AttackMediator::Data&) {}  // Only special magical weapons will override this.  Not const because perhaps the hit may affect the weapon itself too.
        virtual bool isSword() const {return false;}
};

class Weapon : public DamageInflictor {
    protected:
        using DamageInflictor::DamageInflictor;
};

class Sword : public Weapon {
    public:
        Sword() : Weapon (8, Slashing) {}
    private:
        virtual bool isSword() const override {return true;}
};

class MagicalSword : public Sword {  // Increases attack roll by 2 against Trolls.  If originalRoll is 20, then add 15 extra acid damage.
    virtual inline void adjustAttackRoll (AttackMediator::Data&) const override;
    virtual inline void specialDamageAdjustment (AttackMediator::Data&) override;
};

class Armor {
    public:
        virtual void adjustAttackRoll (AttackMediator::Data&) const {}  // Only special magical armor will override this.
        virtual void specialDamageAdjustment (AttackMediator::Data&) {}  // Only special magical armor will override this.  Not const because perhaps the hit may affect the weapon itself too.
};

class LeatherArmor : public Armor {};
template <typename T> class MagicalArmor : public T {  // Lowers attack roll by 2 against sword attacks.  Reduces damage by 10 if the damage is acid damage.
    virtual inline void adjustAttackRoll (AttackMediator::Data&) const override;
    virtual inline void specialDamageAdjustment (AttackMediator::Data&) override;
};

class Spell {
    public:
        virtual void adjustAttackRoll (AttackMediator::Data&) const {}  // Only special spells will override this.
        virtual void specialDamageAdjustment (AttackMediator::Data&) const {}  // Only special spells will override this.  
};

class SwordSpell : public Spell {  // Increases attack roll by 3 if weapon is a sword, by 4 if user is a Fighter, by 5 if user is a Fighter and his race is Elf.  Extra fire damage of 10 if the original roll is 20 and the target is a Troll.
    virtual inline void adjustAttackRoll (AttackMediator::Data&) const override;
    virtual inline void specialDamageAdjustment (AttackMediator::Data&) const;
};

class ArmorSpell : public Spell {  // Lowers attack roll by 4 if the being under the spell is a Troll.  Lowers damage by half if it is fire damage.
    virtual inline void adjustAttackRoll (AttackMediator::Data&) const override;
    virtual inline void specialDamageAdjustment (AttackMediator::Data&) const;
};

class Race {
    public:
        virtual bool isElf() const {return false;}
};

class Elf : public Race {
    private:
        virtual bool isElf() const override {return true;}
};

class LivingBeing {
    private:
        std::string name;
        LivingBeingProxy* proxy;
        Race* race;
        Weapon* weaponInHand;
        Armor* armor;
        std::list<Spell*> spellsInEffect;
    public:
        LivingBeing() = default;
        LivingBeing (const std::string& n, Race* r) : name(n), race(r) {}
        virtual ~LivingBeing() {std::cout << "LivingBeing destructor called." << std::endl;}
        virtual std::string getName() const {return name;}
        LivingBeingProxy* getProxy() const {return proxy;}
        virtual DamageInflictor* getDamageInflictor() const {return weaponInHand;}
        void setWeaponInHand (Weapon* w) {weaponInHand = w;}
        virtual Armor* getArmor() const {return armor;}
        void setArmor (Armor* a) {armor = a;}
        virtual const std::list<Spell*>& getSpellsInEffect() const {return spellsInEffect;}
        void isUnderSpell (Spell* spell) {spellsInEffect.emplace_back(spell);}
        inline void attacks (LivingBeingProxy*);
        virtual void losesHitPoints (int damage) {std::cout << '\n' << name << " loses " << damage << " hit points.\n";}
        virtual bool isWearingArmor() const {return armor != nullptr;}
        virtual bool isFighter() const {return false;}
        virtual bool isElf() const {return race->isElf();}
        virtual bool isTroll() const {return false;}
    protected:
        void createProxy (LivingBeing*);
};

class LivingBeingProxy : public LivingBeing {
    private:
        LivingBeing* actual;
    public:
        LivingBeingProxy (LivingBeing* being) : actual(being) {}
        LivingBeing* getActual() const {return actual;}
        virtual std::string getName() const override {return actual->getName();}
        virtual DamageInflictor* getDamageInflictor() const override {return actual->getDamageInflictor();}
        virtual Armor* getArmor() const override {return actual->getArmor();}
        virtual const std::list<Spell*>& getSpellsInEffect() const override {return actual->getSpellsInEffect();}
        virtual void losesHitPoints (int damage) override {return actual->losesHitPoints (damage);}
        virtual bool isWearingArmor() const override {return actual->isWearingArmor();}
        virtual bool isFighter() const override {return actual->isFighter();}
        virtual bool isElf() const override {return actual->isElf();}
        virtual bool isTroll() const override {return actual->isTroll();}
};

void LivingBeing::createProxy (LivingBeing* being) {
    proxy = new LivingBeingProxy(being);
}

class Fighter : public LivingBeing {
    public:
        Fighter (const std::string& name, Race* race) : LivingBeing(name, race) {initialize();}
        void initialize() {createProxy(this);}
    private:
        virtual bool isFighter() const override {return true;}
};

class Monster : public LivingBeing {
    public:
        using LivingBeing::LivingBeing;
};

class Troll : public Monster {
    private:
        DamageInflictor* claws = new DamageInflictor (10, Slashing);
    public:
        Troll (const std::string& name) : Monster(name, nullptr) {initialize();}
        void initialize() {createProxy(this);}
    private:
        virtual DamageInflictor* getDamageInflictor() const override {return claws;}
        virtual bool isTroll() const override {return true;}
};

inline void LivingBeing::attacks (LivingBeingProxy* target) {
    std::cout << name << " attacks " << target->getName() << ".\n";
    int roll = 20;  // Random, but let's assume it is 20.  Not const because it may be changed.
    AttackMediator mediator (getProxy(), target, getDamageInflictor(), roll);  
    if (!mediator.announceHit())
        return;  // Since the result is a miss.
    show(roll)  ////
    std::cout << name << " hits " << target->getName() << "!\n";
    int damage = getDamageInflictor()->getDamage();  // Not const because it may be changed.
    mediator.setValue(damage);
    mediator.determineSpecialDamageChange();
    target->losesHitPoints(damage);
}

inline bool AttackMediator::announceHit() {
    std::cout << "\nAttackMediator::announceHit() called.\n";
    show(data.originalRoll)  show(data.value)  /////
    data.damageInflictor->adjustAttackRoll(data);
    show(data.value)  /////
    for (const Spell* spell : data.attacker->getSpellsInEffect())
        spell->adjustAttackRoll(data);
    show(data.value)  /////
//  for (const Ring* ring : data.attacker->getRingsWorn())
//      ring->adjustAttackRoll(data);  // etc... for other objects that can increase data.value.
    // For automatic miss, simply return false (setting data.value = 0 is not good enough since data.value can change here still and eventually lead to a hit anyway).
    // For automatic hit, return true immediately (need to give an example though).
    if (data.target->isWearingArmor())
        data.target->getArmor()->adjustAttackRoll(data);
    show(data.value)  /////
    for (const Spell* spell : data.target->getSpellsInEffect())
        spell->adjustAttackRoll(data);
    show(data.value)  /////
//  for (const Ring* ring : data.target->getRingsWorn())
//      ring->adjustAttackRoll(data);  // etc... for other objects that can decrease data.value.
    return true;  // For now, assume a hit is made (false is returned if data.value < data.target->getArmorClass()).
}

inline void AttackMediator::determineSpecialDamageChange() {
    std::cout << "\nAttackMediator::determineSpecialDamageChange() called.\n";
    show(data.originalRoll)  show(data.value)  /////
    data.damageInflictor->specialDamageAdjustment(data);
    show(data.value)  /////
    for (const Spell* spell : data.attacker->getSpellsInEffect())
        spell->specialDamageAdjustment(data);
    show(data.value)  /////
    if (data.target->isWearingArmor())  // Minor flaw.  This is the second isWearingArmor() call, when it should only be checked once.
        data.target->getArmor()->specialDamageAdjustment(data);
    for (const Spell* spell : data.target->getSpellsInEffect())
        spell->specialDamageAdjustment(data);
    show(data.value)  /////     
}

// Increases attack roll by 2 against Trolls.
inline void MagicalSword::adjustAttackRoll (AttackMediator::Data& data) const {  
    if (data.target->isTroll()) {
        std::cout << "attack roll increased by 2 because target is a Troll.\n";
        data.value += 2;
    }
}

// If original roll is 20, then add 15 extra acid damage.
inline void MagicalSword::specialDamageAdjustment (AttackMediator::Data& data) {
    if (data.originalRoll == 20) {
        std::cout << "15 hit points extra (acid) damage because the original roll is 20.\n";
        data.value += 15;
        data.damageType = Acid;
    }
}

// Increases attack roll by 3 if weapon is a sword, by 4 if user is a Fighter, by 5 if user is a Fighter and his race is Elf.
inline void SwordSpell::adjustAttackRoll (AttackMediator::Data& data) const {  
    if (data.damageInflictor->isSword()) {
        data.value += 3;
        if (data.attacker->isFighter()) {
            data.value += 1;
            if (data.attacker->isElf()) {
                data.value += 1;
                std::cout << "attack roll increased by 5 because damageInflictor is a Sword and attacker is a Fighter and of race Elf.\n";
            }
            else
                std::cout << "attack roll increased by 4 because damageInflictor is a Sword and attacker is a Fighter.\n";
        }
        else
            std::cout << "attack roll increased by 3 because damageInflictor is a Sword.\n";
    }
}

// Extra fire damage of 10 if the original roll is 20 and the target is a Troll.
inline void SwordSpell::specialDamageAdjustment (AttackMediator::Data& data) const {
    if (data.originalRoll == 20 && data.target->isTroll()) {
        std::cout << "10 hit points extra (fire) damage because the original roll is 20.\n";
        data.value += 10;
        data.damageType = Fire;  // Bug, what about the previous acid damage?  Everything is now fire damage!
    }   
}

inline void ArmorSpell::adjustAttackRoll (AttackMediator::Data& data) const {  // Lowers attack roll by 4 if the being under the spell is a Troll.
    if (data.target->isTroll()) {
        std::cout << "attack roll decreased by 4 because the target is a Troll.\n";
        data.value -= 4;
    }
}

// Lowers damage by half if it is fire damage.
inline void ArmorSpell::specialDamageAdjustment (AttackMediator::Data& data) const {
    if (data.damageType == Fire) {
        std::cout << "Total fire damage reduced by half because of ArmorSpell in effect.\n";
        data.value /= 2;
    }
}

// Lowers attack roll by 2 against sword attacks.
template <typename T>
inline void MagicalArmor<T>::adjustAttackRoll (AttackMediator::Data& data) const {
    if (data.damageInflictor->isSword()) {
        std::cout << "attack roll decreased by 2 because damageInflictor is a Sword.\n";
        data.value -= 2;
    }   
}

// Reduces damage by 10 if the damage is acid damage.
template <typename T>
inline void MagicalArmor<T>::specialDamageAdjustment (AttackMediator::Data& data) {
    if (data.damageType == Acid) {
        std::cout << "Total acid damage reduced by 10 because of MagicalArmor<T> worn.\n";
        data.value -= 10;
    }
    else
        std::cout << "Despite the MagicalArmor<T> worn, no damage reduction because damage is not acid.\n";
}

int main() {
    LivingBeing *fighter = new Fighter ("Fighter", new Elf),  *troll = new Troll ("Troll");

    fighter->setWeaponInHand (new MagicalSword);
    fighter->isUnderSpell (new SwordSpell);
    troll->setArmor (new MagicalArmor<LeatherArmor>);
    troll->isUnderSpell (new ArmorSpell);

    fighter->attacks(troll->getProxy());
}
\$\endgroup\$
1
  • \$\begingroup\$ I was just getting ready to post some code from an RPG I'm refactoring that involves ignoring defense only when the attacker's class is so-and-so and the weapon is such-and-such. I'll see how this turns out for you first =D. \$\endgroup\$ Commented Sep 21, 2015 at 14:00

1 Answer 1

1
\$\begingroup\$

I don't think this really is the Mediator pattern as shown in Design Patterns. That's generally used to connect multiple, known Colleague classes, whereas here we're coordinating three unknown subclasses (the two creatures and the weapon).


Let's look closer:

        struct Data {
            LivingBeingProxy *attacker, *target;
            DamageInflictor* damageInflictor;
            std::reference_wrapper<int> value;
            int originalRoll;
            DamageType damageType;
            Data (LivingBeingProxy* a, LivingBeingProxy* t, DamageInflictor* d, int& v) :
                attacker(a), target(t), damageInflictor(d), value(v), originalRoll(v) {}        
        };

The constructor is redundant, as it duplicates the default aggregate constructor (and we should be initialising damageType as well).

Is it intentional that attacker, target and damageInflictor can be null? If not, we normally prefer references (or std::reference_wrapper if it needs to be reseatable).


I wouldn't bother writing inline on any declarations - most compilers ignore the hint and make better judgements than authors in any case. inline virtual and inline override are particularly unlikely to be useful.

Don't write both virtual and override - if it's override it must already be a virtual.


This looks like a failure of object-oriented design:

   virtual bool isSword() const {return false;}

as do:

    virtual bool isFighter() const {return false;}
    virtual bool isTroll() const {return false;}

Outside the review fragment, I see

class Troll : public Monster {
    private:
        DamageInflictor* claws = new DamageInflictor (10, Slashing);

There's a leak here, because there's no corresponding delete. Normally, that should be in the ~Troll() destructor - and we need to ensure that the base-class destructor is virtual (even if = default).

However, I don't see any need for a pointer here, since a Troll's claws stay with the object its entire life. Just use a member object:

class Troll : public Monster {
    private:
        DamageInflictor claws{10, Slashing};

In general, we need to be much clearer about which pointers own allocated memory, and which objects are therefore responsible for deleteing it. C++'s smart-pointer classes could usefully provide a way out from the current hole we're in.


Prefer <random> for generating random numbers. We have:

int roll = std::rand() % 20 + 1;

We convey intent better if we use a std::uniform_int_distribution{1, 20} - and all outcomes are equally likely (unlike using %, where there is a slight bias towards the lower numbers).

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Another issue is LivingBeingProxy inheriting from LivingBeing but never using it. \$\endgroup\$ Commented Nov 14, 2022 at 15:33
  • \$\begingroup\$ I didn't comment on the proxy class, as I was trying to focus on the code in the "review" part of the question. The other weird thing about the proxy is that it seems pointless - it just forwards everything to the actual creature unmodified, and is never subclassed. It looks like the remains of an experiment that didn't work out. \$\endgroup\$ Commented Nov 14, 2022 at 16:12

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.