Hello, loves!

A simple-seeming idea has been kicking my tail. I’ll spare you the details of that thrashing. Let’s accept that this won’t just patch in.

I’ll publish my thrashing articles, so you can find them in the chronological listing, but I’ll mark them with some kind of warning. Here’s the short form of what I’ve been unable to crack … so far.

We have a new “subregion” notion that lets a dungeon builder temporarily limit building to a (rectangular) subregion of the whole dungeon space, then move on to another region, rinse repeat. The intended use is to build up small connected “neighborhoods”, with only one exit. Then the dungeon designer can populate that neighborhood with all the necessary objects to solve opening the exit. It would let us set up a staged dungeon, where you conquer one neighborhood after another, before finally reaching some climactic level-winning room.

We build dungeons now using a BuildStepper instance, which holds a list of dungeon-building commands and executes the list to build the dungeon. The Stepper has a developer-only mode where it executes just one step at a time, so that the dungeon designer can better observe what’s going on.

The difficulty is this: the current design is built around a particular flow of operations, room layout, connecting up the rooms, and making passages between the rooms. Then the program starts the dungeon “running”, and only then do we create the DungeonView. And the dungeon view, in its initialization, assigns flooring to the rooms, which includes indications of where the passages are.

As such, the various processes of ensuring connectivity, making passages, assigning flooring, and so on, all were written with just doing them once as part of the implicit spec. Then came the Stepper. When we create the dungeon step by step, we want to show the rooms as they come into being, the passages between them, and so on. So we need to do these formerly batch-oriented steps incrementally.

There is one additional glitch, which I haven’t even started to address yet. Passages between rooms are created whenever one room is adjacent to another. This means that if a path between Room A and Room Z touches Room B, there will be an entry in to the A-Z path from B. Normally, this just makes the dungeon more interesting. But with subregions in play, we can’t allow these extra passages, because they might allow for an easy exit from a neighborhood without solving the neighborhood problems. I have come to believe that this feature will require a special kind of path that is impervious to passages except at its ends, connecting only two specified rooms.

There are three or four articles worth of words and code changes showing me trying to adjust things to make the stepper display what we need to see, while producing the same dungeon as we would get if the stepper were just run to completion.

I had hoped that a few simple changes here and there would let the stepper “just work”, but if there is that kind of an arrangement, I’ve not found it. I have, perhaps unfortunately, committed some changes that seemed productive, so it is quite likely that I’ve made the code worse.

Summary So Far

We are at a point where, years ago, I’d have been saying that this stepper requirement broke our design and that we needed to do a “major redesign” of dungeon building, and I’d be crying either to remove the stepper requirement or to delay the project for a relatively long time while we built the new thing. My beliefs now do not permit that kind of strategy. I am confident that we can always (almost always?) refactor in an orderly step-by-step fashion to get from where we are to where we need to be. We may well have to divert a couple of our devs to this feature (build a dungeon by subregions) for a while, but they will be making incremental progress, and they’ll not be breaking anything that our other developers might be doing with other features.

Of course, there’s only me here, so if this is possible, I’ll be demonstrating it with small commits that keep the system working, which, I claim, shows that a larger team could do this design refactoring without disrupting the whole project.

So that’s what we’ll try to do: we’ll figure out and work toward a more incremental dungeon building scheme that keeps the stepper display working as well as the fast run-through.

The Target

I would not claim to know everything we need to do, nor what the actual code will have to be. If I knew that, we wouldn’t be here now. But here’s what I think:

Allocation of flooring is a major issue. There are some seemingly conflicting requirements:

  • We need to display the rooms at every step of the build process;
  • We can only compute the actual flooring of a room when we know its passages;
  • We can know the local passages as each neighborhood is done;
  • The inter-neighborhood passages can only be done after all neighborhoods are complete;
  • Flooring is generated into a SpriteList. There is no direct mapping from a given Cell to its particular flooring sprite. It is not currently possible to change the sprite for a given cell;
  • Drawing of the dungeon comes down to simply drawing the SpriteLists. The View doesn’t think about rooms or cells or walls when drawing.

Design Ideas

We really need to review the current state of the code to feed into our design ideas, but before that review, I do have some ideas:

  • We might treat all the sprite lists created by the stepper as temporary, tossing out the previous list and creating the dungeon anew. This would change the detailed flooring pattern, as flooring cells have some randomness, but since the purpose of the Stepper view is to watch larger issues, we can probably easily live with that variability.

  • Note: I use “path” to refer to the set of cells that make up a hallway between rooms. A “passage” is a particular pair of cells, between two rooms, through which Dot can pass. Perhaps we should rename one or both of those ideas.

  • Normal passages between rooms are created randomly. Once we have made a passage decision, it would be ideal to retain it rather than recalculate it the next time the Stepper updates the view. This could be handled by a mapping from a pair of rooms to a passage, and simply never creating a new pair if one exists. This feature could perhaps be used to create “anti-passages” by storing something in that mapping that says there is a passage from R1 to R2 but in fact there is none. We have no real use for that though it might be useful for the inter-neighborhood paths.

  • Flow for dungeon creation might go like this:

    • define a subregion
      • create a room
        • remove existing flooring sprite list
        • compute new sprite list for all rooms
      • connect the rooms by adding any necessary paths
      • define neighborhood passages
      • add puzzle-solving content
    • do another subregion
      • …
    • do another subregion
      • …
    • add paths between subregions
    • add new passages connecting to those paths
    • compute final sprite list, or bless the last one created

I think we’ll do a quick review of how things are operating now, and then see about isolating some of the operations describe above. I still think that it should be possible to do this, not only in small steps, but without any final big slam replacement of old ways with new.

Let’s start with the BuildStepper:

class BuildStepper:
    def __init__(self, table, layout, dungeon ):
        self.table = table
        self.index = 0
        self.layout = layout
        self.dungeon = dungeon

    def run_to_completion(self, view=None):
        while self._more_to_do():
            self._do_one_step(view)

    def maker_single_step(self, view=None):
        if self._more_to_do():
            self._do_one_step(view)
        return self._more_to_do()

    def _update_view(self, view):
        if view:
            view.update_KSL()

    def _more_to_do(self):
        return self.index < len(self.table)

    def _do_one_step(self, view):
        self.table[self.index](self.layout, self.dungeon)
        self.index += 1
        self._update_view(view)

Of note is that both single-stepping and running to completion use the same _update_view method. There is some hidden magic currently in update_KSL, I believe:

class DungeonView:
    def update_KSL(self):
        KeyedSpriteListUpdater(self.dungeon, self.keyed_sprites).update()

class KeyedSpriteListUpdater:
    def __init__(self, dungeon, keyed_sprites):
        self.setup_assets()
        self._dungeon = dungeon
        self._keyed_sprites = keyed_sprites

    def update(self):
        self.create_room_sprites(self._dungeon, self._keyed_sprites)
        self.create_content_lists(self._dungeon, self._keyed_sprites)

    def create_room_sprites(self, dungeon, keyed_sprites):
        for room in dungeon.unfloored_rooms:
            view = RoomView(room)
            for cell, sprite in view.generate_sprites(dungeon.layout):
                keyed_sprites.add(cell, sprite)

Note the reference to unfloored_rooms, which is intended to allocate flooring and sprites only once. That tracks way down to the Layout where we have:

class DungeonLayout:
    @property
    def unfloored_rooms(self):
        return  (room for room in self.rooms if room not in self.floored_rooms)

    def all_current_rooms_floored(self):
        for room in self.rooms:
            self.floored_rooms.add(room)

This is an example of the fiddling I’ve been trying to make work, setting strange kinds of state down inside the dungeon and layout and such, so that things would just sort out. I think that scheme was not successful and even had it worked, it wasn’t going to leave the design in a clear condition.

Let’s undo some of this. We’ll change unfloored_rooms to just return self.rooms. If we just did that, I think we would get duplicate sprites for rooms, each time we update KSL. So we’ll change that object to create a new KSL every time.

We can’t quite do that in one go, so I do this instead:

class KeyedSpriteListMaker:
    def __init__(self, dungeon, keyed_sprites):
        self.setup_assets()
        self._dungeon = dungeon
        self._keyed_sprites = keyed_sprites

    def update(self):
        self._keyed_sprites.clear()
        self.create_room_sprites(self._dungeon, self._keyed_sprites)
        self.create_content_lists(self._dungeon, self._keyed_sprites)

The current scheme was to update the KSL incrementally. Now we want to do it all over again, so we clear it before each go:

class KeyedSpriteList:
class KeyedSpriteList:
    def __init__(self, floor_list=SpriteList(), content_list=SpriteList()):
        self.content_by_cell = defaultdict(list)
        self.sprite_dict = dict()
        self.floor_sprite_list = floor_list
        self.content_sprite_list = content_list

    def clear(self):
        self.content_by_cell = defaultdict(list)
        self.sprite_dict = dict()
        self.floor_sprite_list = SpriteList()
        self.content_sprite_list = SpriteList()

These changes draw a rather nice map:

generally good-looking map with neighborhoods

This isn’t perfect, but it is explainable. Check this detailed view:

detail map shows extra passages

We see that the central diamond is missing several walls. And it turns out that Dot can exit through any of those holes. This is because we presently assign passages during the finish operation, and we have not implemented the feature that keeps us from adding a passage if there already is one.

In addition, at the top left of the round room, we see a passage into the path that goes up the right side and across to the left. That path is one of the ones that we think we must protect against intrusions through the sides: it amounts to a leak that could permit the player to access a section of the dungeon without solving all the problems. But it’s OK for now: we intend a new feature to fix it.

But stepping, there is another oddity:

all the rooms are heavily gridded

During some steps, all the existing rooms get gridded like that. A close look shows that each cell is showing four walls with mud in the middle. I think that is a default texture that indicates some error has occurred, so we’ll have to look into that.

Reflection

We’ve made two key changes, in a spike form: we have eliminated the unfloored_rooms notion, and we have created the view’s KeyedSpriteList from scratch every time. Some of those scratch creations seem to be doing something odd with the error tiles.)

I think this is a better way to go. Let’s fully remove the unfloored idea. Done, committed.

The mud room issue was easily discovered. When we’re updating the KeyedSpriteList we need access to all the cells in the dungeon, so as to determine their borders. In the stepper, that can happen at a time when there is a subregion in effect, and that means that when we look for the neighbors of a given cell, to determine borders, the cell comes back None, and we assume a wall. The fix is a hack that I had tried before:

class BuildStepper:
    def _update_view(self, view):
        if view:
            sub = self.layout.subregion
            self.layout.clear_subregions()
            view.update_KSL()
            self.layout.subregion = sub 

Arguably, we should really only do this for the stepper, not in finish mode. But it’s always harmless, as it just saves and restores the old region, so it is transparent to any other changes. We’ll let it ride.

Commit that: stepper properly saves and restores regions while drawing full dungeon view.

That’s enough for this session.

Summary

Arguably, all that I’ve accomplished so far is to get things back to roughly where they were a few steps back, adjusting how the KSLM works, and updating KSL on every Stepper step, and removing the ill-conceived unfloored notion. So be it. I had high hopes for the unfloored idea, thinking that it could serve to get incremental operation to work by suppressing extra operations. I was mistaken, and didn’t find out until after the feature had been committed.

With a more clear picture of what is needed, it seemed better to undo those two bits rather than try to find a previous commit to roll back to. Often going forward is the best thing to do, as rolling back a few commits tends to destroy good work along with bad. It is better, in my view, to fix up the bad bits to make them better rather than go far back in the past.

I tend to reserve rolling back to two circumstances: First, when I’ve just begun on a fresh commit and things don’t go right, just pop back and begin again. Second, sometimes I might work for a half hour or hour or more and nothing good happens. Rolling back then seems to work well for me. But sometimes I feel close to the answer and commit as a save point. If things do not pan out, I’m then two or more commits beyond the last good point, and honestly I am never sure what all is in there.

Arguably I should become more adept at rolling back multiple commits. Perhaps that’s true. Today is not that day, and that day may never come.

I think that we have two changes to make to get where we want to be:

  1. Once a passage is created between two rooms, never change it. This will requires some new data structure.

  2. Devise a way of creating a path (hallway) that can only be entered at the ends, to avoid the kind of leak in the dungeon that is shown in our example above.

Both of those sound easy enough to me. We’ll find out. See you soon!