Let's code with the Roguelike tutorial - Part 6 - Combat engine

A small rust epiphany

These are good rules to follow in general, regardless of language. They're also not particularly groundbreaking. But whereas in many other languages these are enforced by convention, and it might be ok to be a little sloppy about it, in Rust these rules are enforced strictly, down to statement level granularity.

Adding combat attributes

I now have a map, and a bunch of monsters on the map, able to attack and be attacked by the player.

To provide for different kinds of monsters, I needed to add stats like combat strength and HP.

Previously, I had mentioned some concern about the use of inheritance for the different types of objects, and how that probably wasn't a very good architecture. Fortunately, the Python tutorial uses composition, and that's what I'll use also. One common architecture for games of this type is Entity - Component - System (ECS), and that's the type of architecture I'll use here.

First, I needed a fighter component, which will be for all entities that can engage in combat. It will hold combat strength, defense, current HP, and max HP.

How should I represent this in Rust?

At first, I thought I might be able to use traits. Traits are (very roughly) like interfaces, so I could have a trait for each component, and then different types of entities could have different sets of components. At first glance, this seemed like a reasonable idea, but after reflecting on it a bit, I don't think this is the right fit:

So, instead of making these traits, I made them optional fields on the entity. I could have used an array of components per entity, but for this first implementation, the simplest version is sufficient. It's true that this doesn't really get much of the performance benefits of an ECS system. It also might get a bit unwieldy if I add too many components. I will reevaluate and refactor when the time comes.

I started with the fighter component, for things that can take and deal damage. Here I found out that Rust requires every ivar to be initialized on object construction. Even with just a few components, this is going to get tedious pretty quickly. I ended up tagging the class with #[derive(Default)] to enable default struct fields (I needed to add a default for the Color struct also). Even after that, I still needed to go through each creation site and add ..Default::default() to each place where I created an entity. That's the price of safety I guess.

I created some constants for the player and different monster types, to specify the initial parameters. Since I'm using them during object creation, I marked them as #[derive(Copy,Clone)] so that they can be used to populate new entities. In a bigger game these parameters wouldn't be in the code, but probably in some external config file so that they can be balanced or modded without having to rebuild the entire game.

Adding the combat engine

Now to try this out, I added the actual combat code. Previously I had already added println!'s to all the different places where there would be combat, so all I have to do is find and replace those with the real combat code.

In a typical object oriented approach, there would be some methods on the components that would contain the code to handle combat. So for example, there would be a FighterComponent::dealDamage, or something of that nature. However, in an ECS architecture, the component data and the systems are deliberately separated. So, I created a FighterSystem, which will contain attack(attacker,target) and any other functions relevant to combat.

Here is an interesting design problem. To try to keep the FighterSystem::attack function as simple as possible, I had it take the fighter component as parameters. The good part about this design is then I don't have to do any error checking to see if the entity even has fighter components, which keeps the attack function simple and focused on combat. The bad part about this design is that I didn't build in any way to go from the components to the entities that own them. Now, arguably, the fighter component should contain everything that's needed to handle combat. But in this case, I want to print to the UI that something has been attacked, and it's obviously wrong for the FighterComponent to own the name of an entity.

So I think I have two choices here:

I tend to favor compile time validity checking over run time, and I can't imagine that I'm going to need a ton of values from the combat, so I implemented the second option and keep passing in the components only. This is another one of those murky decisions where another programmer could easily justify the first approach and it would be a reasonable choice.

Incidentally, when working on a team project with others, these can be some of the hardest patches to negotiate. I might feel very strongly about one of the above approaches, but I might receive a patch implementing the other approach. In some cases, I might hold firm and argue for the approach that I think is right. On the other hand, there are often many reasonable ways to approach a problem, and sometimes during initial development there just isn't enough data to be confident about the right assessment. In those cases often the only way to make progress is to prototype the best guess, and use that to see what goes wrong.

Anyway, so I tested out this code: 


if let Some(target) = target {
  if let Some(mut player_fc) = player.fighter {
    if let Some(mut target_fc) = target.fighter {
      let damage = FighterSystem::attack(&mut player_fc, &mut target_fc);
      println!("Player attacks {} for {} damage, remaining ({}/{} HP).", target.name, damage, target_fc.hp, target_fc.max_hp);
    }
  }
}

And... it doesn't work. I tried testing it on an orc, and it continously spews: 

Player attacks troll for 4 damage, remaining (12/16 HP).
Player attacks troll for 4 damage, remaining (12/16 HP).

This problem took a while to debug, and was actually kind of interesting.

When debugging, I try to refrain from jumping immediately to using println!'s or stepping through with the debugger. I find it most effective to first spend a little time to think about a hypothesis and the chain of events that might lead up to the failure.

Here are the steps I thought about in this case:

  1. The direct combat code seems to be working: That is the correct damage, and the resulting total, is the same. So extracting the component should be working, and it's getting passed into, and read in, the attack function correctly.
  2. The result doesn't seem to be stored, since every combat starts with the same HP values. But I'm passing in a mutable reference to the components, so I don't understand why that wouldn't be the case.
  3. I started adding some prints to see where the value is being lost. The first place is right after the borrow of the components - I borrow the components again and print out the values - and they're already wrong.
  4. I grumbled a bit, stared at the code, and can't see the problem. I'm borrowing references everywhere. I need a break.
  5. Some indeterminate time later (but quite a bit later), I decide to remove all possibility of copying by removing the derived attribute #[derive(Copy,Clone)]. Now the compiler complains that it can't copy the component - on the line where I'm doing the if let match.

So, in the end, this turned out to be a bit of a knowledge problem. I had thought that matching Some(X) creates a reference to X, but in fact, it makes a copy of X. Also: if I hadn't put in the copy "just in case," I wouldn't have run into this problem. It turns out I didn't need it anyway because Rust knows how to instantiate from the consts without the copy attribute.

Now that I have the combat working, there are two major things I need to finish this part off. I need to handle death detection, and I need to give the enemies some kind of AI. I don't see any dependency between these two features, so I semi arbitrarily just picked the AI next.

Adding an enemy AI

To begin implementing enemy AI, I added an AI component. I only have one kind of AI, with no parameters, so the component won't have any data - it's just a marker to indicate that the given entity supports AI. In keeping with an overall ECS architecture, I created an AISystem that iterate through all the entities with AI and decide what to do with them.

So here's an interesting question: should the AI system explicitly skip entity 0 (the player character)? The player doesn't have an AI, but in the future, it might be interesting to introduce some kind of confusion or mind control effect. In either case, it's easy enough to include the player character in the list of entities to iterate. I just won't give it an AI component, causing the AI system to skip any action on this entity. Since I'm going to be potentially mutating the entities as I go along, I won't be able to keep a reference to the player. So before kicking off any of the AI, I'm going to first borrow a reference to the player, copy its x,y position, and then drop the reference. The player's position will be passed into the AI for distance computation.

My AI implementation was a pretty simple one based on the Python tutorial: If the player is outside the field of view, do nothing; otherwise, move until in range, and then attack.

This was actually a bit more awkward than I thought. When moving the monster, I don't want it to overlap any other monsters, which means I need to iterate through all the entities again, to see if there's anyone else in the space I want to move to. But I've already pinned the entire entities array with the monster I'm mutating, which means I can't borrow another reference. The obvious answer is to use indices instead, and borrow each time through the loop. I spent a while thinking if there's a way to do this using iterators, and I don't think so... at the end of the loop, I ultimately need a mutable reference to an entity, which means pinning the entire array. So I end up using indices. That works well enough for an array but probably wouldn't work that well for an arbitrary data structure.

Also the blocking code is now used in both the movement code and AI code. I make a note to refactor it later.

This simple AI will either move, or attack. If you're a purist, you're probably saying that this isn't right. The AI system should be setting some fields, or using an event messaging system, so that the movement or combat system can handle the respective behaviors. You're probably right. But I never let purity get in the way of simplicity or readability, so I'm going to have the AI system call the attack function directly. Admittedly, this copy and paste is pretty bad also. Since I'm not entirely sure how they'll share behavior yet, I copied and pasted, made it work, and then I'll figure out how to clean it up.

Finally, I have one last problem with combat. To compute the damage, I need statistics from the fighter components of both the player and the monster. But because I've already borrowed a mutable reference, I can't get another reference to get the statistics. So I fall back to using the same split function I used before, to split the entities array into two slices... except this time, it's more confusing because I'm also using indices. My index is from the original array, so I have to do some fudging to figure out what the index is in the new split array. I seem to be having a lot of trouble with mutating operations that require multiple elements of an array, and I don't know if I just haven't figured out the right approach yet, or if that's the expected behavior.

In any case, with this last step, I tested the AI and it behaved as expected.

Refactoring

Now that I have this working, it's time to refactor some of the rough parts.

The first thing is checking to see if a tile is blocked. There's two places that is done, once when the player moves, and once when the monster moves.

So as I began the refactoring, I realized that, although these two seem superficially the same, they're actually subtly different. When the player is moving, I need to know if it's blocked, and if so, if it's by a monster or other combat entity. When the monster is moving, that's actually not necessary, all that is needed is to know is it blocked: true or false? In this case, it doesn't actually cost extra to get the blocking entity, so that's fine, although that won't always be the case in other refactorings.

The actual bigger problem I ran into is defining the interface. What should this function return? I need the result of both whether it's blocked, and what entity is blocking it.

Since I can be blocked by a wall, I can't just return a None entity, since walls don't have entities.

I could just return a boolean, and if I'm blocked, go through all the entities again, to see if there's an entity that is doing the blocking. This goes through the entities vector twice, and even though the performance is going to be fine, it still feels a bit inelegant.

I could make it return a tuple (bool, Some<&Entity>) to return the two pieces individually. I'm not sure I find this particularly readable.

I could split this into two separate functions, wall block and entity block, and have the client call one or the other, or both, depending on which one they need. It's not the greatest interface but it's probably the simplest to understand.

When I actually tried to implement this though, I ran into some problems with the borrow checker. I tried to implement these as methods on Game, but because it's a method on Game, the compiler says it borrows the game state object itself, so I can't borrow the entities array from the game object. That seems odd, but I poked around for a bit and was unable to work around it.

I could try just returning the index... but it's at this point I feel that perhaps these two functions, even though superficially the same, may perhaps not be semantically the same. So instead of messing around more with this, I abort the entire refactor and reset. Perhaps once I get a third client, I'll have a better idea of what to do here.

And yes, I really did poke around at this for a while and then reset it in the source code. Not all design decisions prove to be fruitful, and sometimes you just have to start over.

Death detection

Back to regular feature work. There's no death detection in the combat engine right now, so both player and monster can attack each other endlessly.

The attack code only takes fighter components right now, but to handle death properly, it needs to be able to modify the entity's color and other visual element, remove its AI, etc. I mentioned earlier that I wanted the attack function to take components, and now I'm changing it to take entities instead. I don't think this is contradictory. As new features come in, designs that weren't as good before may look better, and designs that were good may become worse.

I considered one interesting alternative, which was to have a DeathSystem. So instead of having the combat code handle death, another system would be run after, that would iterate through all the entities, find the ones that have zero or negative HP and reap those. This opens up various interesting design ideas like first strike, but I think it's a bit too complicated for my design right now. It would require special casing to handle non simultaneous damage, which would be the typical user expectation in this kind of game. It might be worth considering in the future though, depending on what other game features I add.

So if I change attack to accept entities as parameters, what happens if the entities don't have fighter components? In this case, I made them assertions, which will crash the program. It is the responsibility of the caller to ensure that only fightable entities are passed in.

Here I ran into some interesting problems with the borrow checker. I can't borrow from an entity and an entity's component at the same time. What I wanted to write was: 


if let Some(ref mut player_fc) = player.fighter {
  if let Some(ref mut target_fc) = target.fighter {
    let damage = FighterSystem::attack(player, target);
    println!("Player attacks {} for {} damage, remaining ({}/{} HP).", target.name, damage, target_fc.hp, target_fc.max_hp);
  }
}

In other words, I can't borrow the components from the entity because I want to check it now and use it later, and also borrow the entity itself. I mean, I understand why this is a problem - the FighterSystem borrowing the entity could mutate the component itself. But this feels a bit easier to read than what I eventually wrote: 


if let Some(ref mut target) = target {
  if player.fighter.is_some() && target.fighter.is_some() {
    let damage = FighterSystem::attack(player, target);
    println!("Player attacks {} for {} damage, remaining ({}/{} HP).", target.name, damage, target.fighter.as_ref().unwrap().hp, target.fighter.as_ref().unwrap().max_hp);
  }
}

Now that I modified the attack function to take entities, I can check for entity death. I can't clear the components while I have the entities borrowed, so this is done in two steps: first, borrow the entity and check to see if it's dead. If it is, drop the borrows and clear the entity's component.

Incidentally, here I ran into a bug that would have been difficult to debug in other languages.

This code above assumes that the fighter component is still valid after the damage has been executed. But in my implementation, that isn't true, because entity death causes the component to be cleared. Now, in other languages, it would have been easy to simply hold onto the reference to the fighter component, and ended up modifying a dangling reference. However, because Rust is so strict on the borrow checker, I was forced to drop and reacquire the reference, and I (correctly) get a None reference the second time. So yes, I admit the code I originally wanted to write would have had a dangling pointer.

The last small thing to do with player death is to set the game state to a PLAYER_DEAD state. I didn't want the player to be able to move or do actions after they have died, but I still needed them to be able to quit! So this is just a special state where only exit (and in the future, things like reload or restart game) would be executed.

Fixing enemy drawing

If you play around with this a bit, you'll see that there's a problem with the drawing. When there's an enemy corpse on a square, and the player steps onto the same square, the corpse drawing takes precedence over the player drawing, so you see the corpse instead of the player.

It was fairly straightforward to see what the problem was by direct source code inspection. I draw all the entities in order, with the later entities drawing on top of the earlier entities. Since the player is at index 0, it always gets overdrawn by anything else. To fix this, I need to impose some kind of ordering on the drawing.

There's a couple of different ways to fix this:

Incidentally, when someone asks: why learn something outside your focus area, this is why. Techniques to solve a problem in a particular area can often be adapted to work in other problem areas.

To fix my drawing problem, I added a depth enum to each entity. Rust doesn't seem to allow me to iterate through the different enum values, so I ended up hard coding the drawing order, and iterating through the entity list for each depth. There may be a performance problem here, but with such a small entity list, it doesn't really matter, and when the entity list becomes large, I can profile it and see if I need to change the data representation or if I need to sort the list. Since this is a localized algorithmic change, it shouldn't be a big deal to defer this.

Summary

Looking back, these were a lot of changes that were made to handle actual combat. On the positive side though, I think at this point, most of the fundamental architecture has been implemented, and most of the subsequent work will be iterating and refining the pieces.

On the Rust side, a couple of things that I continue to run into:

Lastly, the code is now in a state where it might seem poorly implemenated to a new observer. For example, if I were given this program, some of my immediate questions would be things like:

In some cases, suboptimal code may be due to incompetence. But in other cases, it may simply be a quirk of evolution and design choices. And if the code is still readable, then maybe it's not worth changing until it becomes a bigger problem.

In this series: Contents, previous, next

by Yosen
I'm a software developer for both mobile and web programming. Also: tech consultant for fiction, writer, pop culture aficionado, STEM education, music and digital arts.