Pyrel dev log, part 5

[Derakon]

Except it's not a success message, it's a failure message. You didn't succeed at the conditional, so you never even got to the "do something" function. However you still need to provide feedback to the player so that they know that something was at least tried.

Failure messages are printed by cancellation procs; success messages are printed by do-something procs. There should never be ambiguity or a failure of feedback under that constraint.

Just because the race condition has a predictable outcome based on the current state doesn't mean it's not still a race condition. The issue is that the behavior can be 'unexpected' from the player point of view (even if reliably repeatable) simply due to the names chosen for the procs, not due to the code each executes.

Could you illustrate this with a concrete example, please? I'm having trouble seeing the problem.

[Kinematics]

Failure messages are printed by cancellation procs; success messages are printed by do-something procs. There should never be ambiguity or a failure of feedback under that constraint.

I was replying to you saying that the "you can't lift that!" message was a success message.

Could you illustrate this with a concrete example, please? I'm having trouble seeing the problem.

I should note that the original comment related to the proposal you seemed to be making (but weren't actually, or changed your mind), of removing the explicit distinction between prep and action calls, and handling it all via tiering. So the conflict largely doesn't actually exist, except in miscommunication.

However, to provide an explicit example:

You have a grenade.
The grenade has procs for: "cursed", "explode", that both handle the "onDrop" trigger condition.

"cursed" prevents you from dropping the grenade except on holy land.
"explode" explodes if you drop the grenade on normal ground. It prevents you from dropping it in water.

Since they both have a cancel condition, they're both moved up a tier above normal proc conditions. Still, they end up at the same level as each other.

You're standing in a consecrated lake.
You attempt to drop the grenade.

In alphabetical order, those will occur as: "cursed", "explode"

"cursed" finds that you're in a valid location to drop the grenade. "You let go of the cursed bauble."
"explode" determines that you're in the water, and won't let you drop the grenade. It cancels. "You're unable to release the grenade here."

Conflict of messages due to the fact that the 'cursed' proc was called before the 'explode' proc solely due to alphabetization. If the first proc was called "haunted" instead of "cursed", the above check would have gone to "explode" first, leading to a proper cancellation and no errant message, but only getting it right due to chance.

[Derakon]

In alphabetical order, those will occur as: "cursed", "explode"

"cursed" finds that you're in a valid location to drop the grenade. "You let go of the cursed bauble."
"explode" determines that you're in the water, and won't let you drop the grenade. It cancels. "You're unable to release the grenade here."

Conflict of messages due to the fact that the 'cursed' proc was called before the 'explode' proc solely due to alphabetization. If the first proc was called "haunted" instead of "cursed", the above check would have gone to "explode" first, leading to a proper cancellation and no errant message, but only getting it right due to chance.

Right, so, the problem in this example is that the "cursed" proc shouldn't print anything until you actually do drop the item; the first pass should just say "yes, as far as I'm concerned this is a legit place to drop" and let things pass on silently.

I think we're both on the same page though.

[Kinematics]

Ok, have written out a lot of thoughts on immutability, but in the end I don't think it's completely useful. Python is, by default, a mutable language, though it does have a few immutable structures (numbers, strings, tuples).

I think there are certain areas where it would be helpful to make aspects of the code immutable, but there are a lot of areas where it's not easy, and would probably add a substantial amount of unnecessary overhead.

Procs, for example, can probably be done as immutables. Everything about a proc is defined on construction, so it doesn't need to change anything after that. On the other hand, there's generally nothing -to- change after that, so calling it immutable is a little redundant. Also, since the triggerMaps is a dict, there's a hole in immutability as well if that's implemented.

One could, perhaps, create a named tuple to store the state of the basic parameters being passed in (eg: (item, user, gameMap, cancelled)) that would simplify the parameter list a bit. If each proc was required to return that tuple that could potentially make it easier to keep a consistent state as each proc was called. However since our list of procs is a bit sparse at this point, it seems premature to say that it can really handle all use cases.

Overall I think the approach should be: Try to make any new class immutable first. Only make it mutable if immutability brings with it impractical issues. Note that I'm only considering behavioral immutability (ie: just treat it as if it were immutable) rather than full code immutability, since all the code to make a class truly immutable is a pain to add in. Only add that if it can be shown to work as an immutable class.

[Kinematics]

Anyway, while writing up thoughts on immutability, I realized there were various things that weren't well-defined in how procs interact with other stuff.

Example: Cast Fireball at a mob that's surrounded by scrolls and stuff. Naturally the scrolls burn up. How does that happen? Is this explicitly built into the fireball effect? Or does the fireball do a stat check on every item it touches, checking for fire vulnerability (which in turn allows for proc checks per item)?

The latter seems the simplest way of going about it, but if it does that, what do the procs on each item's stat check look like? EG: If there's a "Protection from Fire" spell cast on one of the scrolls (thus a proc), what sorts of parameters are passed into it when it's called from a stat check via a fireball effect generated from a wand held by the player?

Is a 'user' passed in as part of the parameters? If a player aimed a wand and caused it, is the player the user? If a dragon breathed, is the dragon the user? If the dragon breathed on the player, and it's affecting scrolls in inventory, which one is the user? If triggered from a wand, is the wand anywhere in the parameter list as the effect gets handled?

Also, do the items destroy themselves (ie: remove their reference from the game map), does the fireball effect destroy them (explicitly manipulate the game map to remove the specified item), or is some message sent to the game world to remove that item from existence (so that the only manipulation of an item's existance happens within the game map class)?

In general we don't want to allow one object to directly manipulate another object. That gets back to the whole 'immutability' aspect; the more immutable an object, the less we have to worry about bugs from other stuff messing with it.

Anyway, just some issues that were rather vague at the moment, and impact what sorts of things the procs argument lists need to handle (and by extension, whether named tuples can be considered generally useful for them).

[Derakon]

Kinematics' increasingly-inaccurately-named "grammar" pullreq is now in! Thanks again for all your work, Kinematics! With any luck the next time it won't take me so long to review your changes.

[Derakon]

I've implemented basic pathfinding for creatures now. This includes:

* A system to simplify the game map down to an array of numbers, making it easier and more efficient to analyze; this will also be useful for calculating line-of-sight and possibly for other things.
* An implementation of the A* search algorithm, and a few associated unit tests.
* Addition of the new "creature update" proc trigger condition, and an associated BasicAIProc on all monsters (by way of a new "opponent" creature template that all enemy creatures inherit from).

I'd appreciate a code review if anyone's interested in providing it. You can see the commits here, starting from eb09d52. Unfortunately I can't figure out how to get Bitbucket to display a diff of multiple commits without having a pullreq, and I can't make a pullreq since my repo is master. I should probably do something about that...

EDIT: also, if you'd care to suggest algorithms for doing LOS, that's my next step...

[Magnate]

EDIT: also, if you'd care to suggest algorithms for doing LOS, that's my next step...

A couple of years ago there was a LOS thread on here that got so long and so detailed that someone went off and started an entire wiki discussion somewhere to carry it on. There were a lot of good ideas and even better rebuttals so it's probably worth re-reading. Sorry I don't have time to find the link!

[mtadd]

My contribution to last year's discussion of LOS / FOV was this link on FOV using recursive shadowcasting. The article describes the algorithm nicely, as well as provides and implementation in python at the end.

[debo]

Overall I think the approach should be: Try to make any new class immutable first. Only make it mutable if immutability brings with it impractical issues. Note that I'm only considering behavioral immutability (ie: just treat it as if it were immutable) rather than full code immutability, since all the code to make a class truly immutable is a pain to add in. Only add that if it can be shown to work as an immutable class.

FWIW -- I like to think of things in terms of "value objects" and "entities". If something has an actual identity that persists throughout the program, it's an entity and it's probably ok to mutate it. If it's just describing something and equality is based on e.g. field equality, then it's a value object and should probably be immutable.

Aggregates and services are another story

(Stolen from Domain Driven Design)

[Derakon]

My contribution to last year's discussion of LOS / FOV was this link on FOV using recursive shadowcasting. The article describes the algorithm nicely, as well as provides and implementation in python at the end.

Thanks; I'll have to check that out when I get a chance. For now I'm working on a copy of the "precise permissive field of view" algorithm described here -- mostly because d_m handed me a copy of the code that worked already. Of course I'll be trying to design things so that we can swap in different LOS/FOV algorithms with minimal pain. I don't have any sacred cows in this particular area; I just want something that basically works so I can start working on player knowledge.

[mtadd]

Derakon,

I'll take a look at your implementation of A* soon. In the meantime, I was wondering if you ever read this article on Dijkstra Maps by the developer of Brogue? Its an interesting technique of creating gradients using a modified Dijkstra graph search through the passable portions of the map with differing intial conditions to assist in pathfinding, autoexploring, and mob AI.

[Derakon]

I haven't seen it, no. Add another thing to my list of stuff to check out.

In general I do embarrassingly little research on most of these algorithms, instead relying mostly on what I learned as an undergrad and what I've heard about / used in the past in my years as a hobbyist game developer and general professional software developer. I won't remotely claim that this is the right way to do things, but working on Pyrel is a hobby for me so I'm going to do what I enjoy, and sometimes that's redesigning or reimplementing the wheel.

[Derakon]

Okay, player field-of-view is implemented!



Here we can see the debug town, complete with an alleyway holding the Phial and the Arkenstone. The best thing about this is that those artifacts are guarded by a filthy street urchin, which the player cannot see. Yes, the player's information is incomplete for the first time! This involves having the player maintain a mental copy of the game map which is updated each turn based on what's in their field of view. As squares leave their FOV, all "transient" entities are removed from that mental copy, thus removing things like creatures but leaving terrain, items, etc. behind.

One point of amusement here is that the field of view is square -- 20 steps in each direction, including diagonally. It looks kind of funny but is the Right Thing to do (so long as diagonal movement costs the same as orthogonal movement, anyway).

One thing I haven't done yet is keep track of cells that the player has seen but that are currently empty, hence why all non-visible empty cells aren't displayed at all rather than being displayed as dim '.'s. That shouldn't be difficult though.

Oh, yes -- I ought to have mentioned this earlier. Between the pathfinding and the field of view work, I've decided it makes the most sense to just have Pyrel include Numpy as a dependency rather than try to do all these array operations etc. ourselves. Numpy is reliably efficient and makes dealing with large arrays of numbers in Python much more pleasant (compared to working with lists of lists, for example). It is one more thing to install though.

EDIT: checked out the dungeons, fixing a few bugs in dungeon generation in the process. Looks like some of this code didn't get very thoroughly tested; oh well. Anyway, turns out A* pathfinding isn't the best idea for monsters in actual dungeons -- I was looking at about .5s/step. Guess I'll need to switch to something like heat maps (a.k.a. the "Dijkstra Maps" mtadd linked earlier) so there's less work for each individual creature to do when deciding where to go next.

Oh well. The A* pathfinding system works, and it could still come in handy for one-off effects. I could imagine e.g. a spell that draws a glowing line on the ground to lead the player to some objective, for example; sure, a heat map would do the same thing, but A* has the advantage in one-offs of not needing to examine the entire map.

[Magnate]

One point of amusement here is that the field of view is square -- 20 steps in each direction, including diagonally. It looks kind of funny but is the Right Thing to do (so long as diagonal movement costs the same as orthogonal movement, anyway).

Thank you for the bait - I've been meaning to air this for a while. To me, diagonal movement costing the same is Just Wrong. There's no truly compelling reason - the strongest one is visceral ugh, the next is that it makes something that looks like a square actually function as a circle, which just jars. Neither is game-breaking: most roguelikes have done this for years with no particular ill effects. But I like the idea that Pyrel isn't bound by these tropes, and could actually implement proper movement costs. Yes, this would mean that if you move diagonally a monster could move horizontally and then attack you before you move again. But that feels intuitively correct, rather than diagonals being handy escape routes.

Oh, yes -- I ought to have mentioned this earlier. Between the pathfinding and the field of view work, I've decided it makes the most sense to just have Pyrel include Numpy as a dependency rather than try to do all these array operations etc. ourselves. Numpy is reliably efficient and makes dealing with large arrays of numbers in Python much more pleasant (compared to working with lists of lists, for example). It is one more thing to install though.

Installing deps is trivial for Linux folks but horrible for Windows users. Is there some way we can bundle Numpy with the Pyrel setup for Windows? Worth investing time in an msi?

EDIT: checked out the dungeons, fixing a few bugs in dungeon generation in the process. Looks like some of this code didn't get very thoroughly tested; oh well.

There are two types of tests. There are code tests (unit tests, end-to-end tests, whatever they're called), and playing tests. The former require much more professionalism and discipline than your contributors have, and the latter are almost impossible at such an early stage of development - there are doubtless hundreds of bugs in my item code which haven't been found yet because items haven't been picked up, wielded, used, damaged, destroyed etc. etc. The only bugs I've been able to test for are the ones in generation - and I had to write a chunk of wizmode even to do that. So I don't think it's surprising or reprehensible that the dungeon code, by the same token, isn't fully tested.