The Zoom Ensemble met last night. Our ‘work’ gave me some ideas. Plus we have an issue to resolve.

It’s very interesting looking at entirely different code that’s solving much the same problems as the program here. Despite being in a different language, Python, and despite that it uses a game framework, if I’m not mistaken, it has many of the same problems we see here, such as whether rooms intersect, whether they are accessible to each other, and so on.

The author’s coding style is different from mine, mostly shown in a tolerance for larger methods than I like. But they’ve gone down very similar paths to what we’ve done here. For example, in dealing with room adjacency, I finally found it necessary to write everything out longhand, each case separately, because I couldn’t manage the generalities in my head. the Python game’s author also broke out that code into its many cases.

To me, it’s fascinating that different products with different authors have similar problems and create similar solutions–and it’s even more fascinating when our solutions differ in detail or in larger ways.

From the Ensemble session this time, I accumulated these ideas for what I’m doing here:

  • Doors are created twice (we knew that but the session reminded me);
  • Path length between rooms could be useful for separating sht start from the treasure room or a boss room;
  • The notion of a boss room, which hadn’t yet entered my mind;
  • Using table lookups to avoid messy if statements.

We’ll see if some of those arise as we go forward. The question before me this morning is how to go forward.

Pikachu, I choose …

The duplicate doors. Each room has two copies of each of its doors. The reason is simple: our search for neighbors compares every room to all others. So if A is a neighbor of B, then we’ll also find that B is a neighbor of A. The Neighbors table ensures that only one of those is kept, but by the time that happens, the Neighbor has already told the Room to add a door:

function Neighbor:init(room1, room2, isPrimary)
    self.isPrimary = isPrimary
    self.room1 = room1
    self.room2 = room2
    if room1.number > room2.number then
        self.room2,self.room1 = room1,room2
    self.room2:addDoor(self) = getmetatable(self) = self.toString

As soon as the Neighbor is created, boom, we add doors. Now it seems to me that one door per location is enough, so we want to ensure that the doors collection in the room has only one copy of each door. We could do that in at least two different ways:

  1. We could wait to add doors until the room graph is done, and then iterate over all saved Neighbors and tell them to tell their rooms to add doors.
  2. We could leave it the way it is and ensure that the rooms don’t save duplicate doors.

I somewhat favor the latter solution. It is arguably less efficient, but it doesn’t require a special large-scale mechanism to trigger at the right time and do the door generation loop: instead it is part of other things that are going on.

Now the doors we create right now are quite simple:

function Room:addDoor(aNeighbor)
    local x,y = aNeighbor:getDoor(self)
    table.insert(self.doors, {x=x,y=y})

We ask the Neighbor to give us the Door (coordinates) and save a little table of those coordinates for each door. That’s all we needed to get the job of displaying the white door markers on the screen:

door markers

When I described this issue to the Ensemble last night, one member asked “Neighbor?” when he heard that my neighbor object has two rooms (which are neighbors). It occurs to me now … why isn’t that object a Door??? It connects two rooms, and it knows the coordinates of the door in each room. We could give it a more abstract name like “RoomConnection”, but I think it’s a Door, or could become one. So let’s rename the thing and then change Rooms to just save the doors when they are handed to the room … saving only one copy.

I hate the rename. I don’t have a refactoring tool and this will be a pain. I guess there’s nothing to it but to do it.

Well. That turned out to be easier than I thought. I only missed a few changes and the tests caught those. No point posting the code changes here, they were all straightforward except for this:

function Room:addDoor(aDoor)
    self.doors[aDoor:key()] = aDoor

Not that that’s tricky. I just changed the use of the self.doors collection to a hashmap, and used the existing key() function on Door née Neighbor:

function Door:key()
    return self.room1.number*1000 + self.room2.number

Tests are working, except for that darn ignored one. Let’s commit and I’ll fix or delete that test. Commit: rename Neighbor to Door and use as door.

The test that’s ignored is:

        _:ignore("NO MORE FRACTIONS? intersect bug", function()
            local r1 = Room:fromCorners(517.5,359.5,632.5,450.5)
            local r2 = Room:fromCorners(612.5,450.0,	713.5,540.0)

We have other intersection tests. I think this was literally here to check fraction handling. Or something. I don’t know. I don’t see any use for it. Deleting. Commit: removed ignored test as useless.

So. The doors in rooms now know what rooms they connect to, because they are instances of Door. The duplication of doors in rooms is fixed. The world is a better place by a small margin.

Let’s take a quick glance at the code and see what’s egregious enough to need improvement even though it’s Saturday.

There’s a Stack class that I wrote and tested speculatively, that has never been used. Delete and commit.

This is nasty, and called every time we want door coordinates … like every time we do a draw cycle.

function Door:getDoorCoordinates(aRoom)
    local sp
    local x,y
    local wall,center
    local otherRoom = self:otherRoom(aRoom)
    if aRoom:closeEnoughOnWestTo(otherRoom) then
        sp = SegmentPair:forY(aRoom,otherRoom)
        wall = aRoom:east()
        center = sp:overlapCenter()
        return wall,center
    elseif aRoom:closeEnoughOnEastTo(otherRoom) then
        sp = SegmentPair:forY(aRoom,otherRoom)
        wall = aRoom:west()
        center = sp:overlapCenter()
        return wall,center
    elseif aRoom:closeEnoughOnNorthTo(otherRoom) then
        sp = SegmentPair:forX(aRoom,otherRoom)
        center = sp:overlapCenter()
        wall = aRoom:south()
        return center,wall
    elseif aRoom:closeEnoughOnSouthTo(otherRoom) then
        sp = SegmentPair:forX(aRoom,otherRoom)
        center = sp:overlapCenter()
        wall = aRoom:north()
        return center,wall
        return nil

We’ve computed or passed near this information in setting up the door, and we could cache this information: it never changes once the rooms are set up. I think I mentioned yesterday that I was OK with redoing this, but today I’m less OK with it. I was thinking that we could cache the values but since we have two rooms and return different coordinates for each, even that’s a bit tricky. We’ll let that ride but I don’t like that code much.

At a quick scan through, that’s the only thing I see that seems really nasty. Good place to stop for the morning.

See you next time!