Kotlin 62: Room Actions

GitHub Repo

Let’s make some progress toward Room actions. Some fairly nice refactoring and feature addition in here.

Hello! It’s 0909 here at the Jeffries-Hughes hacienda¹, and while I have a small idea for something to do, I’m not sure whether we’ll do that or find something else. There are certainly some infelicities² in the code, and improving them might be a worthy task. We’ll review and think.

The Idea

This one is p-baked for p about 0.25. We’re planning to have two Actions maps, one in the World and one in the individual Room. (Which means there are many maps, but only two are of interest, world and current room.) The basic scheme is that we’ll get a Phrase and first try a lookup in the Room’s actions. If the Room doesn’t have an action that it wants to apply, we’ll pass things to the World’s actions.

This is going to put some constraints on the Actions already, since if a Room’s Actions were to capture all commands, that might be bad. (Then again, we could probably imagine a really frustrating Room where nothing works unless you say “Simon says” or something.) Additionally, there might be a Room that wants to accept some command, do something, say something, and then defer to the World to do the rest of the command’s execution.

I can imagine that it could get even more weird, where we’d want prefix room commands, suffix room commands, all kinds of options. For now, I’m trying hard not to build that, or even think much about it, because we have no identified need in the game. But it does seem prudent to allow for a Room’s action deciding to execute the World’s corresponding action … or even a different action?

So the nascent idea that I have is that there might be some kind of a little object containing pointers to world and room, and both action tables, and enough methods to allow for at least reasonable hand-off between room and world’s actions. Sort of an “action context”. Just a fraction of an idea, but the current situation needs to change.

Review

Where are we, anyway? We’ve cleaned up the command a fair amount, and arranged that the Imperative is just created at the last possible moment:

    fun command(command: Command, world: World) {
        world.response.nextRoomName = roomName
        val phrase: Phrase = makePhrase(command, world.lexicon)
        val imp = Imperative(phrase,world, this)
        imp.act(world.lexicon)
    }

    private fun makePhrase(command: Command, lexicon: Lexicon): Phrase {
        val factory = PhraseFactory(lexicon)
        return factory.fromString(command.input)
    }

Ah. I think I see something that definitely needs doing. Let’s remember that we want to try our commands in the Room’s actions, and only after it rejects them, in those of the World. I think it makes sense to assume, for now, that the imperative has the right things, the Phrase, the World, and the Room. But the imperative is presently told to act and given a lexicon. Let’s follow that code:

class Imperative ...
    fun act(lexicon: Lexicon): String {
        lexicon.act(this)
        return testingSaid
    }


class Lexicon ...
    fun act(imperative: Imperative) = actions.act(imperative)


class Actions( ...
    fun act(imperative: Imperative) {
        val imp: (Imperative) -> Unit = find(imperative)
        imp(imperative)
    }

The name there in Actions.act. It shouldn’t be imp, short for imperative. It’s the action found by the find function. Rename that right now.

    fun act(imperative: Imperative) {
        val action: (Imperative) -> Unit = find(imperative)
        action(imperative)
    }

Test, commit: tidying.

OK. Anyway, it seems to me to be clear most of this forwarding is now mistaken. It might have made sense to send the the act message to the Imperative: it’s the object in hand. But passing it the Lexicon and plunging down through there, no, I don’t think we can tolerate that. We need to pass in the specific Actions that it should work on. And, arguably, we should be asking the Actions to do the Imperative. The choice isn’t obvious to me.

Dirty Trick?

A dirty trick has come to mind. The tests are probably full of calls that pass in a lexicon. Let me verify that. Yes. There are in fact 20 such calls in the tests. I’d like to be able to change this without modifying all those tests before I’m back to green.

This is a common situation: we want to do something simple and sensible, but if we do, a raft of tests break. So we don’t make the improvement, and work around it, leaving our workspace just a bit more messy than it could be. This slows us down, inevitably.

In Room, we are asking the Imperative to act, giving it a lexicon. We want to give it an actions table instead. Suppose we were to implement two methods on Imperative, one receiving a Lexicon, one receiving an Actions map, and forwarded the Lexicon one to the Actions one? The tests would continue to work.

Let’s try this, and discuss whether it was dirty later, after we see whether it works.

Here’s Imperative.act again:

    fun act(lexicon: Lexicon): String {
        lexicon.act(this)
        return testingSaid
    }

Just for recall, note that the Lexicon does this:

    fun act(imperative: Imperative) = actions.act(imperative)

Our call above is the only usage of that method in Lexicon. Let’s unwrap it and do it inside Imperative:

    fun act(lexicon: Lexicon): String {
        lexicon.actions.act(this)
        return testingSaid
    }

This should run green. Yes. We can remove Lexicon.act. Still green. Commit: Imperative act(lexicon) pulls actions from lexicon and sends act, skipping lexicon forwarding.

But now, back in command:

    fun command(command: Command, world: World) {
        world.response.nextRoomName = roomName
        val phrase: Phrase = makePhrase(command, world.lexicon)
        val imp = Imperative(phrase,world, this)
        imp.act(world.lexicon)
    }

Let’s posit a new method on imperative, also act, that accepts an Actions as its parameter. We code:

    fun command(command: Command, world: World) {
        world.response.nextRoomName = roomName
        val phrase: Phrase = makePhrase(command, world.lexicon)
        val imp = Imperative(phrase,world, this)
        imp.act(world.lexicon.actions)
    }

Of course IDEA and Kotlin disapprove: there is no such method. But they also volunteer to create the method. I allow it. It sets me up like this:

    fun act(lexicon: Lexicon): String {
        lexicon.actions.act(this)
        return testingSaid
    }

    fun act(actions: Actions) {
        TODO("Not yet implemented")
    }

Pretty clear what this has to be:

    fun act(actions: Actions) {
        actions.act(this)
    }

Test, expecting green. Commit: new method act(Actions) on Imperative.

Forward the lexicon method to the new method.

    fun act(lexicon: Lexicon): String {
        act(lexicon.actions)
        return testingSaid
    }

Test again. Green again. Commit: act(lexicon) calls act(actions)

A warning tells me that Imperative.setNoun is no longer used. Don’t ask questions, just remove it. Test. Green. Commit: tidying, remove unused setNoun.

Head’s Up! Reflect!

Was that a dirty trick, or not? By leaving the act(lexicon) method in Imperative, we kept 20 tests running. That allowed us to do five commits, five sets of changes, green to green, in less than a half hour. And we were able to keep our mind on what we’re really up to, rather than distract ourselves wondering how to fix the tests. I prefer to work without distracting myself, even when the test changes are simple, which I expect most of these will be. I still need not to get distracted, lest I lose the thread of my plan.

I think that since the code was actually simplified, it was a righteous change, but it does have the interesting case of a method name with two different parameter lists. We could change that by renaming one or the other method, but I’d rather allow multi-signature methods as part of our coding standard, so I’d like to get used to them.

Did you notice that weird return testingSaid in the act(lexicon)? That’s a hack that was put in to allow tests to determine whether the right action was selected. We now know that no one is calling that version of the method other than tests: there’s only exactly one call to act in the game itself. We could rename that method to testingAct. Should we? Despite my willingness to permit multi-signature methods, I think our code would better communicate if we were to rename that method, and IDEA will happily do it. Here we go:

    fun actForTesting(lexicon: Lexicon): String {
        act(lexicon.actions)
        return testingSaid
    }

IDEA changed 29 calls for me. I’m starting to like this system. In fact, let me digress to talk about my current feelings about Kotlin and IDEA.

Kotlin and IDEA Are Fine

There’s a lot to like about the combination of Kotlin and IDEA. I like the language itself. It’s much more clean than Java, and overall more compact. I think it’s easier to build up an intuition about how it’s going to work than it is with Java. Kotlin seems to hide away some of Java’s, um, oddities.

IDEA is really helpful. I’m coming from a very weak IDE in Codea. I do like that system and language, and it’s marvelous given that it’s on the iPad, but IDEA is in a different class.

Perhaps my favorite IDEA feature is Command-B. Put the cursor on a method call, and Command-B takes you to the definition. Command-B again gives you a list of all the calls to that method. If, instead, you want to trace how that method works, set the cursor on the method you want to see next, Command-B again takes you there. Drilling down through calls, as you saw above where we were following act, is just a couple of keystrokes.

Another nice one is that two quick taps of the shift key brings up a search that will find anything you search for, apparently in the entire universe, not just your program. If you have something selected, it’ll be searching for that. I use this one to find things like error messages, to see where they come from.

Rename a method? Shift-F6. F6 moves things.

Command-T pops up a list of refactorings that apply to where your cursor is, or to what’s selected. You can extract a value or method, or inline a variable. I use this one, when I’m in a learning mood, to find out what the keystroke is, if there is one, and then I escape the menu and do the keystroke, trying to drill it into my brain.

Oh, and multi-cursor editing. Are you serious? select something, control-G selects the next copy of the thing, repeat until there’s a cursor on all of them, then your arrows and option arrows and such all do the right thing in whatever line that cursor is on. Really nice for wrapping things, adding a parameter, or the like.

I’ve been a fan of IntelliJ for years, but for years I’ve not been working in a situation where I could use their tools. I’m glad to be back, and as I gain comfort I am more and more pleased. I’m having fun with IDEA and Kotlin, and impressed with both.

End Unsolicited Praise.

Back to Work

OK, where were we? Ah, yes, we have imperatives and actions arranged so that we can readily ask an imperative to use any set of actions we may have to hand, in particular, the room’s or the world’s.

The question before us, I think, is to work out how to actually do this.

And we really ought to do it with some tests, don’t you think? I have the feeling that there may be a start at this.

Yes, the Room DSL understands action:

    // DSL Builders

    fun action(verb: String, action: Action) {
        actions[verb] = action
    }

A quick Command-B will take me to whoever is doing this. It turns out that there’s one usage in the actual game world, but it doesn’t do anything, because we aren’t using the room’s actions.

I guess I have to make up a test. Here goes. I get this far:

    @Test
    fun `room action`() {
        val world = world {
            room("busy") {
                desc("you are in a busy room", "you are in a very busy room")
                action()
            }
        }
    }

Then I realize that the new scheme is to provide a Phrase, and this action method is expecting a verb (String). Let’s review how we define actions elsewhere.

Shift-Shift and type “makeActions” to find:

    private fun makeActions(): Actions {
        return Actions(mutableMapOf(
            Phrase("go") to { imp: Imperative -> imp.room.move(imp, imp.world) },
            Phrase("say") to { imp: Imperative -> imp.room.castSpell(imp, imp.world) },
            Phrase("take") to { imp: Imperative -> imp.room.take(imp, imp.world) },
            Phrase("inventory") to { imp: Imperative -> imp.world.showInventory() },
            Phrase() to { imp: Imperative -> imp.room.unknown(imp, imp.world) }
        ))
    }

Right. So we’ll want our action method in Room to be similar. I think this won’t be convenient, but it’ll be a start. I come up with this, which is awkward but may work.

class Room(val roomName: String) {
	...
    private val actionMap = mutableMapOf<Phrase,Action>()
    private val actions = Actions(actionMap)

    // DSL Builders

    fun action(phrase: Phrase, action: Action) {
        actionMap[phrase] = action
    }

That should let me continue with my test …

    @Test
    fun `room action`() {
        val world = world {
            room("busy") {
                desc("you are in a busy room", "you are in a very busy room")
                action(Phrase("look","around"),
                    {imp-> imp.world.say("Lots happening")})
            }
        }
        val room = world.unsafeRoomNamed("busy")
        val response = GameResponse()
        val command = Command("look around")
        room.command(command,world)
        assertThat(response.resultString).contains("Lots happening")
    }

This ought to fail with the message not being there, but I expect it to run otherwise. It turns out I have to comment out the version of action in the world example, which uses a string rather than phrase. I could have fixed it but commenting is easier.

Test fails, but not as I had hoped. Instead:

lateinit property response has not been initialized

Setting up these world tests is tricky. What have I missed.

Aside

This, plus some interruptions here at home, is derailing me. I am feeling tense and not on top of things. But I do think I’m on the right track. What’s going on here, I think, is that the test setup is just too intricate to be allowed to exist, but I’ve been too lazy to work on it.

The only difference that I see is that I tried to bypass the world and call to room. I guess World.command does something useful. Let’s look just to firm it up in our mind, even though probably just calling it would be fine.

    fun command(cmd: Command, currentRoom: Room, newResponse: GameResponse) {
        response = newResponse
        currentRoom.command(cmd, this)
        response.nextRoom = roomNamedOrDefault(response.nextRoomName, currentRoom)
    }

Right, it wants to tuck the response away. That stuff really needs to be untangled, but we are on another quest just now. Change the call:

    @Test
    fun `room action`() {
        val world = world {
            room("busy") {
                desc("you are in a busy room", "you are in a very busy room")
                action(Phrase("look","around"),
                    {imp-> imp.world.say("Lots happening")})
            }
        }
        val room = world.unsafeRoomNamed("busy")
        val response = GameResponse()
        val command = Command("look around")
        world.command(command,room, response)
        assertThat(response.resultString).contains("Lots happening")
    }

This better give me the error I expect. It does:

Expecting:
 <"unknown command 'look around'
you are in a very busy room
">
to contain:
 <"Lots happening"> 

We see that we’ll need to work out a signal on whether to call the world’s actions or not, but for now, let’s put this into room.command:

    fun command(command: Command, world: World) {
        world.response.nextRoomName = roomName
        val phrase: Phrase = makePhrase(command, world.lexicon)
        val imp = Imperative(phrase,world, this)
        imp.act(actions)
        imp.act(world.lexicon.actions)
    }

We’ll just call the room’s actions all the time. They’re usually empty so I have no real sense of what will happen. I’m afraid a bunch of tests will fail, but I think we can fix that readily.

Test, paying attention just to ours.

LOL. The room action test passes. Thousands of others fail, saying:

Key Phrase(verb=null, noun=null) is missing in the map.

These are all due to the attempt to find things in an empty map. I see a couple of ways around this. One is to ensure that the room’s actions always include the empty phrase. Let’s try that first. (The other way would be to check in actions for an empty table. this seems slightly more elegant.)

private val actionMap = mutableMapOf<Phrase,Action>(
        Phrase() to {}
    )

This creates a lot of little maps, one per room, but who’s counting? Let’s see if we can get the test green. Yes, we are totally green. However, let’s break the test to see the output: it’s not quite what we want:

Expecting:
 <"Lots happening
unknown command 'look around'
you are in a very busy room
">
to contain:
 <"Lots xxxhappening"> 

As I anticipated, the “unknown command” message comes out, because we are calling out act on both the room actions and the world actions. We need some way to communicate between the two action sets. It seems that the usual case should be that if the room handles the command, the world should not get a shot, and if the room doesn’t, then it should.

However, our test is green, even though the feature is not yet live. Let’s set it back and commit. A release at this point would be perfectly safe. Commit: Initial room actions in place. world actions always done after room actions.

It’s 1109, so I am exactly two hours in. Let’s take a break.

Summary

We only added one commit in the last half hour. Not great but we had things to do. The steps were all still small, and we never left the system broken for more than a few minutes at a time. And there were places in there when I could have committed, but I wasn’t certain yet of just how things should be done, so while I might have made a save point, I felt it was too likely that I’d want to roll back.

I take a save point when I’m pretty sure I’ll keep going forward, and I don’t when I think I might need to roll back. That lets me always work right at HEAD, and I never have to back up more than my current typing. If I were to have to back up several commits, I’d feel bad for having been off track for so long.

We have an actions table in each room, and it has the standard format. We’re executing its commands when it has them, and doing nothing when it doesn’t. Then we execute the world’s commands as well. We probably want to rig up a way for the first result to condition what the second call does, or whether it even happens at all. Probably the most common case will be room handles, and world doesn’t, but it’s also likely that we’ll want both to handle a command, and perhaps even for the room to return a new command to be done instead of the original.

I believe we’ll want a little object with both sets of actions, and that we’ll probably provide some way for our Action lambdas to communicate through that little object. Right now, I don’t see how we could do that without changing a lot. We’ll find out. Perhaps our lambdas just need another parameter.

For now, good progress, rooms have actions. Check with me next time!