Wednesday, December 28, 2011

From collaborator hell to dependency-free bliss

Let's say you're a developer on a C# app and just got a batch of new requirements. Let's also say that it has a fairly simple requirement: when a user closes his or her account, the account status should be set to closed. You, as a good Test Driven Developer, write your unit test, watch it fail, then add the following code to the Account class and watch your test pass:

public void Close() {
  this.Status = AccountStatus.Closed;
}

Easy-peasy.

Many weeks later you come across a requirement that says when a user closes his or her account, the account owner's IsSpecial flag should be set. To make your latest unit test pass, you do the simplest thing that could possibly work.

public void Close() {
  this.Status = Status.Closed;
  this.Owner.IsSpecial = true;
}

Your test went from Red to Green and now it's time to Refactor. Having your Account mutate some value on another object breaks encapsulation so you Control+R & E to Extract Method then Control+R & M to Move your method to the Owner class and give it a clear name.

public void Close() {
  this.Status = Status.Closed;
  this.Owner.ClosedAccount(this);
}

That's a little better. In stead of just setting some property you're now telling the other object that something happened. It's free to handle that how it wants and the Account needn't know about it - a textbook case of encapsulation and The Hollywood Principle.

You've worked in this domain long enough to know that all kinds of things are going to happen when an account is closed. Maybe the other things should be moved to a different method? Not entirely YAGNI, but modifying the account and telling others about it are two different things so you could make the argument that they should be two separate methods.

public void Close() {
  this.Status = Status.Closed;
  this.OnClosed();
}

private void OnClosed(){
  this.Owner.ClosedAccount(this);
}

It's a judgment call but this does clearly separate things. There's one method where the account modifies itself and another where all the other objects get to react to the account being updated. You know that if you have requirements that start with when an account is closed, then the code that deals with the account should go in the Close method and code that deals with other classes goes in the OnClosed method. But wait a second - doesn't this sound familiar? There's even something built into the C# language for just this scenario: events and delegates.

public delegate void ClosedEventHandler(object sender, AccountClosedEventArgs e);

public class Account {
  public event ClosedEventHandler Closed;

  public void Close() {
    this.Status = Status.Closed;

    if (this.Closed != null)
      this.Closed(this, new AccountClosedEventArgs(this));
  }
}

Now the owner can register a callback with the account's Closed event and the account can just tell whoever's listening when it has closed. The Account class no longer needs to reference the Owner class and we can rejoice.

But something about this still doesn't quite seem right. Now instead of the Account telling the Owner, the Owner has to listen to the Account. You still have one class directly referencing and depending on another - you just switched the dependency direction. A day or two later you remember you once read something about Messaging and Domain Events so you look it up and get that started.

public void Close() {
  this.Status = Status.Closed;
  EventBus.Publish(new AccountClosed(this));
}


/* ... elsewhere ... */
public class AccountClosed{
  public Account Account { get; set; }

  public AccountClosed(Account account){
    this.account = account;
  }
}


/* ... elsewhere ... */
class AccountEventsHandler {

  private IRepository repository;

  public AccountEventsHandler(IRepository repository){
    this.repository = repository;
  }

  public void Handle(AccountClosed e){
    this.repository.GetOwner(e.Account).IsSpecial = true;
  }
}


/* ... elsewhere ... */
public void WireEverythingTogether(IRepository repository){
  var accountHandler = new AccountEventsHandler(repository);
  EventBus.Subscribe<AccountClosed>(accountHandler.Handle);
}

Much nicer. Not only is the dependency completely removed from the Account and Owner classes, but each class no longer exposes methods that are just used for reacting to each other. The Account only modifies itself and creates events and the Owner only modifies itself and creates events. Now you have the Account depend on the EventBus and somewhere else you subscribe other classes to the appropriate events. You have a simple AccountEventsHandler that deals with coordinating other objects when the account changes so the domain objects themselves don't need to. It can be little bit more work, but it does reduce coupling and allow much cleaner domain objects. But the Account is still referencing the EventBus directly. You could use a service locator:

public void Close() {
  this.Status = Status.Closed;
  Services.Get<IEventBus>().Publish(new AccountClosed(this));
}

But the service locator can cause problems - like if an IEventBus wasn't registered - and is considered an anti-pattern by some people. You could use constructor injection to pass in the dependency instead.

public Account(IEventBus bus){
  this.eventBus = bus;
}

public void Close() {
  this.Status = Status.Closed;
  this.eventBus.Publish(new AccountClosed(this));
}

That's clean and a good use of the Dependency Inversion Principle, but maybe overkill since the EventBus is so simple and ubiquitous. Maybe hard coding a such a simple dependency is ok. In my limited experience this is an acceptable time for bending the rules and relying on a statefull static class. You will probably never hear me say that again since I hate working with static classes and singletons that have state or complex behavior. On the other hand, constructor injection is a great first step to using a Dependency Injection Container to wire up the EventBus and other dependencies.

Maybe months from now you can switch to actual full-scale event sourcing and CQRS. With command handlers, the Domain Objects create events and store them locally. Then the command handler would fetch all the events from the domain objects and pass them to the event bus; your domain objects don't even need to know about the event bus anymore since the command handler deals with that.



This series of slightly different ways of dealing with different objects involved in the same action is something that I've had a lot of problems with. It's all the same basic result but the important difference is in how to deal with coordinating different objects when something happens. These different designs have very different benefits and costs. The earlier examples are easy for anyone to do, easy to see exactly what happens when debugging or looking at the code, and you know exactly where to make the change: right in the method itself. The downside is that it becomes collaborator hell and all your classes quickly become so bloated that most of the class's code is actually about other classes. The classes become so interdependent that you can't change one thing without having to change everything else. Having lots of dependencies like this can also make build times suffer too. The latter examples only rely on the Event Bus, Service Locator, or Inversion Of Control Container so they are as non-dependent as possible but I can't really tell what's going to happen when the program runs. If everything is wired together with configuration in xml files or sql tables then it can also become exceedingly tedious and error prone to change. This kind of configuration becomes especially difficult if the developer's environment, qa environments, and production environments can all be configured differently. Not everyone will be familiar with or even like the event-based designs either. The whole thing is made even more confusing if different sections of the project have different ways of doing things and people mostly just do whatever they copied and pasted from - not that I've ever done that of course....

Sunday, December 4, 2011

Randomized metroidvania update: simple fix

I decided to take a break from skeletal animation and focus more on the room generation for the Metroidvania game. While implementing architecture and art for underground caves, I think I found out how to fix the problem where transitioning into a new room would "bump" you to an unexpected place. I say that I "found out" instead of "figured out" because it was really just a lucky observation and a lot of trial and error.

Why did it take me so long to figure that out? Well, the problem seemed to only happen when transitioning from one room to another room, while on the floor (i.e. not jumping), where the new room's ceiling was lower on the world map than the room you were transitioning from. Confusing enough? Even then, it only happened some of the time. After a lot of logging, I noticed an odd pattern: the player's new y coordinate would be slightly higher than expected (by less than a single pixel). After all that, here's the fix:

private function checkBounds():void
{
    /* code to figure out the new room and new player.x and player.y values */

    if (toRoom.mapY > currentRoom.mapY)
        player.y -= 1; // gravity causes weird things sometimes....   
}

Although I'm glad to finally have this fixed, as far as I can tell at least, I'm still annoyed by it. What annoys me most is that I have no idea how I could have prevented this and no idea how to even figure out this fix. It was a lot of observation and trial and error. There were at least a dozen previous attempted fixes that didn't fix anything. I still don't fully understand why having a y coordinate of 144 is ok but one of 144.00115 causes you to fall to the room below and have a different x coordinate. I don't even know how to write a unit test for this: it involves Flixel physics, Flixel collision detection, and the constantly changing algorithms that I use to create rooms. Could TDD have prevented this? Is there a unit test that reliably shows what the bug was?

Tuesday, November 22, 2011

Skeletal animation with actual images

It took quite a while because I was busy with other things and because I tried to do too much at once. Once I broke the problem down into smaller pieces, things quickly fell into place. I had to rewrite most of the underlying bones and joints but I eventually got the skeletal animation to work with actual graphics. Here's a half-sized Galamoth from C:SOTN.




I completely got rid of bones and just use FlxSprites now. Each FlxSkeletonJoint is responsible for how the parent and child are positioned relative to each other. Joints do this by attaching the "ball" point of one sprite to the "socket" point of another at a given angle. Each joint can still have it's own set of animations. The FlxSkeleton itself just holds the bones and joints and helps assemble and detatch them.

Next is basic inverse kinematics - just enough to support things like "look at that" or "put your right hand here". That should be good enough for what I need.

Monday, November 14, 2011

Skeletal animation with basic physics

Skeletons are now FLxObjects and not FlxGroups. There's a few other minor changes but that's the big one. You can see the bounding box by toggling debug mode by pressing [~] then clicking the bounding box icon in the upper right.

Press [w] to jump, [a] or [d] to walk, hold [shift] to run, and [k] to kill yourself. There's not really anything to do here - just jump around and self-kill when bored.




Completed this time:
  • Make sure skeletons play well with Flixel physics (collisions, velocity, acceleration, etc.)
  • Detaching bones (exploding skeletons!)
  • More animations for HumanoidSkeleton
Next up:
  • Make sure regular sprites and animated sprites can be used as bones
  • Constraints for joints
  • Ragdoll physics

Friday, November 11, 2011

Skeletal animation with forward kinematics

I'm off to a good start with my little detour into skeletal animation: I've got bones and joints that can be animated. The joints use keyframes to hold the target angle of the joint and interpolate between the current angle and the target angle so switching animations should smoothly transition from one to the other. I had a bit of difficulty since rotating a FlxSprite rotates about the center and I need to rotate around the joint. I also wasted nearly an hour on a really stupid bug where animating a joint would sometimes make it freak out (hint: -45 as a Number and -45 as a uint are two very different values). Once I'm done, I plan on submitting a pull request to the original Flixel project so maybe the next version of Flixel will support skeletal animations....




Done so far:
  • FlxSkeletonBone - a FlxSprite with a start and end point
  • FlxSkeletonJoint - a class that connects bones at an angle
  • FlxSkeleton - a FlxGroup that wires everything together
  • HumanoidSkeleton - a pre-made human skeleton with some generic animations
  • Keyframe based animations per joint
  • Simple forward kinematics

To do:
  • Make sure regular sprites and animated sprites can be used as bones
  • Make sure skeletons play well with Flixel physics (collisions, velocity, acceleration, etc.)
  • Constraints for joints
  • Ragdoll physics
  • Detaching bones (exploding skeletons!)
  • Add inverse kinematics
  • More animations and helpers for HumanoidSkeleton
  • Walking velocity determined by feet positions?

Monday, November 7, 2011

Randomized metroidvania 09: slightly better architecture

It took a few days longer than I'd like but I was able to implement some new stuff:
    Slopes and stairs based off of https://github.com/krix/SlopesTest/blob/master/src/FlxTilemapExt.as
    Different background colors, patterns, and decorations (just columns for now)
    Different art and architecture preferences for each region




Each region gets a few architecture and art preferences. The architecture preferences are used to determine the room generation: what tiles are empty, solid, or stairs. The art preferences are used to determine the graphics: what do the background, stairs, walls, and floors look like. There's still not much content but I have a simple system set up and I can add more and better art or architecture styles later. Alpha content will suffice for now.

There's still a bug where moving into another room sometimes bumps you to a different space. I can't pinpoint where or why though. It seems to only bump you towards the bottom of the map and never the top. It also seems to bump to the left and never the right. So it bumps up the Y axis and down the X axis.... weird.

I think it's time to focus on player graphics for a while. Since I'm better at programming than art, I think I'll try my hand at skeletal animation. Flixel doesn't have anything like this as far as I can tell but forward and inverse kinematics should be fun and make a lot of the art stuff easier too. I estimate about a week until I have something useful.

Friday, November 4, 2011

Randomized metroidvania 08: initial architecture

I've got some initial architecture. It should now be possible to move from any room to any room. It certainly ain't pretty and is sometimes frustrating to get the jumps but it does allow you to get from one room to the next. I also added restricted regions to the world generation so the resulting map won't be as dense.


[it seems that transitioning from one room to the other sometimes "bumps" the player to a different spot... worked fine on my machine. I'll look a little closer into that code.]

Part of automating the process of creating room internals was changing the screen size to 32x21 tiles. Next up is adding sloped tiles for stairs, more variety, and more randomness to the architecture.