Codifying some facts about pulling things

I was travelling and wasn’t able to hack code as much as I would liked to (and on top of that, I wanted to play some computer games too). But I did manage to push next feature forward just a little bit and write some BDD specifications for it. It’s a bit odd to write BDD specs for a single person project, but I view doing that as a good training and a nice favour to my future self.

I’m using behave for writing BDD specifications. It’s relatively straightforward and easy to get started. It can also handle fairly complex cases, although I prefer keeping things as simple as possible.

Every .feature file has a little header that tells name of the feature and some justification for its existence:

Feature: Pulling things
  as an character
  in order to modify me environment
  I want to pull both creatures and items

After that, there are one or more scenarios that detail common use cases for the feature. One can abstract common parts of scenarios to something called background, but nowdays I try to avoid that. I think it’s useful to have whole scenario spelled out clearly, in easy to read format. If I’m using backgrounds, I have to jump back and forth in file and that gets tedious.

Simplest happy-path scenario I could think of is below:

  Scenario: Pulling smaller character
      Given Pete is Adventurer
        And Pete is standing in room
        And Uglak is goblin
        And Uglak is standing next to Pete
       When Pete pulls Uglak
       Then Uglak is where Pete was
        And Pete is still next to Uglak

Pete and Uglak are standing in a room and when Pete pulls Uglak, they both move to same direction. Simple enough (I’m not even sure if this is too simple specification, but it’s a start). Often I start coding new feature by writing down bunch of these to clarify what I’m actually about to code. In this case, I wrote 4: pulling smaller character, pulling larger character, pulling somebody into a trap and pulling smaller items.

When I run behave, I get bunch of errors that I have undefined steps. This means that behave has encountered steps that it doesn’t know how to handle. Being a well behaving tool, it will output me sample definitions to get started:

@when(u'Pete pulls Uglak')
def step_impl(context):
    raise NotImplementedError(u'STEP: When Pete pulls Uglak')

These sample steps are meant as easy way to get started. They don’t handle any parameters and only raise NotImplementedError. After defining all of them, I’ll still get bunch of errors, but this time because of the raised error:

Failing scenarios:
pyherc/test/bdd/features/pulling.feature:6  Pulling smaller character
pyherc/test/bdd/features/pulling.feature:15  Pulling character into a trap
pyherc/test/bdd/features/pulling.feature:24  Pulling larger character
pyherc/test/bdd/features/pulling.feature:33  Pulling smaller items

Step implementation for pull action is shown below. This function gets called when behave encounters string “pulls” that has some free-form string before and after it. These free-form strings are passed in the actual function as named parameters and can be used to control what the step actually do. In this example they’re of course names of our two brave characters: Pete and Uglak.

@when(u'{actor_name} pulls {target_name}')
def step_impl(context, actor_name, target_name):
    actor = get_character(context, actor_name)
    target = get_entity(context, target_name)
    add_history_value(actor, 'location')
    add_history_value(target, 'location')
    pyherc.vtable['\ufdd0:pull'](actor, target)

Since characters, items and generally anything under test are referred by name and not by instance, first step at the beginning of this step is to retrieve correct objects from context parameter. Since I know that I’ll be interested on knowing how these objects moved, their original location are recorded with add_history function. This simply copies value of given attribute and stores it for later use. Last step is to actually call the pull function to make objects hopefully move around.

Final step to get our pieces in place is to add the new action (which will fail because of the assert):

(defn+ pull [character target]
  "character pulls target that can be either another character or item"
  (assert false "not implemented"))

Now we have first set of BDD specifications that are actually failing because of the implementation and not only because lack of some scaffolding. After this point it will be just the regular cycle of hacking until more tests/specifications pass, cleaning up a bit and moving to next error.

pull function follows the true and tried pattern I have used for other actions too (maybe I should write a macro to capture the pattern?):

(defn+ pull [character target]
  "character pulls target that can be either another character or item"
  (if (call pull-legal? character target)
    (do-monad-e [direction (direction-to-pull-m character target)
                 character-location (location-at-direction-m (. character location)
                 target-location (location-at-direction-m (. target location)
                 _ (move-character-to-location-m character
                                                 (. character level))
                 _ (move-character-to-location-m target
                                                 (. target level))
                 _ (add-tick-m character Duration.slow)]
                (Right character))
    (do-monad-e [_ (add-tick-m character]
                (Left "pulling wasn't legal"))))

First legality of the action is checked (legal action isn’t sure to succeed though, this check just removes some completely illegal situations). If action isn’t legal, time passes for the character so it doesn’t try to act immediately again and error message is returned. In case of legal action code figures out direction where to move character and target, moves them and updates time of the acting character. In case any of the steps or their sub-steps fail, appropriate error message is returned.

What I’m not completely sure about this approach is where to raise domain events (complete separate, yet equally interesting question is when they should be raised). Currently move-character-to-location-m is responsible of moving character to a given location in level and raising appropriate event so that UI knows to update its state. Other creatures might be interested on these events too, in case they’re tracking player or something fancy like that. The problem is that event being raised is move-event, which simply tells that a character has moved to a new location. In case something is being pulled, pull-event might make more sense. However, the code is shared between move and pull functions.

I could raise event from the same level where move-character-to-location-m is raised, but that creates several other problems that I’m not really willing to tackle now. Remember to raise events is bad enough, but having to tackle with correct order of events that might be raised from deep function hierarchy will be a nightmare. I’m already semi-seriously considering making some combination of IO and Either monad with extra filtering/replacement routines built into it. Then called code could raise all the events they want, but they would be filtered and processed all together in the end, before any events are raised. If this sounds complicated, it probably is (the reason why I haven’t started with this yet).

While coding this feature, I started wondering how to organize my code (that seems to happen fairly often). Every action is broken into two functions: one to check if given action is legal to perform (which doesn’t necessarily mean it’s possible to perform) and another to actually perform the action. The latter one is split into more fine-grained functions that I try to keep reusable.

Having bunch of reusable functions is fine and all, but they often raise another interesting question: where to put those functions? Moving and pulling both deal with setting characters location to something and possibly triggering all the traps in that new location (among other things, like tracking if they have entered a new level and such). Originally it might have made sense to place this code together with rest of the movement code, as it was the only place where it was needed. After pulling entered to picture, this changed. One could still keep the code there and reference it from the pulling code. This hopefully wouldn’t lead into any circular references. Or, one could also find some other place and move the code there.

For now I have decided to keep the movement code where it is and just refer it from the pulling code. It’s not too ugly of a situation and I can always move the code somewhere else if I later decide to try something else.


Leave a Reply

Fill in your details below or click an icon to log in: Logo

You are commenting using your account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s