Correcting a large disadvantage to the decorator pattern

Question

I decided a while back, in refactoring some game combat code, to try the decorator pattern. The combatants can have various passive abilities, and they may also be different types of creatures. I figured that decorator lets me add on behavior in various combinations at run time so I don't need hundreds of subclasses.

I've almost finishing making the 15 or so decorators for the passive abilities, and in testing I discovered something - a rather glaring disadvantage to the decorator pattern that I'm surprised I haven't heard of before.

For the decorators to work at all, their methods must be called on the outermost decorator. If the "base class" - the wrapped object - calls one of its own methods, that method won't be the decorated overload, since there's no way for the call to be "virtualized" to the wrapper. The whole concept of an artificial subclass breaks down.

This is kind of a big deal. My combatants have methods like TakeHit which in turn call their own Damage method. But the decorated Damage isn't getting called at all.

Perhaps I've chosen the wrong pattern or been overzealous in its application. Do you have any advice on a more appropriate pattern in this situation, or a way to work around this flaw? The code that I refactored from just had all the passive abilities sprinkled all over the combat code inside if blocks in seemingly random places, so that's why I wanted to break it out.

edit: some code

public function TakeHit($attacker, $quality, $damage)
{
    $damage -= $this->DamageReduction($damage);

    $damage = round($damage);

    if ($damage < 1) $damage = 1;

    $this->Damage($damage);

    if ($damage > 0)
    {
        $this->wasHit = true;
    }

    return $damage;
}

This method is in the base Combatant class. DamageReduction and Damage can and are both overridden in various decorators, for example a passive that cuts damage by a quarter, or another that reflects some damage back to the attacker.

class Logic_Combatant_Metal extends Logic_Combatant_Decorator
{
    public function TakeHit($attacker, $quality, $damage)
    {
        $actual = parent::TakeHit($attacker, $quality, $damage);

        $reflect = $this->MetalReflect($actual);
        if ($reflect > 0)
        {
            Data_Combat_Event::Create(Data_Combat_Event::METAL_REFLECT, $target->ID(), $attacker->ID(), $reflect);
            $attacker->Damage($reflect);
        }

        return $actual;
    }

    private function MetalReflect($damage)
    {
        $reflect = $damage * ((($this->Attunement() / 100) * (METAL_REFLECT_MAX - METAL_REFLECT_MIN)) + METAL_REFLECT_MIN);
        $reflect = ceil($reflect);

        return $reflect;
    }
}

But again, these decorator methods are never getting called, because they aren't being called from the outside, they're called inside the base class.

Answer 1

tl;dr: A decorator is meant to change the behavior of an object or function, but it does not override the behavior of the original like subclassing does.

If the "base class" - the wrapped object - calls one of its own methods, that method won't be the decorated overload, since there's no way for the call to be "virtualized" to the wrapper. The whole concept of an artificial subclass breaks down.

If I'm understanding correctly, you're saying this -

decorated_thingy_instance = DecoratorA(OriginalThingy))

given

DecoratorA{
    decoratedThingy = ...;
    doStuff(){
      decoratedThingy.doStuff()
      ...
    }
    doOtherStuff(){
      decoratedThingy.doOtherStuff()
        ...
    }
}

and

 OriginalThingy{

    doStuff(){
       this.doOtherStuff()
    }
    doOtherStuff(){
       ...
    }
}

Your problem is that DecoratorA's doOtherStuff isn't called. It's better to think of decorators applied to functions than objects and it's not exactly like subclassing. In principle, the behavior of each decorator shouldn't affect that of the other decorators or the inner object for the same reason you mentioned, you can't alter the control flow like you would a subclass.

In practice, this means you can alter the result of the functions exposed by the interface (multiply the output by 2), but you can't change how the wrapped class calculates the function. You could make a wrapper that completely discards the output of the wrapped class or doesn't call it altogether, e.g,

DevNullDecorator{

    decoratedThingy = new Thingy();
    doStuff(){
      //decoratedThingy.doStuff() 
      // do whatever you want
    }
    doOtherStuff(){
       ...
    }
}

But this more or less breaks the spirit of the pattern. If you want to modify the inner object itself, you'll need to write methods in the interface with getters and setters, this also more or less breaks the spirit of the pattern, but might work for your case.