Dungeon content has aspects of model and aspects of view. Let’s wrap it in a ContentView and then evolve to something more cohesive. Sort of Part I.

A case could be made that I plan to work on today is premature. Everything is working as intended and the code isn’t that bad. But it doesn’t feel right to me, and we are about to do some work that will be impacted by this design and that will, in turn, impact it. We need to add more content of kinds similar to our Button and SecretItem, but different. We need to have some content items change appearance in operation. It is possible that we’ll even have active content items that can move around or take other actions.

I think most folx would agree that changes like those are likely to go more smoothly if the dungeon aspects of those changes (model) are as independent as they can be from the visual aspects (view). But the question remains: should we clean up the mess now, or do more work, the better to see the mess, and then clean it up. If we do it now, we risk doing it speculatively, without enough concrete feedback, getting it wrong somehow and then having to change things around again. If we defer, we may gain better understanding, and it will be at the cost of having more of a mess to clean up.

It’s always a judgment call, and in my view, it’s never a big risk either way, so long as we are good at keeping on top of code before it gets seriously out of control. If we’re running short of new features, maybe do those at the cost of more mess. If not, maybe take the time to make the area better before we make the feature changes.

By the way …
On a Real Life™ project, I would generally recommend not cleaning up a mess if there is not a near-term need to work in that area. Cleaning up that mess has no immediate payoff, other than perhaps making us feel a little better. It provides no new capability, and since we’re not going to work in that area, it doesn’t even make our work easier. You do you, of course, but unless I just couldn’t resist, I’d say to save cleaning up the code for times when we need to be there anyway.

My call today is that we’re going to start the necessary changes now. My reasons might not apply anywhere other than in this long-running exercise, so don’t go running around saying I said you should clean up your code right now. Not that you would.

Let’s do ContentView

What is it? Like most any view, it’ll be a wrapper around a content item. Its job will be to manage the visible properties of that item, in particular its Sprite and the Sprite’s texture or textures. We’ll see what else arises. Visibility will certainly arise and, at some point, changing appearance.

My tentative plan is to create a trivial ContentView that can wrap the content item, with no behavior at all. Then we’ll see what the content item is being asked to do that should be done by the view, and move the behavior. That process may take place over some time, as new needs arise.

I see some issues. First, as things stand now, the DungeonView just creates a bunch of sprites and thereafter pretty much ignores content. Relatedly, the content item itself knows the path to its texture. I think we’ll need to sort that out, but perhaps not first.

The first thing, I believe, is to change DungeonView to create and save a collection of ContentView instances. The current related code is this:

class DungeonView:
    def setup(self):
        self.setup_scroller()
        self.setup_cameras()
        self.shape_list = arcade.SpriteList()
        self.create_rooms(self.shape_list)
        self.content_list = arcade.SpriteList()
        self.create_content_list(self.content_list)

    def create_content_list(self, content_list):
        for cell, content in self.dungeon.contents.items():
            for item in content:
                sprite = item.get_sprite()
                sprite.position = cell.center_position(cell_size)
                content_list.append(sprite)

I think that should become a two-phase operation. First, create a content_view_list, giving it the Sprite, and then, in a separate pass, make the sprite list. We could do them together, I suppose.

That might actually be easier. We’ll create them together.

class DungeonView:
class DungeonView(arcade.View):
    def __init__(self, dungeon, testing=False):
        if not testing:
            super().__init__()
        self.dungeon = dungeon
        self.key_lock = None
        self.room_sprite_list = None    # renamed
        self.content_views = None       # new
        self.content_sprite_list = None # renamed
        self.dungeon_camera = None
        self.dungeon_camera_bounds = None
        self.scroller = None
        self.scroller_camera = None

Then this:

    def setup(self):
        self.setup_scroller()
        self.setup_cameras()
        self.room_sprite_list = arcade.SpriteList()
        self.create_rooms(self.room_sprite_list)
        self.create_content_lists()

    def create_content_lists(self):
        self.content_views = []
        self.content_sprite_list = arcade.SpriteList()
        for cell, content in self.dungeon.contents.items():
            for item in content:
                self.content_views.append(ContentView(item))
                sprite = item.get_sprite()
                sprite.position = cell.center_position(cell_size)
                self.content_sprite_list.append(sprite)

It remains to create the ContentView class:

class ContentView:
    def __init__(self, content_item):
        self.item = content_item

I think this should work correctly. It certainly passes all the tests because we have few tests if any for the views. Unfortunately. Makes us run the program and look to see if it works. I wish I had a better idea.

To no one’s surprise, this works. Commit: building ContentView.

Clearly the view should hold the sprite, not the item. Let’s change that. We’ll pass the sprite in the view’s constructor. That seems better than a setter.

class ContentView:
    def __init__(self, content_item, sprite):
        self.item = content_item
        self.sprite = sprite

class DungeonView:
    def create_content_lists(self):
        self.content_views = []
        self.content_sprite_list = arcade.SpriteList()
        for cell, content in self.dungeon.contents.items():
            for item in content:
                sprite = item.get_sprite()
                sprite.position = cell.center_position(cell_size)
                self.content_views.append(ContentView(item, sprite))
                self.content_sprite_list.append(sprite)

This will of course work, since nothing has really changed. Run it anyway. Yes. Commit. ContentView installed with no behavior.

Now about that behavior. It seems to me that the View should create the sprite, which is presently created by the content item. I think that, for now, we’ll allow the item to know the path to its resource, and that we’ll move the sprite creation itself to ContentView.

I wish I had not done the change above. Could reset, I guess. But let’s move forward. I’ll code what I intend here.

We’ll want to create our ContentView using the resource path and the position for the sprite, and have it create the sprite. Change Signature:

class ContentView:
    def __init__(self, content_item, resource, position, scale):
        self.item = content_item
        self.sprite = resource

I decided to include the scale, since we know we need it. Perhaps I should have waited.

    def create_content_lists(self):
        self.content_views = []
        self.content_sprite_list = arcade.SpriteList()
        for cell, content in self.dungeon.contents.items():
            for item in content:
                resource = item.get_resource()
                position = cell.center_position(cell_size)
                view = ContentView(item, resource, position, 1)
                self.content_views.append(view)
                self.content_sprite_list.append(view.sprite)

This is what I call Programming by Wishful Thinking. I type in what I want to have work, and then make it work. We need get_resource on content items. I have to add that to four classes. That’s part of the problem we’re hoping to solve.

Now, I think I can create a ContentPainter in ContentView.

class ContentView:
    def __init__(self, content_item, resource, position, scale):
        self.item = content_item
        self.painter = ContentPainter(resource, scale)
        self.painter.sprite.position = position

This is all good, I think, although it isn’t actually doing anything. I add this:

class ContentView:
    @property
    def sprite(self):
        return self.painter.sprite

Now we should be using the sprite from the ContentView. Let’s be sure by getting rid of the content painters in the content items. A couple of tests fail.

This is a larger change than I should have made. We’ll try to bull through: I think we’re close.

The tests both relate to receiving content. That needs to change, substantially. We’ll leave them failing because, in the end, they should work. (If they don’t work, it’s not the end.)

Let’s see what breaks in the game. Probably more than just receiving items.

Ah, set visibility also fails, no surprise but unpleasant.

My tentative plan for these capabilities is to use PubSub to trigger “events” that the DungeonView and other Views can subscribe to. Possibly doing that first would have been better. For now, I’ve commented out the line that breaks the game.

We get the display, with the objects too large, which is due to my defaulting scale to 1 instead of 0.5:

map with content items 2x proper size

As expected, if Dot steps on these items, there is an exception. As unexpected, the program does not terminate. Somewhere in Arcade, exceptions are being swallowed, and the game keeps running. Basically it seems to ignore exceptions in at least some kinds of events. Must study that.

I need a break: there is something in my eye. Let’s publish this half.

Summary of Part I

I think it might have been better to do the pub-sub for item removal and visibility first. But if we had, who would receive them and what would they do? Hm. Dungeon would subscribe: it’s the only active view that we have. DungeonView has the dungeon, so it could, as an interim matter, use existing facilities to do the work, and then we could modify that after we build out ContentView a bit.

I think it might be wise to reset and do it that way. I’ll decide after I get this thing out of my eye.

See you next time!