0

It used to be fairly common for people to call a getter, do some calculation on it, then call a setter with the result. This is a clear sign your calculation actually belongs to the class you called the getter on. "Tell, don't ask" was coined to remind people to be on the lookout for that anti-pattern, and it worked so well that now some people think that part is obvious

(source: Karl Bielefeldt's answer)

This is by far not the first time I hear a piece of such advice.

Yet, looking at the game I'm trying to make, I simply can't seem to see how to bring this code to conformance with this advice. Admittedly, this anti-pattern is rampant there.

This is the current design: Monsters can be under Effects and can have Actions queued for execution. Monsters are instances of a class specific to their species, which inherits from the Species abstract class - which in turn holds state such as their current HP, stamina, a list of Effects they are under or a list of Actions that are queued for execution.

Let me put here a few examples why it is so hard for me to conform to this advice. For example, let's pay a closer look on the Attack Action. Seems straightforward enough: total attack power is computed as Attack.power * attacker.power (attacker is a monster) and this value is substracted from the defender's current HP. But now, where to put this logic? To conform with this piece of advice we would have to have a makeAttack method on the attacker, which passes its current power to the attack, which then multiplies this value by its own power and passes it to the defender's beAttacked method, which then substracts this amount from its own HP. And, indeed, this is how I started this.

However, it turns out, this is only the simplest case. The Attack on its own can be under Effects. Soon, as the complexity of the game rules was growing, I was finding myself making more and more fields public and moving more and more logic to the effects and actions themselves and away from whatever is under these effects and whatever these actions are supposed to affect.

Consider these requirements:

  • If an Attack is of type Fire AND it penetrates the defender's shield then a Burn debuff is applied to the defender whose strength is proportional to the amount of fire damage that has passed through the shield.
    • This necessites either querying the defender stats after the damage by the Attack or polluting the defender's beAttacked method with additional arguments and logic or having an applyBurn method on the defender.
  • If the defender is blocking (is under a Block effect) he mitigates a portion of the incoming damage at the cost of some of his Stamina (he can't, therefore, mitigate more damage than he has stamina for). How much damage is exactly mitigated depends on the ratio of both monsters' power.
    • Again: put this logic in the Block buff, which then has to query the state of both monsters AND the power of the attack? Or pollute other classes with unecessary methods?
  • If the defender is under a Dodge buff, he has a chance of avoiding the damage completely, depending on both monsters' speed ratio.
    • Same problems as above...
  • Attacks that have the AreaOfEffect field set to True cannot be dodged at all...
  • The battle arena itself can be under effects - for example, Hurricane or Conflagration. But- there is a combo - if both effects are in play, thy are both removed and a third effect, Firenado is placed, which is more powerful than both Hurricane and Conflagration at the same time.
    • When Conflagration is being placed it has to ask the battlefield if there is a Hurricane already and remove it if necessary...
  • Force attacks ignore some portion of shield completely, Pierce ones are instead more powerful to the extend they are blocked by shield
    • Make it work with the blocking semantics...

I could go on and on and on. The bottom line is: the only alternative to tell, don't ask seems to be... For each subclass of Effect that can affect a monster, make a method on the monster. For each subclass of Effect that can affect an Action, make a method on that action. For each subclass of Action that can target a monster, make a method on that monster.

I don't like this. Aside from the clutter such a solution would introduce, this kind of defeats the purpose of having separate Effect and Action subclasses. Logic that is semantically theirs would, in the end, be moved to methods of other classes! So, AFAIK, tell, don't ask is supposed to precisely avoid this what it would now introduce! If I have, say, 15 different types of Effects, then it makes sense to enable these Effects to simply operate on instances of IMonster whose requirements are kept simple (IMonster is supposed to have HP, stamina, etc, but not necessarily provide a burnMe method) rather than instances of IBurnableMonster which kind of makes little sense because every monster is burnable.

This also seems more elegant to me insofar as modifying the game's rules (adding a new effect or something like that) is supposed to boil down to only adding a new Effect subclass and/or making changes to one Effect subclass rather than also considering all classes this Effect would affect. (OK I'm lying here: the practice shows this is not the case, however, this is what I - ideally - wanted to achieve through this design).

So... currently I have a lot of getters and a lot of public fields. Which, I suppose, is a little tragedy in the eyes of any experienced engineer. As the number of special cases was growing, I even removed the makeAttack and beAttacked methods from the monster and instead put this logic in the Attack action.

However, now that I described the "architecture", how should this be handled in a more right-handed way?

Improve this question
1
  • It seems to me, on first reading that the buffs, and debuffs could be best applied as decorators on the character, perhaps with round/timer countdowns. I think that may more accurately capture the logic of the adjusted pass through values. So, not ask / don't tell, a different pattern? Commented Mar 8, 2019 at 12:04

1 Answer 1

Reset to default
1

While it makes sense in some cases, I wouldn't apply "tell, don't ask" as a universal rule. In this case, I think it would create a lot of undesirable coupling - if you're implementing a game design where there are many abilities and they have a lot of individual behavior (MtG, for example), then you want to contain that to the abilities themselves as much as possible.

For data that's publicly available information in the game design (ex: Power/Toughness in MtG) it's not breaking encapsulation to make that available - that information is part of the interface.

That said, the situation you describe is going to be complex no matter how you design it, because there are multiple types of defensive abilities that interact with multiple types of attacks. Handling each interaction individually will quickly become unwieldy.

For the specific functionality you describe, what I might do is to use tags as a bridge between the attacks and defenses. For example (very rough draft):

class IncomingDamage { public int Amount { get; } public IMonster Source { get; } public bool HasTag(string tag) => _tags.Contains(tag); ... (omitted because language-specific, just the usual) } interface IMonster { double Power { get; } double Speed { get; } // Runs the damage through a series of IDefenseLayers // Returns the damage that penetrated IncomingDamage ApplyDamage(IncomingDamage damage); } interface IDefenseLayer { IncomingDamage Apply(IMonster self, IncomingDamage damage); } interface ITargetedAbility { void Apply(IMonster origin, IMonster target); } class FlameAttack : ITargetedAbility { public void Apply(IMonster origin, IMonster target) { var initialDamage = new IncomingDamage(GetBasePower() * origin.Power, target, AttackTags.Fire); var finalDamage = target.ApplyDamage(initialDamage); // Check for tag again, so we can support something like a "water shield" which removes that tag if (finalDamage.Amount > 0 && finalDamage.HasTag(AttackTags.Fire)) { target.ApplyCondition(new BurnCondition(finalDamage.Amount)); } } ... } class DodgeDefense : IDefenseLayer { public IncomingDamage Apply(IMonster self, IncomingDamage damage) { if (!damage.HasTag(AttackTags.AoE) && RNG.Roll() < CalcDodgeChange(self.Speed, damage.Source.Speed)) { return damage.Negated(); } return damage; } ... }

Of course the details will depend on what type of abilities you need to support.

IME, even with a clean design, complex gameplay rules are going to produce fairly complex code. The kind of ultra-simple examples often shown when discussing design patterns are generally only possible for simple problems.

Improve this answer
1
  • Hmm, reading through your code I have a feeling that I may be doing something similar. Thank you. Still I'm looking for ways to reduce is spaghettiness though... Commented Mar 11, 2019 at 19:37

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.