Up-front Design? In Agile? Sure, let’s do some. I call it ‘thinking’.

We’re in pretty good shape with the string definition of Decor, which parses down to some ParseElements, each of which can define a Tile or Loot or Decor. A Decor has a Tile and a Loot. A Loot has a Tile, which can be nil and will be until the Decor rezzes the Loot upon being bumped.

With that much in hand, we’re also in good shape to build up arrays of these definitions, so that we can fully specify the Decor in a dungeon level. I think we can almost imagine the Level Designers beavering away, producing these lists, and telling us to install this list here on dungeon level such and so. It seems likely that the lists will be placed in or near the DungeonBuilder. In some larger-scale impllementation, the lists might be read from files or something like that.

But I’m thinking about some design issues. We will have other arrays, not just this Decor kind. We’ll probably have Monster ones, maybe possibly someday, and we’ll surely have Loot ones, if only to prove that we can do it. The Loot ones, if we stick with Loot being created separately from Decor, would just have two elements, the Loot identifier and a Tile. (Also on a card somewhere, Decor can have a “pain”, an optional damage property that bites you when you go messing around with it.) So the string definitions will be different for different DungeonObjects that we might wish to create.

That could even imply a more complex string, and thus a more capable parser. That doesn’t bother me, the parsing is all nicely packaged up in the Parse object. What does kind of bother me is: who does which parts of the work?

Right now, DungeonBuilder is in essence a dungeon factory. As such, it knows how to build everything, at least down to some level of detail. The customizeContents method tells the tale:

function DungeonBuilder:customizeContents()
    self:placeSpikes(15)
    self:placeLever()
    --self:placeDarkness()
    self:placeNPC()
    self:placeLoots(10)
    self:placeDecor(30)
    self:placeThings(Key,5)
    self:placeWayDown()
    self:setupMonsters()
end

And in placeDecor we see the issue in more detail:

function DungeonBuilder:placeDecor(n)
    local sourceItems = {
        "catPersuasion",
        "curePoison",
        "pathfinder",
        "rock",
        "health",
        "speed",
        "nothing",
        "nothing",
        "nothing"
    }
    local loots = {}
    for i = 1,n or 10 do
        local kind = sourceItems[1 + i%#sourceItems]
        local loot = Loot(nil, kind, 4,10)
        table.insert(loots, loot)
    end
    Decor:createRequiredItemsInEmptyTiles(loots,self)
end

And finally we defer off to Decor:

function Decor:createRequiredItemsInEmptyTiles(loots, runner)
    return ar.map(loots, function(loot)
        local tile = runner:randomRoomTileAvoidingRoomNumber(666)
        return Decor(tile,loot)
    end)
end

Compare this to what we currently have in our test for the decor definition strings:

        _:test("Decor from definition string", function()
            local def = "Skeleton1;health(5,10);avoid(1)"
            local parsed = Parse:decorDefinition(def)
            _:expect(parsed.valid).is(true)
            local decorKind = Decor:legalKind(parsed.decor.kind)
            _:expect(decorKind).is("Skeleton1")
            local lootElement = parsed.loot
            local loot = Loot(nil, lootElement.kind, lootElement.min, lootElement.max)
            _:expect(loot:is_a(Loot)).is(true)
            _:expect(loot.kind).is("health")
            _:expect(loot.min).is(5)
            _:expect(loot.max).is(10)
            local builder = GameRunner():defineDungeonBuilder()
            builder:buildTestLevel(10)
            local re = parsed.room
            local tile = builder:findTile(re.kind,re.p[1])
            _:expect(tile:isRoom()).is(true)
        end)

In the existing implementation, we calculate some Loots and then tell a factory method in Decor to do the rest. In the new implementation, in the test, we have an input string that tells us a particular combination of Decor, Loot, and Tile to create. Decor will be happy enough to do that for us, even if we need to give it a new constructor.

But as it stands, once we parse the string, we pull out the various components (which are known in the parser to be in a known order), and then we call a unique method for each. For Decor, it’s legalKind, for Loot it’s just the Loot standard constructor, and for the Tile, we use the Builder itself to find a suitable one.

Imagine that test was the method to create our Decor instances: it’s very close if we take out all the checks. What class should contain that method? Its job is to create Decor. Should it be a Decor factory method? But it knows some pretty weird details of how to find a Tile. It also knows how to create a Loot, which isn’t too big a deal.

Point is, wherever this code sits, it knows an awful lot of detail for Decor, Loot, and Builder, and, indirectly, Tile. So the design question is: where should this soon-to-be-written Decor factory method reside? Should it be in Decor, because the string is in Decor format? Should it be in Builder, because it calls the Builder’s room-finding Code?

Should it be passed around from object to object, until finally we have a fully-defined Decor?

We could spend the whole morning thinking about this, especially if we could rope a couple of colleagues into the discussion. We could draw pictures and browse code. If there is a good answer, it isn’t obvious, because we’ve thought about it enough already to get any obvious answers.

When a design question’s answer isn’t obvious, there are at least two things we can do. We can spend as much time as it takes to answer the design question. Or we can just make an expedient decision. What may surprise you—though it shouldn’t—is that I’m going to recommend making the best quick decision that comes to mind.

Will this lead to an inferior design? Absolutely, compared to some idea that we don’t have. But it will also be the best design available right now. This is OK, for two reasons.

First, if we never need to create another factory kind of thing like this, then this implementation is as good as any other. And, second, if we do need to create another one, we’ll surely copy this style, which will leave both cases the same, which isn’t all bad. And third—I lied about the two—with two examples in hand, there’s a good chance we’ll see an even better answer, and I trust us to refactor to that better answer.

Now if I didn’t trust us to move to the better decision, I’d still choose the best idea available right now, because copying the idea later will still be reasonably sensible.

So, there you go, up front design in my style, which is to think a bit and then go with the best idea I have. Today the best idea I have is:

Leave the construction code in DungeonBuilder, and wait to see what happens.

Let’s get to the next step.

What IS the Next Step??

Well, we have a test that says that given one of these string definitions, we can create all the elements of the defined Decor. I guess next we need to create a lot of them. Let’s do that in a test and then use that to create a DungeonBuilder method.

        _:test("Collection of Decor", function()
            local defs = {
                "Skeleton1,health(5,5),avoid(1)",
                "scoobie,foo,bar",
                "Chest,catPersuasion,avoid(1)"
            }
            local decors = fromDefs(defs)
            _:expect(#decors).is(2)
        end)

Here I’ve created what I believe to be too good definitions and one bad one, and I’m imagined a new function, fromDefs that will create the Decors. I expect 2 of them, and plan to check more deeply when I get them. Unless I don’t think it’s of value.

Test should fail on fromDefs:

6: Collection of Decor -- DungeonBuilder:126: attempt to call a nil value (global 'fromDefs')

Now I could write this as a method on DungeonBuilder right now but let’s work in the test for a bit:

After a bit too much of a delay, I decide that I can’t take this big a bite … I have this in the test, which should show us why the bite is too big:

        _:test("Collection of Decor", function()
            local fromDefs = function(defArray)
                local builder = GameRunner():defineDungeonBuilder()
                builder:buildTestLevel(10)
                return ar.map(defArray, function(def)
                    print(def)
                    local parsed = Parse(def)
                    if parsed.valid then
                        print("valid", parsed.decor.kind)
                        local decorKind = Decor:legalKind(parsed.decor.kind)
                        print(decorKind)
                        local loot = Loot(nil, parsed.loot.kind, parsed.loot.min, parsed.loot.max)
                        local tile = builder:findTile(parsed.room.kind,parsed.room.p[1])
                        return Decor(tile,loot,kind)
                    else
                        return nil
                    end
                end)
            end
            local defs = {
                "Skeleton1;health(5,5);avoid(1)",
                "scoobie;foo;bar",
                "Chest;catPersuasion;avoid(1)"
            }
            local decors = fromDefs(defs)
            _:expect(#decors).is(3)
        end)

Along the way I’ve learned to put semicolons in between the fields, and I’ve learned that having an invalid decor kind doesn’t make the parse invalid, only a syntax error does that.

But mostly I’ve learned that that function is too long and intricate for me to write out longhand.

So I’m going to delete that implementation and try again.

        _:test("Collection of Decor", function()
            local defs = {
                "Skeleton1;health(5,5);avoid(1)",
                "scoobie;foo;bar",
                "Chest;catPersuasion;avoid(1)"
            }
            local decors = fromDefs(defs)
            _:expect(#decors).is(3)
        end)

Starting from there, let me try again.

        _:test("Collection of Decor", function()
            local fromDefs = function(defArray)
                return ar.map(defArray, function(def)
                    return fromDef(def)
                end)
            end
            local defs = {
                "Skeleton1;health(5,5);avoid(1)",
                "scoobie;foo;bar",
                "Chest;catPersuasion;avoid(1)"
            }
            local decors = fromDefs(defs)
            _:expect(#decors).is(3)
        end)

That should fail looking for fromDef:

6: Collection of Decor -- DungeonBuilder:128: attempt to call a nil value (global 'fromDef')

OK, so far so good. Now fromDef:

        _:test("Collection of Decor", function()
            local fromDef = function(def)
                return def
            end
            local fromDefs = function(defArray)
                return ar.map(defArray, function(def)
                    return fromDef(def)
                end)
            end
            local defs = {
                "Skeleton1;health(5,5);avoid(1)",
                "scoobie;foo;bar",
                "Chest;catPersuasion;avoid(1)"
            }
            local decors = fromDefs(defs)
            _:expect(#decors).is(3)
        end)

The test will pass, we’re just counting the little guys. Make the test a bit harder:

            _:expect(#decors).is(3)
            _:expect(decors[1].decor.kind).is("Skeleton1")

My plan here is just to return the parsed item for now. (I notice that my Loot definition in the first input is “health”. It should be capitalized. I do that.

Test should fail on “Skeleton”:

Well, no, it fails on the string not knowing decor. Makes sense. Carry on:

            local fromDef = function(def)
                return Parse(def)
            end

I think it should pass. It doesn’t:

6: Collection of Decor  -- Actual: nil, Expected: Skeleton1

I am a bit confused. I really expected that to work. Let’s print the parsed item before we return it.

I finally realize that the call to parse isn’t the creator. It’s decorDefinition.

            local fromDef = function(def)
                local parsed = Parse:decorDefinition(def)
                print(parsed)
                return parsed
            end

I expect progress. And, indeed, the test passes. Let’s reflect a moment on what just happened.

In the first attempt, I’m sure that I made the same mistake, using Parse(def) instead of Parse:decorDefinition(def). But the code had all that other stuff in it, and it seemed that part must be OK, so I didn’t see the issue. Here, I saw the issue quickly.

This is why the smallest imaginable steps are rarely too small, while perfectly reasonable-seeming larger ones are often too large. You’d think I’d learn this lesson, but sometimes my reach exceeds my grasp.

Carrying on now, we want to actually start creating the individual Decors.

Despite my advice above, I decide to try a larger bite, to create the entire Decor:

            local fromDef = function(def)
                local parsed = Parse:decorDefinition(def)
                local decorKind = Decor:legalKind(parsed.decor)
                local loot = Loot(nil, parsed.loot.kind, parsed.loot.min, parsed.loot.max)
                local tile = nil
            end

Well, almost. I decided at the last minute not to do the tile.

I’m not done up there, but I want to be sure to enhance the test. I got ahead of myself. Am I going to fall, or just wave my arms a bit?

            _:expect(#decors).is(3)
            _:expect(decors[1].kind).is("Skeleton1")
            _:expect(decors[1].loot.kind).is("Health")

Test just to see it break. Well. I didn’t expect this:

6: Collection of Decor -- Inventory:383: attempt to concatenate a nil value (field 'description')

That’s surely coming from my loot creation that I just typed in.

A bit of browsing and I have come to think that it’s “health”, not “Health”. Change that, test again. Same message though. I think that’ll be my error item. Let’s make the middle item legal.

            local defs = {
                "Skeleton1;health(5,5);avoid(1)",
                "Box;pathfinder;avoid(1)",
                "Chest;catPersuasion;avoid(1)"
            }

There are times when no step is small enough. Let’s run the test and then reflect.

6: Collection of Decor  -- Actual: 0, Expected: 3
6: Collection of Decor -- DungeonBuilder:147: attempt to index a nil value (field 'integer index')

The second error is just due to the fact that we don’t have any items. The problem is, we are’t returning anything from our growing function. I decide to try this:

            local fromDef = function(def)
                local parsed = Parse:decorDefinition(def)
                local decorKind = Decor:legalKind(parsed.decor)
                local loot = Loot(nil, parsed.loot.kind, parsed.loot.min, parsed.loot.max)
                local tile = nil
                return Decor(tile, loot, decorKind)

Test. A nice result!

6: Collection of Decor  -- Actual: Skeleton2, Expected: Skeleton1
6: Collection of Decor  -- Actual: health, Expected: Health

I reckon those are both mistakes in the test. But no, only one is, the “Health” should be lower case. Here’s our whole test.

        _:test("Collection of Decor", function()
            local fromDef = function(def)
                local parsed = Parse:decorDefinition(def)
                local decorKind = Decor:legalKind(parsed.decor)
                local loot = Loot(nil, parsed.loot.kind, parsed.loot.min, parsed.loot.max)
                local tile = nil
                return Decor(tile, loot, decorKind)
            end
            local fromDefs = function(defArray)
                return ar.map(defArray, function(def)
                    return fromDef(def)
                end)
            end
            local defs = {
                "Skeleton1;health(5,5);avoid(1)",
                "Box;pathfinder;avoid(1)",
                "Chest;catPersuasion;avoid(1)"
            }
            local decors = fromDefs(defs)
            _:expect(#decors).is(3)
            _:expect(decors[1].kind).is("Skeleton1")
            _:expect(decors[1].loot.kind).is("health")
        end)

We got Skeleton2 back, not Skeleton1. That’s the default when we pass in an invalid kind. I think I need to know what we got. No. The bug is we didn’t fetch it: Code now is:

            local fromDef = function(def)
                local parsed = Parse:decorDefinition(def)
                local decorKind = Decor:legalKind(parsed.decor.kind)
                local loot = Loot(nil, parsed.loot.kind, parsed.loot.min, parsed.loot.max)
                local tile = nil
                return Decor(tile, loot, decorKind)
            end

Test again. Test runs. Permission to commit before we reflect? Thanks! Commit: progress on creating decor from array of def strings.

Let’s reflect:

Reflection

Mostly I want to take a short break. This is still a bit ragged. I knew I was taking a slightly larger bite than I ought to, but I thought I could do it. And I did it, but it took more thinking and testing than I needed. Had I just returned the kind instead of going to the Decor, I’d have seen noSuchKind and almost certainly would have found the problem sooner. Still, it was only a few minutes but I don’t like feeling confused even for a few minutes.

I think there’s a fundamental issue with what’s going on here. It may be an aspect of the earlier design concerns.

We have here a function, soon to be method, that takes a description of a Decor, which amounts to a description of three complex objects, a loot, a tile, and the decor itself, and so we need to decode all those elements and create all the bits and pieces.

We’re locked into that a bit, because a Decor can’t be created without a loot and a tile (though we see here that the tile can be nil; it just wouldn’t show up in the dungeon).

Perhaps I should have created the loot first and checked it, but I was tired of creating wrong kinds of things in my output, just to satisfy temporary checks. So I took a larger bite. And I got away with it, but honestly, I think the longer route would have been faster, and certainly more comfortable. It would have provided more “ease”, which is a thing that I want more of.

One point, for me, is that this function is complicated, and there seems to be no good way to uncomplicate it, but I can at least develop it in as simple an order as possible, and check the individual bits. But there may be a large point.

Maybe the Decor and Loot and tile are a bit too closely intertwined. Maybe we should be able to create a Decor with no tile, but Decor properties, a Loot with no tile, but Loot properties, and a tile … and then at the last moment, weave them together. It would be easy enough with the Loot, which can already be quite happy with no tile. The Decor doesn’t really do anything with its loot until we bump it.

Now I normally like to create objects that are good to go. The pattern is called Complete Construction Method, and it means that we don’t create empty objects and then send them messages to fill them in, we create them all at once with everything they need.

We might have an exception here. But we’re nearly done, so we’ll table that concern, while noting that we have had various problems arising from object creation being difficult, especially in testing.

And let me digress for something small but important.

As I wrote the above, I was browsing the Decor class, so that the things I said about it had a chance of being true. I noticed some methods that were not in alpha order, and two that I had intentionally written near the test, just the other day, and that I had intended to put where they belonged but had forgotten.

So I tidied up the class, just cutting and pasting to get things closer to alpha order. I’ll commit that separately: Tidy Decor.

Kent Beck is writing a series of books, one of which is on Code Tidying. He’s publishing them bit by bit, and I recommend that you sign up to see them as they are created. Check this out.

Let’s get back to it.

Back To It

Where were we? Ah yes:

            local fromDef = function(def)
                local parsed = Parse:decorDefinition(def)
                local decorKind = Decor:legalKind(parsed.decor.kind)
                local loot = Loot(nil, parsed.loot.kind, parsed.loot.min, parsed.loot.max)
                local tile = nil
                return Decor(tile, loot, decorKind)
            end

We are creating our Decor correctly, given valid loot info, but without the tile. To get the tile requires some messing about, but it looks like this:

            local builder = GameRunner():defineDungeonBuilder()
            builder:buildTestLevel(10)
            local fromDef = function(def)
                local parsed = Parse:decorDefinition(def)
                local decorKind = Decor:legalKind(parsed.decor.kind)
                local loot = Loot(nil, parsed.loot.kind, parsed.loot.min, parsed.loot.max)
                local tile = builder:findTile(parsed.room.kind, parsed.room.p[1])
                return Decor(tile, loot, decorKind)
            end

I did it again, writing the code before the test. I just get so excited when I think I know what to do. Let’s add a check:

        _:test("Collection of Decor", function()
            local builder = GameRunner():defineDungeonBuilder()
            builder:buildTestLevel(10)
            local fromDef = function(def)
                local parsed = Parse:decorDefinition(def)
                local decorKind = Decor:legalKind(parsed.decor.kind)
                local loot = Loot(nil, parsed.loot.kind, parsed.loot.min, parsed.loot.max)
                local tile = builder:findTile(parsed.room.kind, parsed.room.p[1])
                return Decor(tile, loot, decorKind)
            end
            local fromDefs = function(defArray)
                return ar.map(defArray, function(def)
                    return fromDef(def)
                end)
            end
            local defs = {
                "Skeleton1;health(5,5);avoid(1)",
                "Box;pathfinder;avoid(1)",
                "Chest;catPersuasion;avoid(1)"
            }
            local decors = fromDefs(defs)
            _:expect(#decors).is(3)
            _:expect(decors[1].kind).is("Skeleton1")
            _:expect(decors[1].loot.kind).is("health")
            _:expect(decors[1]:currentTile():isRoom()).is(true)
        end)

This passes. We had to gin up a builder to find the tile, but we don’t mind that because we’re actually doing all this in DungeonBuilder anyway.

Now that this is working this well, let’s do move the functionality from the test into DungeonBuilder methods. We’ll recast the test:

        _:test("Collection of Decor", function()
            local builder = GameRunner():defineDungeonBuilder()
            builder:buildTestLevel(10)
            local defs = {
                "Skeleton1;health(5,5);avoid(1)",
                "Box;pathfinder;avoid(1)",
                "Chest;catPersuasion;avoid(1)"
            }
            local decors = builder:fromDefs(defs)
            _:expect(#decors).is(3)
            _:expect(decors[1].kind).is("Skeleton1")
            _:expect(decors[1].loot.kind).is("health")
            _:expect(decors[1]:currentTile():isRoom()).is(true)
        end)

Running it will tell us that builder doesn’t know fromDefs:

6: Collection of Decor -- DungeonBuilder:136: attempt to call a nil value (method 'fromDefs')

Put it in:

function DungeonBuilder:fromDefs(defArray)
    return ar.map(defArray, function(def)
        return fromDef(def)
    end)
end

Test will discover that we don’t know fromDef.

6: Collection of Decor -- DungeonBuilder:380: attempt to call a nil value (global 'fromDef')

Put it in and link it up.

function DungeonBuilder:fromDef(def)
    local parsed = Parse:decorDefinition(def)
    local decorKind = Decor:legalKind(parsed.decor.kind)
    local loot = Loot(nil, parsed.loot.kind, parsed.loot.min, parsed.loot.max)
    local tile = self:findTile(parsed.room.kind, parsed.room.p[1])
    return Decor(tile, loot, decorKind)
end

function DungeonBuilder:fromDefs(defArray)
    return ar.map(defArray, function(def)
        return self:fromDef(def)
    end)
end

I expect the test to run. It does. Commit: Decor creation uses builder:fromDefs and fromDef methods.

This is still not terribly bulletproof in case of errors. I can check at least one case:

        _:test("Collection of Decor with Syntax Error", function()
            local builder = GameRunner():defineDungeonBuilder()
            builder:buildTestLevel(10)
            local defs = {
                "Skeleton1;health(5,5);avoid(1)",
                "Box,pathfinder,avoid(1)",
                "Chest;catPersuasion;avoid(1)"
            }
            local decors = builder:fromDefs(defs)
            _:expect(#decors).is(2)
            _:expect(decors[1].kind).is("Skeleton1")
            _:expect(decors[2].kind).is("Chest")
        end)

I expect this test to run. Ah. It does not:

7: Collection of Decor with Syntax Error -- Inventory:383: attempt to concatenate a nil value (field 'description')

I had this check in my longhand example, but left it out of the current one. Let’s fix:

function DungeonBuilder:fromDefs(defArray)
    return ar.map(defArray, function(def)
        if def.valid then
            return self:fromDef(def)
        else
            return nil
        end
    end)
end

I had high hopes for that but it doesn’t work. In ar.map we can’t just return a nil. We’re expected to return something that can be put into the output. We’ll have to give up on our use of map here. It carried us this far but now we’ve got to write it out longhand … no, wait … let me try this:

Meh. I tried to filter the array but somehow it didn’t work.

function DungeonBuilder:fromDefs(defArray)
    local valids = ar.filter(defArray, function(def) return def.valid end)
    return ar.map(valids, function(def)
        return self:fromDef(def)
    end)

I see my problem. Let’s try once more.

function DungeonBuilder:fromDef(parsed)
    local decorKind = Decor:legalKind(parsed.decor.kind)
    local loot = Loot(nil, parsed.loot.kind, parsed.loot.min, parsed.loot.max)
    local tile = self:findTile(parsed.room.kind, parsed.room.p[1])
    return Decor(tile, loot, decorKind)
end

function DungeonBuilder:fromDefs(defArray)
    local parsed = ar.map(defArray, function(defString)
        return Parse:decorDefinition(defString)
    end)
    local valids = ar.filter(parsed, function(item) return item.valid end)
    return ar.map(valids, function(item)
        return self:fromDef(item)
    end)
end

OK. Test runs. We parse them all. We select the valid ones. We create those. I want some renaming:

function DungeonBuilder:fromParsed(parsed)
    local decorKind = Decor:legalKind(parsed.decor.kind)
    local loot = Loot(nil, parsed.loot.kind, parsed.loot.min, parsed.loot.max)
    local tile = self:findTile(parsed.room.kind, parsed.room.p[1])
    return Decor(tile, loot, decorKind)
end

function DungeonBuilder:fromDefs(defArray)
    local parsedArray = ar.map(defArray, function(defString)
        return Parse:decorDefinition(defString)
    end)
    local valids = ar.filter(parsedArray, function(parsedItem) return parsedItem.valid end)
    return ar.map(valids, function(parsedItem)
        return self:fromParsed(parsedItem)
    end)
end

Is this better than the for loop I was going to write? Not notably, but I’m trying to get used to using the functional programming functions, if only to learn why not, but hopefully to learn how to make them more useful.

What if we didn’t write those functions in line?

function DungeonBuilder:fromDefs(defArray)
    local function parseItem(defString)
        return Parse:decorDefinition(defString)
    end
    local function valid(parsedItem) return parsedItem.valid end
    local function fromParsed(parsedItem) return self:fromParsed(parsedItem) end
    
    local parsedArray = ar.map(defArray, parseItem)
    local valids = ar.filter(parsedArray, valid)
    return ar.map(valids, fromParsed)
end

I’m declaring that better. Commit: Improve fromDefs to use FP ar. functions.

I would like to do this:

function DungeonBuilder:fromDefs(defArray)
    local function parseItem(defString)
        return Parse:decorDefinition(defString)
    end
    local function valid(parsedItem) return parsedItem.valid end
    local function fromParsed(parsedItem) return self:fromParsed(parsedItem) end
    
    local input = Array(defArray)
    return input:map(parseItem):filter(valid):map(fromParsed)
    --[[
    local parsedArray = ar.map(defArray, parseItem)
    local valids = ar.filter(parsedArray, valid)
    return ar.map(valids, fromParsed)
    --]]
end

I’m not sure the FP functions are rigged to be fluent. Test.

7: Collection of Decor with Syntax Error  -- Actual: 1, Expected: 2
7: Collection of Decor with Syntax Error -- DungeonBuilder:154: attempt to index a nil value (field 'integer index')

It’s clear the “bug” is in FP. Let’s look. Yes. I beef up the FP to be sure to return instances of Array or Dictionary and the fluent form works.

I’ll write that up tomorrow, but for now, I’m committing this:

function DungeonBuilder:fromDefs(defArray)
    local function parseItem(defString)
        return Parse:decorDefinition(defString)
    end
    local function valid(parsedItem) return parsedItem.valid end
    local function fromParsed(parsedItem) return self:fromParsed(parsedItem) end
    
    return Array(defArray):map(parseItem):filter(valid):map(fromParsed)
end

I like the fluent form, and it works. I suspect that my fp functions aren’t really as robust as I’d like, but that’s OK, they’re a work in progress as well.

Commit: use fluent FP methods in fromDefs

And commit fp: Make fp functions more capable of fluent use.

I think we’re done for the morning, given that it’s 1225. Let’s sum up.

Summary

We’re moving toward creating the Decor in a dungeon level from an array of string definitions. I think the current code will create the Decor correctly from correct input, and that it is not robust in the face of errors in the individual fields of the input string, particularly Loot. I think the Decor will default to Skeleton2 in the face of error, and if I recall correctly, the tile finding code defaults to “avoid(1)`. So we have a bit of error checking to do.

Then, I think what I’d like to do is to create a new table for DungeonBuilder, containing for at least one level, a complete description of the Decor. We’ll need, of course, to do our current random thing, or a better random thing, for levels without a table. We’ll discover that as we go.

I want to return to the question of design, this time from an overview of the whole subject of defining Dungeon contents in terms of DungeonObjects of all kind, including Decor. In particular, would we be happier if we had known beforehand what had to be done and had “just” built it once and for all? Or, from the other side, does this continual learning at the last minute not cause us distress and rework?

I freely grant that even I can see that I’m trying things to see if they fit, and often they don’t, and I have to sand one thing and shim up another until they do fit. It would certainly be ideal if every bit fit perfectly. But I don’t believe that more thinking, or more diagram drawing, would make that better.

And I don’t believe that if we were to write down, even right now, exactly what we need, we could just type it in. I think, instead, we would likely come up with a number of objects that don’t even exist now, I don’t know, DecorFactories and LootMakers and TileFindingMonsters and for that matter MonsterAllocators and NpcDefiners, and we wouldn’t be able to do anything until we had most of those objects done. Oh, we might just start with the LootMaker … but even there, it would have a lot of methods and capabilities to be built. I think we’d very likely spend a lot of time building up the infrastructure before we could build anything.

I could be wrong, but six decades of software development makes me pretty sure that even though this process feels like we’re wandering in the forest, we find a mushroom often enough that we can deliver something good every few days, and that keeps us from starving.

And even if I’m not wrong, this might not work for you, or anyone else. I think others can learn to to it, because I know tens of others who can do it, and most of them would continue to do things this way. But that still doesn’t mean that you should do it.

My mission is for you to see what happens, understand it, and decide, for yourself, and for your team, how much of this incremental stuff you want to take up. I think you’d benefit from pushing it until it seems like you’ve pushed too much, because my experience is that if I push it down to one line at a time, it’s still not too far, but you get to decide if you even want to try.

If all I can do is to serve as a bad example, I’m OK with that. Because this stuff works for me, it provides me with ease, and with things to write about.

Uh, everything’s under control. Situation normal.
We’re fine. We’re all fine here now, thank you. How are you?
– Han Solo

One thought on estimation: I thought I’d be done with Decor this week, and that I’d probably have Loot done as well. Today’s Thursday, so what we can see is that, one week out, I can’t guess how much I’ll get done with any great accuracy. YMMV on that, too.

BTW: I’ve not had my followup call with Hammond of ScopeMaster, but if and when that happens, I’ll report the results.

See you next time!