Notes on coding the RPG Combat kata in TypeScript
-
Original instructions by Daniel Ojeda (link to slide show).
-
My repository (Github) with step-by-step commit history.
It had been over a year since I last tried out this variant of the kata (the original one) at its fullest, so I decided to do it again and take notes on the go so I could share them later.
Some housekeeping items first, though:
-
This solution is overengineered for the scope of the problem. My main goal was to practice SOLID principles, and I think a kata such as this, with a relatively small scope is good for that.
-
The kata is incremental; i.e. the instructions are split into steps and you are not supposed to look ahead. This offers the opportunity for refactors. Keep in mind that I've done this kata in the past, so I remembered some stuff, but forgot about much other, so I believe this take is realistic in this sense.
-
I used Test-Driven Development to implement the kata, adding unit tests before implemented the related functionality. In these notes tests are usually omitted, but you can check them out in the repository. Integration tests, though, were added after the implementation.
-
In my repository you can find step-by-step commits if you want to follow along. There are also branches for every step of the way, including refactors.
Table of contents
- Iteration #1: Characters
- Iteration #2: Levels
- Iteration #3: Attack range
- Retrospective
- Iteration #4: Factions
- Iteration #5: Props
- Final retrospective
Iteration #1: Characters 🔝
See the code branch in Github.
Requirements
- Characters have Health, starting at
1000
health points.- Health cannot be dropped below zero. If this happens, health is set to
0
. - Health cannot go above
1000
health points.
- Health cannot be dropped below zero. If this happens, health is set to
- Characters can perform these actions:
- Deal damage to another character.
- Heal themselves.
- Characters can be dead or alive.
- A character with
0
health points is dead. - Dead characters cannot perform actions.
- A character with
The Character
class
Since I wanted to practice SOLID, I opted for a OOP approach, with a class to model characters.
I implemented a Character
class with a health
property.
health
users a setter to ensure we stay within the valid health points range ([0,1000]
) .isAlive
is a "computed property" (in TypeScript, a getter).
💡 TIP: The instructions don't mention it, but in RPG's the amount of damage a character deals when attacking is usually part of their "stats" or "attributes". The same goes for the amount of healing.
So I'm adding these as properties for
Character
, rather than passing those parameters to theattack
andheal
methods we need to create later.
To configure the initial variables for these character properties we will pass an options object to the constructor. So we can instantiate characters like this: new Character({health: 500})
.
// src/character/character.ts
export default class Character {
// ...
constructor(options: CharacterOptions = {}) {
const defaults = {
health: 1000,
damage: 0,
healing: 0,
};
const { health, damage, healing } = { ...defaults, options };
// ...
}
}
I decided to treat attacking or healing with dead characters as an error. In TypeScript, we use exceptions to handle errors, so we'll throw some of these.
Last, to try this Character
class out, I instantiated two of them in src/index.ts
and have them fight each other.
Iteration #2: Levels 🔝
See the code branch in Github.
Requirements
- A character cannot target themeselves for attacking.
- Characters have a Level, starting at
1
. - Levels are taken into account for attacking:
- If the target is 5+ levels above the attacker, the attack will have a debuff of
-50%
to the damage. - If the target is 5+ levels below the attacker, the attacker will have a buff of
+50%
to the damage.
- If the target is 5+ levels above the attacker, the attack will have a debuff of
Adding levels
Since we have not received requirements to level up characters, I'm assuming the level will not change after the Character
instance has been created. Then, we can just have a readonly
property of Character
for this.
I'm handling the constraint of characters not being able to target themselves for attacking as a thrown exception, like the previous attack restrictions.
Computing the final damage done to another character is handled in a private helper function, to help code readability.
// src/character/character.ts
export default class Character {
// ...
attack(other: Character) {
if (!this.isAlive) {
throw new Error("Dead characters cannot attack");
}
if (this === other) {
throw new Error(
"Invalid attack target: Characters cannot attack themselves"
);
}
const damage = this.#damageTo(other);
other.health -= damage;
}
#damageTo(other: Character) {
const diff = this.level - other.level;
const coeff = diff >= 5 ? 1.5 : diff <= -5 ? 0.5 : 1.0;
return this.damage * coeff;
}
}
Iteration #3: Attack range 🔝
See the code branch in Github.
Requirements
-
Characters have an attack range.
- If they are a Melee figther, their range would be
2
meters. - If they are a Ranged figther, their range would be
20
meters.
- If they are a Melee figther, their range would be
-
When dealing damage in an attack, the target must be within range.
Melee and ranged fighters
Here's comes the first dilemma: shall Melee
and Ranged
be their own types?.
-
TypeScript does have abstract classes, so we could make
Character
abstract and only provideMelee
andRanged
as concrete classes to be instantiated. -
But is this justified just because we can? For the time being, I don't think so. It would also pose different problems if then we also have yet another new behavior that depends on a different "subtype" (for instance, if we had jobs for classes that give buffs to certain attack types, or to healing, etc.).
- We just need this distinction to determine the value of a
range
property for attacking. We can get by with just having anattackType
property and computingrange
out of it. - This
attackType
has a union type of `"melee" | "ranged", since I prefer unions to enums in TypeScript.
- We just need this distinction to determine the value of a
-
An alternative to the above could be to opt for composition, and make this
attackType
its own class, but it does feel overkill.
// src/character/character.ts
export type AttackType = "melee" | "ranged";
export default class Character {
// ...
readonly attackType: AttackType;
constructor(options: CharacterOptions = {}) {
const defaults = {
// ...
attackType: "melee" as AttackType,
};
// ...
this.attackType = attackType;
}
get range() {
return this.attackType === "melee" ? 2 : 20;
}
// ...
}
It is not mentioned in the instructions, but we have an implicit requirement of keeping track of characters' positions. I chose to assume a 2-dimensional space and use the Eclidean distance to compute the distance between two characters.
Position is modeled with a new type, Vec2d
:
// src/character/character.ts
export type Vec2d = { x: number; y: number };
"Out of range" attacks are considered as an error, so an exception is thrown when this happens.
// src/character/character.ts
export default class Character {
// ...
position: Vec2d;
constructor(options: CharacterOptions = {}) {
const defaults = {
// ...
position: { x: 0, y: 0 } as Vec2d,
};
this.position = position;
// ...
}
attack(other: Character) {
// ...
const isWithinRange = this.#distanceTo(other) <= this.range;
if (!isWithinRange) {
throw new Error("Invalid attack target: Out of range");
}
// ...
}
#distanceTo(other: Character) {
const dx = this.position.x - other.position.x;
const dy = this.position.y - other.position.y;
return Math.sqrt(dx * dx + dy * dy);
}
// ...
}
Retrospective 🔝
See the code branch in Github.
Prompts
-
Are you keeping up with the requirements?
-
Do you feel good about your design? Are you confident it is scalable and easily adapted to the the new requirements that will be introduced in the next iterations?
-
Is everything tested? Do your current tests give you confidence?
A reflection…
I'm not happy with the current design: the Character
class is getting huge and it does many things:
- Deals with health.
- Holds immutable character stats, like the fighter type, level, etc. and mutable stats (like position).
- Deals with positioning and calculating distances.
- Deals with attacking:
- Validates the attack before performing it (character is dead, out of range, etc.).
- Calculates the damage being dealt depending on other stuff (level difference).
- Deals damage to the other character.
- Deals with healing:
- Validates healing.
- Recovers health.
I opted for a refactor to mitigate this before advancing to the next iterations of the kata. The goal would be to increase the cohesion of the design, by applying the Single Responsibility Principle of SOLID.
Extracting positioning and distances
This is low-hanging fruit. Although position has its own type Vec2d
, I decided to extract it into its own class and add a method to compute the distance between two points.
// utils/vec2d.ts
export default class Vec2d {
x: number;
y: number;
// ...
distanceTo(other: Vec2d) {
const dx = this.x - other.x;
const dy = this.y - other.y;
return Math.sqrt(dx * dx + dy * dy);
}
}
Using this new class is as easy as substituting new Vec2d
for our existing {x: foo, y: bar}
expressions.
Organizing character properties
I opted to diffentiate immutable and mutable character properties.
-
Immutable properties (
attackType
,level
,damage
andhealing
) will be all grouped under a private#stats
property, and each of those will have its own getter for easy access. -
Mutable properties (
health
,position
) will remain separated, as they currently are.
// src/character/character.ts
export default class Character {
#health: number;
position: Vec2d;
#stats: {
damage: number;
healing: number;
level: number;
attackType: AttackType;
};
// ...
get level() {
return this.#stats.level;
}
// more getters here
// ...
}
Extracting the attack action
I decided to give combat actions (attack, healing) their own type, to lift this responsibility off Character
.
For attacks, I implemented a new AttackAction
class, that is instantiated with a source
and a target
, and has a run()
method to perform the action.
// src/actions/attack.ts
export default class AttackAction {
readonly source: Character;
readonly target: Character;
constructor(source: Character, target: Character) {
this.source = source;
this.target = target;
}
run() {
if (!this.source.isAlive) {
throw new Error("Dead characters cannot attack");
}
if (this.source === this.target) {
throw new Error(
"Invalid attack target: Characters cannot attack themselves"
);
}
const distance = this.source.position.distanceTo(this.target.position);
const isWithinRange = distance <= this.source.range;
if (!isWithinRange) {
throw new Error("Invalid attack target: Out of range");
}
this.target.health -= this.#damage;
}
get #damage() {
const diff = this.source.level - this.target.level;
const coeff = diff >= 5 ? 1.5 : diff <= -5 ? 0.5 : 1.0;
return this.source.damage * coeff;
}
}
The code for AttackAction
is almost verbatim that the one we have in Character
, only that now this.source
and this.target
are used in place of this
and other
. I also moved all the attack tests to their own spec file (src/actions/attack.spec.ts
).
Once the tests pass, we can remove the attack-related code from Character
and update the main src/index.ts
file to use the new action instead of calling Character.attack()
:o
// src/index.ts
new AttackAction(orc, elf).run();
Extracting the heal action
This is the same procedure as with the AttackAction
:
- Create a
HealAction
class, with a constructor that sets asource
. - Copy the code from
Character.heal()
toHealAction.run()
, changing the previousthis
forthis.source
- Move the healing tests to their own
src/actions/heal.spec.ts
spec file, and adapt them so they compile and useHealAction
instead. - Delete healing-related code from
Character
. - Use the new
HealAction
insrc/index.ts
.
// actions/heal.ts
export default class HealAction {
readonly source: Character;
constructor(source: Character) {
this.source = source;
}
run() {
if (!this.source.isAlive) {
throw new Error("Dead characters cannot heal");
}
this.source.health += this.source.healing;
}
}
Dealing with coupling
With the changes above, the cohesion of the domain has increased, since positioning, attack and healing are separated now.
However, by doing so there's a new problem: coupling. While Vec2d
is fully isolated, both AttackAction
and HealAction
depend on Character
.
A tight coupling makes the code harder to adapt to new requirements. To lessen the coupling, we can apply some SOLID principles: Dependency Inversion, Liskov Substitution and Interface Segregation.
Regarding inverting the dependencies, it's half done already: characters that are involved in the combat actions are passed as constructor parameters to AttackAction
and HealAction
. We just need to apply the other principles to set it up properly.
For segregating the interfaces there's work to do: we must ensure that the actions don't require methods or properties that they don't need… And since the current code takes Character
instances, there's indeed uneeded stuff (for instance, AttackAction
does not need to know about healing
).
- The way to solve this in TypeScript is with interfaces. I'll create the ones needed and have
Character
to explicitly implement those.
As with the Liskov substitution principle, we'd need to ensure that AttackAction
and HealAction
work with concrete types that implement those new interfaces (Character
for now).
Loosening the coupling in HealAction
HealAction
is more simple than AttackAction
, so it's a good starting point. Both of these need to know about the health points of a character, so I added a common HasHealth
interface to represent this.
For healing, the action needs to know about the healing
property of an entity: the Healer
interface will take care of this.
// src/actions/index.ts
interface HasHealth {
health: number;
isAlive: boolean;
}
export interface Healer extends HasHealth {
healing: number;
}
// src/character/character.ts
export default class Character implements Healer {
// ...
}
The next step is to use this new interface in HealAction
in place of Character
:
// actions/heal.ts
export default class HealAction {
readonly source: Healer;
constructor(source: Healer) {
this.source = source;
}
// ...
}
And last, the unit tests for HealAction
need to not use instances of Characters
. Since now we have an interface, we could just create some mocks that implement this interface:
// src/actions/heal.spec.ts
const anyHealer = ({ health = 1000, healing = 1 } = {}) => {
return { health, healing, isAlive: health > 0 };
};
// ...
it("Healers heal themselves", () => {
const source = anyHealer({ health: 100, healing: 50 });
const heal = new HealAction(source);
heal.run();
expect(source.health).toBe(150);
});
Loosening the coupling in AttackAction
For attacks, the apporach is very similar, but different interfaces for attack source (Attacker
) and target (AttackTarget
) are needed.
// src/actions/index.ts
interface HasLevel {
level: number;
}
interface HasPosition {
position: Vec2d;
}
export interface Attacker extends HasHealth, HasLevel, HasPosition {
damage: number;
range: number;
}
export interface AttackTarget extends HasHealth, HasLevel, HasPosition {}
// src/actions/attack.ts
export default class AttackAction {
readonly source: Attacker;
readonly target: AttackTarget;
constructor(source: Attacker, target: AttackTarget) {
// ...
}
// ...
}
For the unit tests, we can just use more mock objects that satisfy the interfaces:
// src/actions/attack.spec.ts
const anyAttacker = ({
health = 1000,
damage = 1,
level = 1,
range = 0,
position = new Vec2d(),
} = {}) => {
return { damage, level, range, position, isAlive: health > 0, health };
};
const anyTarget = ({
health = 1000,
level = 1,
position = new Vec2d(),
} = {}) => {
return { health, isAlive: health > 0, level, position };
};
describe("Character attack", () => {
it("Deals damage to another character", () => {
const source = anyAttacker({ damage: 50 });
const target = anyTarget({ health: 1000 });
const attack = new AttackAction(source, target);
attack.run();
expect(target.health).toBe(950);
});
// ...
});
We should also mark Character
as a type that implements those interfaces:
// src/character/character.ts
export default class Character implements Healer, Attacker, AttackTarget {
// ...
}
Integration tests
By removing the coupling between Character
and the combat actions, we now have a true separation between these. We can see that the actions do not import Character
at any time, not even for testing!
However, the main program does need Character
instances to attack one another, and heal themselves… This is why integration tests are needed, to check that using Character
in AttackAction
and HealAction
works as intended.
These tests don't use mocks, but real objects.
// test/combat.spec.ts
describe("Character combat", () => {
it("Allows characters to attack one another", () => {
const attacker = new Character({ damage: 100 });
const target = new Character({ health: 1000 });
new AttackAction(attacker, target).run();
expect(target.health).toBe(900);
});
it("Allows characters to heal themselves", () => {
const healer = new Character({ health: 900, healing: 50 });
new HealAction(healer).run();
expect(healer.health).toBe(950);
});
});
Iteration #4: Factions 🔝
See the code branch in Github.
Requirements
-
There are Factions now.
-
Characters can join or leave one or more factions.
-
Characters that belong to the same faction are considered allies.
- Characters can heal themselves, and allies.
- Characters cannot attack allies.
Implementing a FactionManager
Where do factions belong?
A first idea could be to add joinFaction
and leaveFactions
methods to Character
. This, however, would imply:
-
Decreasing the cohesion of
Character
, since it would be doing yet another new thing —and I just refactored it to take features out of it. -
Going against the Open-Closed principle, in which we shouldn't modify existing classes (or if we must, do it in the minimum way possible) to add new features.
Instead we could have factions completely independent and handled by a new class, FactionManager
, in a similar way as the combat actions.
-
FactioNManager
will havejoin
andleave
methods. -
FactionManager
will be responsible of determining whether two characters are allies.- And we have to do this without coupling
Character
andFactionManager
.
- And we have to do this without coupling
On a practical level, a faction is identified by a string, and will store its members of any
type, in a Set
(which, in TypeScript, automatically forbids duplications of members). In TypeScript, this means that a ==
comparison is performed by Set
to determine whether two members are the same. Two objects are considered to be equal if they have the same memory address.
Factions should't be repeated either, so they will be stored in a Map
, with the faction name (a string
) as key.
// src/factions/factions.ts
export default class FactionManager {
#factions: Map<string, Set<any>>;
constructor(factions = new Map<string, Set<any>>()) {
this.#factions = factions;
}
join(faction: string, member: any) {
const members = (this.#factions.get(faction) ?? new Set()).add(member);
this.#factions.set(faction, members);
}
leave(faction: string, member: any) {
this.#factions.get(faction)?.delete(member);
}
members(faction: string) {
if (!this.#factions.has(faction)) {
throw new Error("Faction does not exist");
}
return this.#factions.get(faction);
}
areAllies(member: any, other: any) {
return [...this.#factions.values()].some(
(faction) => faction.has(member) && faction.has(other)
);
}
}
💡 TIP: Implementing
members
was not a requirement of the kata, but it is handy for debugging and testing.
Checking alliances in AttackAction
AttackAction
needs to check alliances before the damage is dealt. Shall we add a brand new action to do this and, at the same time, comply with the Open-Closed principle?
-
The instructions don't tell us whether the old behavir (no checking alliances) need to be kept or not, so we can just choose what suits us best.
-
The attack rules are essentially the same, just the new check is needed.
This is kind of a grey area, but I opted for modifying the existing class instead. To keep backwards-compatibility with the old API, the faction manager will be passed to the action constructor as an optional parameter.
Also, to avoid coupling AttackAction
with FactionManager
, a new interface is needed:
// src/actions/index.ts
export interface AllianceInformer {
areAllies: (a: any, b: any) => boolean;
}
// src/actions/attack.ts
export default class AttackAction {
// ...
readonly #alliances?: AllianceInformer;
constructor(source: Attacker, target: AttackTarget, alliances?: AllianceInformer) {
// ...
this.#alliances = alliances;
}
run() {
// ...
if (this.#alliances?.areAllies(this.source, this.target)) {
throw new Error("Invalid attack target: cannot attack allies");
}
// ...
}
A new action for healing
For healing, things are a bit different, because the behavior does change: instead of healing themselves, healer make another character to recover health points.
-
I could make the constructor accept two optional paramters (the target and the faction manager), but…
-
Keeping the same action feels messy. I think it is justified to create a brand new action,
HealAnotherAction
to heal other characters. Since healing another character requires checking whether they are allies, the faction manager will not be an optional parameter here.
Note that, since we are creating a new class and keeping the existing one HealAction
, we don't need to worry about backwards compatibility.
// src/actions/heal.ts
export class HealAnotherAction {
readonly source: Healer;
readonly target: HealTarget;
#alliances: AllianceInformer;
constructor(source: Healer, target: HealTarget, alliances: AllianceInformer) {
this.source = source;
this.target = target;
this.#alliances = alliances;
}
run() {
if (!this.source.isAlive) {
throw new Error("Dead characters cannot heal");
}
if (!this.target.isAlive) {
throw new Error("Invalid heal target (cannot heal dead characters)");
}
if (!this.#alliances.areAllies(this.source, this.target)) {
throw new Error("Invalid heal target (can only heal allies)");
}
this.target.health += this.source.healing;
}
}
Integration tests for factions
Like in the refactor, integration tests are needed to check that Character
, FactionManager
, and the combat actions can all be used together.
// test/combat.spec.ts
describe("Factions", () => {
it("Characters can join and leave a faction", () => {
const factions = new FactionManager();
const orc = new Character();
factions.join("horde", orc);
expect(factions.members("horde")?.has(orc)).toBeTrue();
factions.leave("horde", orc);
expect(factions.members("horde")?.has(orc)).toBeFalse();
});
it("Characters can heal an ally", () => {
const healer = new Character({ healing: 50 });
const target = new Character({ health: 50 });
const factions = new FactionManager();
factions.join("foo", healer);
factions.join("foo", target);
new HealAnotherAction(healer, target, factions).run();
expect(target.health).toBe(100);
});
it("Disallows characters to attack allies", () => {
const attacker = new Character({ damage: 100 });
const target = new Character();
const factions = new FactionManager();
factions.join("foo", attacker);
factions.join("foo", target);
const attack = new AttackAction(attacker, target, factions);
expect(() => {
attack.run();
}).toThrow(/invalid attack target/i);
});
});
Iteration #5: Props 🔝
See the code branch in Github.
Requirements
-
Characters can damage Props (a tree, a house, etc.). Any entity with Health that is not a Character, is a Prop.
-
Props cannot heal or be healed.
-
Props cannot deal damage.
-
Props do not belong to any faction, they are neutral.
Implementing Prop
Since Prop
need to function as attack targets, this clas need to implement AttackTarget
. This requires:
- Implementing
HasHealth
(i.e.health
andisAlive
properties). - Implementing
HasLevel
. The instructions don't say anything about this, so I opted for having Props with level set to1
. - Implementing
HasPosition
.
// src/prop/prop.ts
interface PropOptions {
health?: number;
position?: Vec2d;
}
export default class Prop implements AttackTarget, Neutral {
#health: number;
readonly position: Vec2d;
constructor({ health = 1000, position = new Vec2d() }: PropOptions = {}) {
this.#health = health;
this.position = position;
}
get level() {
return 1;
}
get isAlive() {
return this.health > 0;
}
get health() {
return this.#health;
}
set health(value: number) {
this.#health = Math.max(0, value);
}
}
Factions and props
How to disallow props from joining factions?
An option could be to have as faction members, instead of any
, an interface that Character
would implement (for instance, HasID
with an id
string property). However, this would require to modify both Character
and FactionManager
.
Another solution could be to have FactionManager
to reject Prop
instances somehow.
-
If I want this to be decoupled, I cannot use a
instanceof Prop
check inFactionManager.join()
. -
Instead, I could have a new interface,
Neutral
, and haveFactionManager.join()
check that. This still requires to modifyFactionManager
, but at least this new behavior is related to factions.
The new Neutral
interface is just an isNeutral
property set to true
. Once created, we just need a type guard to check whether an object satisfies that interface.
// src/factions/index.ts
export interface Neutral {
isNeutral: true;
}
// src/factions/factions.ts
export default FactionManager {
// ...
join(faction: string, member: any) {
if (isNeutral(member)) {
throw new Error("Invalid member: neutral entities cannot join factions");
}
// ...
}
}
function isNeutral(value: unknown): value is Neutral {
return !!value && (value as Neutral).isNeutral;
}
// src/prop/prop.ts
export default class Prop implements AttackTarget, Neutral {
// ...
readonly isNeutral = true;
}
Integration tests for props
Integration tests are needed to check that the restrictions props have are actually in place.
⚠️ WARNING: Some of these tests will not be possible to write, because of type checking. This is indeed a good thing! Leveraging this at compile time instead of at running time is one of the advantages of using TypeScript.
// test/combat.spec.ts
describe("Props", () => {
it("Cannot join factions", () => {
const house = new Prop();
const factions = new FactionManager();
expect(() => {
factions.join("foo", house);
}).toThrow(/invalid member/i);
});
it("Can be damaged by characters", () => {
const house = new Prop({ health: 1000 });
const attacker = new Character({ damage: 100 });
new AttackAction(attacker, house).run();
expect(house.health).toBe(900);
});
});
Final retrospective 🔝
TL;DR: This solution is obviously over-engineered for the scope of the problem, but my intent with the kata was to practice applying SOLID principles, and this was achieved.
What problems did you encounter?
-
Not being able to "look ahead" led to a poorer design (for instance, I would have found a way to restrict faction membership that is less clunky had I known in advance). But I do recognize this just makes the kata more fun! 🙂
-
Code duplication in some places (for instance, health-related code). I feel this kata could leverage composition.
-
Overall, the kata can feel a bit long. The part of positioning, ranges, etc. IMHO do not add much value or pose interesting questions, but did require quite a bunch of typing.
What have you learned?
Rather than learning something new, I think it was a good practice of SOLID, since all the five principles were applied.
-
Liskov Substitution principle, by having the attacks work against both
Character
andProp
. Also, all the unit tests (with mocks) work because of this. -
Open-Closed principle, specially the way the factions work and how their restrictions in combat were handled.
-
Single responsibility principle was the main driver of the huge refactor after the first chunk of functionality.
-
Interface segregation principle, by using narrow interfaces as the argument of methods, instead of concrete types that have more functionality than is actually needed.
-
Dependency inversion principle was enabled by the interfaces. A good example are the combat actions and their constructors.