Not Good Enough
Hello, loves!
I think the new Content scheme isn’t good enough. Planning to reset. Might save the tests.
All that seems to be happening in moving from the separate Content classes to the new CombinedContent object is that all the code from each of the separate classes moves over into the Combined class. That’s not better and is arguably worse, as the result of that will be one big class with chunks of capability that never apply to any particular use of the class. It’s like ordering stew and then eating only the potatoes. (OK, not every simile can be a winner.)
We did get some pretty compact tests for our new object, and I want to keep those in the repo: they may serve as ideas for future tests. Here goes.
I had a little difficulty finding quite where to reset to, and I think there’s a chance that I overwrote an article from the afternoon of June 18th. A pity, if so, it was probably the best ever. Anyway, we are now back on the Content hierarchy and the dungeon seems to run OK, although the doors don’t appear: was that just a spike or did I lose them?
I could have reverted all the commits, I suppose, saving all the intermediate steps between the 18th and today, and then I could go back and root through them looking for useful bits. I prefer the solid commitment to starting anew, as if yesterday never happened … except that I saved the tests, all the new code, and what remains in my brain. It’s just that the production code no longer uses the experimental CombinedContent thing.
I do still feel that there ought to be a better scheme than the current one with an implementor of the Content protocol for every kind of content in the dungeon. I think I’ll go through the Content classes and make a table of what they do.
| class | init | interact | run | callbacks |
|---|---|---|---|---|
| Drawable | name resources scale | return True | pass | |
| Receivable | name resources scale | give, announce, return True | pass | |
| DungeonControl | name resources scale state enterable | increment state, publish state, return enterable | state=0 | |
| Animated | name resources scale state cases cycling time | return True | state=0, time=0, cycling=True, subscribe to on_update and controls | cycle on update, adjust cycling on controls (using cases) |
| Appearing | name resources scale | give self & announce if visible, return True | publish state, subscribe to control, set invisible | announce appearance, become visible, publish state |
Some thoughts about this, pretty random:
- State occurs in quite a few of these, so it might be made standard in all subclasses.
- In Appearing, could use state as the visible flag if we made it standard.
- There is really no reason to be carrying the resources in the content, they are a view thing. Perhaps just a key to a table would suffice.
runis really more likeinitializeand could be renamed.- Possibly a content item should be made up of some separate objects. It might have a thing to give to the player rather than be a thing that can be in the dungeon or given.
- Another sub-object might be a state machine kind of thing for managing state like Animated uses.
- We publish state_number when we want the view to update. An explicit message might be better;
- Similarly we use state_number to signal control to another content. Explicit might be better.
- Possibly we should provide a list of event/caller_id/callbacks to the class, which it would set up for us, avoiding the need for any code in the methods of our Content classes: it could all be done in the creation.
- Possibly, along the sub-object idea, we should be passing in little objects that
runandinteractforward to, rather than creating a whole class with similar behavior.
What are we going to do? We have features that we could be building. In a real product situation, I think the answer would have to be to back-burner this topic and get down to using what we have to build the doors and other features that we need, refactoring and improving in small bits as we can see them.
However, we only have four, maybe six hours invested in the CombinedContent idea, counting article-writing time, which is half or more of the elapsed time spent when I’m coding and writing at the same time. Recall that sometimes an article winds up describing very few lines of code.
I’m inclined to push on the content idea with some more experimentation. I feel sure there is a pony in here somewhere.
I was thinking we might start by making state a standard element of content, initialized to zero and used as needed. But that rubs my nose in one of the most irritating things about the current scheme, namely that each of our Content implementors has its own init, much like the other inits.
I think I’m going to relax the rules about inheritance of concrete behavior and see if that bites us. We won’t do a complex hierarchy but will try to be judicious about it.
Some of our inits accept a list of resources and some expect only one. Let’s change them all to expect a list.
That goes easily. There may be a scaling problem with the torch. Note made. Commit.
Now let’s put a default __init__ in Content and inherit it. I’ll add the state=0.
class Content(ABC):
def __init__(self, name, resources=None, scale=0.5):
self.name = name
self.resources = resources
self.scale = scale
self.state = 0
Two of the concrete classes still have a bit of init left:
class DungeonControl(Content):
def __init__(self, name, resources, enterable: bool):
super().__init__(name, resources)
self.scale = 0.75
self.enterable = enterable
class Animated(Content):
def __init__(self, name, resources, cases):
super().__init__(name, resources)
self.scale = 0.75
self.cases = cases
self.cycling = True
self.time = 0
We should be able to pull scale out into the creation of those two and leave it in the init. That’s readily done and now the inits are a bit simpler:
class DungeonControl(COntent):
def __init__(self, name, resources, scale, enterable: bool):
super().__init__(name, resources, scale)
self.enterable = enterable
class Animated(Content):
def __init__(self, name, resources, scale, cases):
super().__init__(name, resources, scale)
self.cases = cases
self.cycling = True
self.time = 0
AppearingContent, DrawableContent, and ReceivableContent all just inherit the Content __init__, reducing them substantially.
Works. Commit: inheriting init from Content, so sue me.
I think we’ll wrap up here. What have we wrought?
Summary
I feel good about resetting the CombinedContent idea. We’ll keep the tests and the class, which is inside the test file, because it embodies some fairly simple testing of content and an interesting implementation of pluggable behavior, even though it seemed to get too heavy for actual use.
I think there’s something weird about the interact code, in that it has two functions: it is used to evoke an action, such as giving an item to the player, or toggling a visible object in the dungeon. And its return of True/False is used to ascertain whether the player can step into that tile or not.
It seems likely that we should probably separate out those two notions, interacting with the player and deciding whether she can enter.
I’ll stare at the table above, and perhaps make some different pictures of the situation. I still feel that there is too much code here, and that somehow there are some smaller ideas that can be assembled to make up the larger behavior. Maybe it’ll come to me.
For the morning, we have reset out an idea that wasn’t panning out, and have made a few simplifications to the existing content sets, and smoothed out some differences that didn’t need to be there.
Small progress but feels like progress. See you next time!