Pyrel dev log, part 5

[Magnate]

Minor aside:

Magnate, if you're going to continue to break this up into multiple threads, please be sure that the first post has links to the previous threads, and that the last post has a link to the next thread.

Fair point.

[Magnate]

Could you list some of those advantages? I can see much more of a case for rings/amulets than for the other flavored item types -- affixes make much more sense for equipment than for consumables IMO, especially with the attendant stacking issues.

I think the thing that bugs me most about this is that the player treats each ring and amulet flavor as its own separate type of item. Players don't think "Ring that has the Speed affix", they think "Rings of Speed". There will be inevitable confusion when they go to the backend and discover that it doesn't work that way. Perhaps I'm making a big deal over nothing -- it would help to see your couple-of-paragraphs description of how new items would be created under this system.

I don't think players of FA think like that; I think they're quite comfortable with the idea of ego jewelry. I'm also not sure that catering to an ossified mindset is what Pyrel is about?

But leaving that aside, I think I can persuade people that the system will be better rather than worse. One of the benefits is no longer having this faintly ludicrous situation:

"I want to fiddle with speed a bit. Oh here we are, if I want to adjust speed on rings of teleportation I go to object.txt. But wait a minute, if I want to adjust speed on boots I go to ego_item.txt (or affix.txt). And if I want to adjust speed on rings I need to edit obj-make.c ??"

[Kinematics]

Created a separate branch to fix up some of the things I noted in the earlier overview. Holding, pending review of main grammar branch.

Remove type-based function call for flavors from the item factory. Only nameInfo flavorTypes are used now.

Remove other references to .type from itemFactory.

Added a cap as a head armor type template. Not used by any current objects; just annoyed me that it wasn't there after seeing one too many Novice Rangers with little feathered caps.

Fix all the mushroom entries, which weren't noted as mushrooms. Along with that, fixed some minor flaws in the food template structure.

Does flavors.py really belong under gui? It's part of the naming code, so would expect it with grammar under util. But I guess it affects the colors too, so... Not moving it, just curious.

Updated comments, rearranged code, and renamed some functions in a few places to help clarify usage.

They're minor changes, and not worth updating the main grammar branch for, so will put into a separate pull request later.

[Kinematics]

Suggestion for stress-testing procs:

Build everything possible using procs (including things such as weight). Put as much as possible of the code into procs to see what is and is not feasible, and how easy it is to achieve.

After done with procs, major systems that can logically be done much more succinctly in core code can be moved back to internal.

~~

Going through the process of how it would work with weight:

Given the last revision advancement, it looks like even things like weight may actually be nicely handled with procs.

When each trigger condition can only initiate a single proc, and each proc needs its own class, any event that has multiple trigger conditions (such as adding and removing weight) becomes a tedious affair.

However with procs being able to handle multiple triggers, and mapping those triggers to specific functions, something like handling weight becomes quite seamless.

Add this to the base item template:

"procs": [{"name": "adjust weight", "triggerCondition": ["onPickup"]},
          {"name": "adjust weight", "triggerCondition": ["onDrop", "onThrow", "onFire", "onBreak", "onDestroy", "onSteal", "onOverflow"]}]

And with this in the 'adjust weight' proc:

    self.triggerMaps = {"onPickup": self.addWeight,
                        "*": self.removeWeight}

And the only thing you need to worry about is making sure the item in question is in the player's inventory, if you're trying to remove weight. All the coding is centralized in one spot.

It could even be streamlined further.

"procs": [{"name": "adjust weight", "triggerCondition": ["onPickup", "onDrop", "onThrow", "onFire", "onBreak", "onDestroy", "onSteal", "onOverflow"]}]

Since the mapping is stored in the proc, we don't need to separate out the different types in the object definition.

Alternatively, rather than add or remove weight, just add all the weight up. Then you don't need two separate functions.

Of course one could take that a step further (more in line with how stats in general are done) and say that you can just add up all the weight every time you need to calculate it, and not need procs for adding/removing weight. In that case it wouldn't be a call to adjust weight, it would be an adjustment to encumbrance when weight changes.

"procs": [{"name": "adjust encumbrance", "triggerCondition": ["onPickup", "onDrop", "onThrow", "onFire", "onBreak", "onDestroy", "onSteal", "onOverflow"]}]

Though maybe you should break that into logical unique components.

item -- "procs": [{"name": "adjust weight", "triggerCondition": ["onPickup", "onDrop", "onThrow", "onFire", "onBreak", "onDestroy", "onSteal", "onOverflow"}]

player -- "procs": [{"name": "adjust encumbrance", "triggerCondition": "onWeightChanged"}]

And the adjust weight proc doesn't have to actually calculate the weight, just fire the event that weight changed, which causes the player to re-calculate encumbrance, which requires querying the player's weight stat. [1]

This also makes sense in case you might have effects that change effective encumbrance -- eg: Spell of Featherweight: effective inventory weight is reduced 25%; that's not something easily accessible from the "adjust weight" proc, and it's not like the weight actually changed, just its effect on encumbrance.


[1] Hmm. The player's carried weight doesn't fit into standard stat calculations neatly. Those are all based on there being stat mods that can be added up, but I wouldn't expect items to do that for their weight. If it did, though, it would tie all the way back to the original item procs -- "adjust weight" would have to change the statMod associated with that item. I suppose it would work well enough, that way. 'Adjust weight' just changes a specific stat mod (would have to be named uniquely, maybe match the item's ID value, like "10865-weight"), then fires the onWeightChanged event.

[Kinematics]

Now that I got the nice stuff out of the way, what about the not-so-nice stuff?

First, what values get passed to the proc?

From item:

    ## Use us -- invoke any appropriate onUse procs.
    def onUse(self, user, gameMap):
        self.triggerProcs(user, gameMap, 'item use')

    ## Trigger procs that have the specified trigger.
    def triggerProcs(self, user, gameMap, trigger, *args, **kwargs):
        for proc in self.procs:
            if proc.triggerCondition == trigger:
                proc.trigger(self, user, gameMap, *args, **kwargs)

proc.trigger(self, ...

That's going to pass in the current item to the proc. However the triggerProcs() call is going to loop through all procs. If we use a potion, does the "adjust weight" proc get called before or after the use effect?

Speaking of which, I left out an event from my earlier post -- onUse -- though it will only matter for consumables.

Anyway, we have an onUse event occur. One of the procs will apply the effect of the item (eg: restore hit points), while the other changes the quantity. However we don't know which is going to happen first.

Well, as it happens, the earlier revisions also allowed for this: give a priority value to the proc.

"procs": [{"name": "adjust weight", "triggerCondition": ["onPickup", "onDrop", "onUse", "onThrow", "onFire", "onBreak", "onDestroy", "onSteal", "onOverflow"]},
          {"name": "use item", "priority": 1, "triggerCondition": "onUse"}]

Assuming a default priority value of 0, this means the "use item" proc will always happen before the "adjust weight" proc.

And, assuming the item class was revised to use the procs list instead of an explicit list, the call would instead be:

    ## Trigger procs that have the specified trigger.
    def triggerProcs(self, user, gameMap, trigger, *args, **kwargs):
        self.procs.runAll(trigger, self, user, gameMap, *args, **kwargs)

However we've finally reached the sticking point that I didn't manage to work out last time -- parameter values.

'onFire', for example, will likely also pass in a target parameter, and maybe the launcher weapon that was used, whereas 'onOverflow' might have the item that was unequipped as one of the parameters. With this mixture of possible values coming in, how do you ensure that "adjust weight" always gets exactly the argument values that it needs, and always mapped to the correct parameter names?

The *args and **kwargs largely makes it transparent as long as the extra parameters are at the far end of the list, and we're only interested in the first parameters. However it's not a guaranteed solution, especially as the probable mix of multiple trigger conditions grows.

I see two possible solutions:

1) Define fixed starting parameters (eg: item, user, target, gameMap) that all procs need to recognize. That ensures that the manner in which it gets called will remain consistent, so that varying parameters don't get mixed in with things that we'll probably always be passing through. Ensures that certain parameters can always be counted on regardless of the mix of parameters. Works as long as procs that handle a mix of trigger conditions are only interested in those fixed parameters.

Downside: Requires procs have parameter slots for things they may not be interested in.

2) Require all args to be passed as keywords. EG: (item=self, user=user, target=None, gameMap=gameMap). That way we don't have to predefine anything, and the proc class only has to worry about the keyword parameters it itself is interested in. Means a proc can handle any mix of trigger conditions, as long as they all provide common paramaters that match what the proc is looking for.

Downside: Requires standardization on various parameter names. If you're going to build a new proc, you have to know what all the other procs have named a particular parameter, and that it agrees with whatever is used when calling the procs. Limits the ability to pass dynamic data.


Needs more thought, again.

[Kinematics]

Another thought on name procs, evolved from the grammar changes and my pondering of the proc discussion.

Looking at the nameProcs that I made, there's almost nothing that isn't data that can be drawn entirely from the item itself. Essentially, there's no -real- reason that all that code can't be in the item object. It was made a proc solely because that's the way it was 'supposed to be'. This has bothered me a bit.

On the other hand, the extra hooks I left in for modification -- alterNameCallbacks -- are themselves exactly what I think name procs are really intended for.

My thoughts on a complete rewrite:

getName() is a normal function in the item class (or secondary class combined with grammar). It contains all the code currently in nameProc, as far as constructing the name.

One small tweak: the article is calculated before the call to grammar.getFullItemName().

Then:

item.getName():
    ** current code**
    
    ** call name procs here **
    
    grammar.getFullName(parts)
    
    return final name

Then the name procs can be stuff like:

uniqueNameProc:
    change article from "a"/"an" to "the"
    -- if identification is defined by procs, how do we determine that cleanly?
    -- priority -1 so that other procs happen first; how do we know if other
    -- 'unidentified' procs happened?

unknownItemProc
    -- initially attached to factory, and copied when item is created.
    -- as they're identified, remove from item and factory.
    -- item can keep a reference to the factory to know whether item type
    -- was identified, and remove itself if so.
    remove any information aside from flavor and modifier/base noun

unidentifiedProc:
    remove any affix information (leave variantName)

unidentifiedNameProc:
    -- for items with different identified and unidentified names
    -- eg: Sword of Chaos vs. Doomcaller
    change base noun value

That means that the called procs have to have access to all the parts of information constructed by the naming function (eg: flavors can't be pre-inserted into the prefix/suffix lists). That alters the structure of what gets passed around a bit, but is fairly minor.

And what parameters need to get passed through the entire run?

[Derakon]

Keep in mind that identification isn't binary -- you can know or not know basically every trait of the item. You learn the name (if unique) when you pick it up, you learn the to-hit/dam pluses when you hit something with it, you learn it gives rFire when you get breathed on, etc. Each of these attributes can show up individually and will modify the name to suit.

[Kinematics]

You learn the name (if unique) when you pick it up

Really? I'd never noticed that happen. It always stays an ordinary item until I identify it.


However the finer granularity is still possible. Have an unknownAffix proc, and one instance per affix. Each time a given affix is identified, remove that proc. If an item is identified, obviously remove all of them.

When constructing the name, for each affix you attempt to add, do a check to see if an unknownAffix proc blocks knowledge of it. Can also have an unknownProperties proc for the basic info.

unknownPropertiesProc (blocks basic properties such as to-hit) [note: this isn't displayed in the present name construction yet anyway]
unknownAffixProc (blocks individual affixes or themes)
unknownItemProc (blocks variantName)
unknownNameProc (blocks artifact names)

In this case, returning a value from the function is probably the best pattern. Simply return True if it exists and it can be considered blocked.

This also implies procs should have an inherent ability to remove themselves to simplify handling:

Hit with fire
Scan through items looking for Resist Fire
For each one found, trigger an UpdateKnowledge(fire) event on that item
Proc receives trigger
If trigger params match what the proc is blocking, call parent.Procs.removeProc(self)


Obviously another instance where a single proc handling multiple triggers is useful.

[Kinematics]

When someone adds a proc, they:
1) give it a name so that it can be referenced externally
2) have to add some point in the code that raises the event (trigger condition) that the proc will make use of
3) add something in the game that is aware of that trigger condition, and will thus be the initiator of the call to the proc

The number of points where the event is raised is limited. An item, for example, will raise the event for each of its onXXXX functions. It would not be difficult to ensure that every event that an item can raise is given the same base arguments -- it already does so for all but getting the name.

Procs can be attached to:
1) player
2) creatures
3) items
4) terrain
5) stat mod

For the procs to be run, something has to be aware of them and initiate them. For a given object type, it only makes sense for its procs to be initiated from an object of that type. One might raise an event with another object type (eg: changing weight might raise an "encumbrance change" event on the player), but it's that other object that will handle calling its own procs.

But then, how would one raise that event on the other object? If I want to raise an "encumbrance change" event on a player, how do I go about that? Do I call a method directly? (eg: player.onEncumbranceChanged()) Or indirectly? (eg: player.raiseEvent("encumbrance change") If I do it directly, I'll need to have all the parameters that go with the event on hand. If I do it indirectly, I can't pass in the parameters that might be needed.

For example, suppose items had a sturdiness rating, and instead of just an "it broke" event, there's a certain amount of damage done, and if it exceeds the sturdiness then it breaks. So I'd need to call an onDamage(type, amount) function. That would then need to call the item's procs (first for onDamage, then if broken call onBreak), passing in the type/amount data (and returning a value?), but will it have user and gameMap values?

So an event handler (onDamage(type, amount) function) needs to be aware of what data needs to go into, and what data needs to be returned from, any procs that are run due to a given trigger condition.

Conceivably, each of these event handlers can have unique requirements on both data to pass in, and data expected out, and any procs that handle that trigger condition have to conform to those expectations.

The problem (and it exists even in the current design) is when you want to write procs that react to an event, but don't necessarily conform to the original needs of the event handler.

Suppose, for example, we added an event handler that made a cracking noise when an item was damaged and an exploding noise when it broke. This is actually a variant on the preparing/doing event split. Another variant would be a proc on the player to make clangy/clashy noises whenever they attack or are attacked by a mob. Anyway..

The onDamage proc wouldn't want to modify the damage (as the onDamage() event handler would normally expect from any onDamage procs); it just wants knowledge that damage was done, and that the item didn't break yet (it needs to have a negative priority so that it's run after all procs that might modify that damage are finished). It probably doesn't even return a value, which is problematic if you're trying to use all the return values to determine the modified damage. sum([1, None, 4]) generates an error when it encounters a NoneType value, so its a potential bug even when packing up the return values in a list.

Plus, as we get back to the earlier example, the argument values that are being passed through are different from the values used in other event calls, so it's difficult to write a proc that can generically accept onDamage as one of its triggers if it also handles other triggers.

There's also always the issue of getting data back and forth.

If the procs return a value, that value can't (generally) be passed on to another proc. So return values are only good if we only expect a single proc to ever exist, and if that's the case we might as well just use a flag and an internal function call.

In the above example about an onDamage proc, each proc on the item should have the opportunity to modify the damage done, and it should be cumulative with previous results. Even though we can get a list of return values, that's more awkward for figuring out a combined effect. It's better if the input to one proc is the output from another proc.

However we can't use output > input mechanics freely, because the output isn't going to match the general input needed (all the other parameters that the function takes). That leaves us with only one easy option: a mutable collection of params. Note that we can't just use the function parameters themselves (as simple as that would make things) since they're (mostly) immutable, at least with respect to fixed pieces of data.

How much do we want to consider mutable or immutable? Is there always a general 'state' of things that one might expect? And what collection of things might you want to be able to change?

For example, any time we handle an event on an item, there's a few bits of state that are always expected: the item object, the user, and the game map info. If an orc picks up an object, is the orc the user? If you're looking at an object across the room, what is the game map telling you? If you fire an arrow, does it include the target creature/object/location? State is a general list of things that you may want to know about, but don't necessarily need to modify.

The params likewise vary per event. onDamage may include how much damage was done. onBreak may include how many objects broke. onEquip needs to know what slot the item is being equipped on. Some of these are fixed values and shouldn't be changed (eg: the equip slot), while others are things we may want to be able to modify. The ones we want to modify need to be enclosed in a mutatable object (ie: list or dict).

Can we encapsulate state? EG: onPickup(self, user, gameMap) -- the item object itself, the user that picked it up, and the game map state at the time. Because user comes in as a parameter, we can't refactor a simple getState() call that would be able to determine that. Querying for the game map info also seems like an awkward handling.

Event handlers in something like C# are very constrained in their signature:

public delegate void EventHandler(
	Object sender,
	EventArgs e
)

Of course part of that is that they *must* have a consistent signature to work in that language, where Python isn't so restricted. However there are still certain benefits from that approach.

The basic event in C# always starts with the sender (ie: the object that called the event). We see an analogous aspect in the trigger signature:

proc.trigger(self, user, gameMap, *args, **kwargs)

Where the trigger starts by sending 'self' (in this case, the item).

It's also reasonable to expect a value for user and gameMap under any conditions that you're raising an event. Those are the three objects that define the overall state that controls the conditions under which the proc was triggered. Those are aspects that I think could be considered pervasive under any proc trigger.

Checking through the existing procs, the basic opening vars are either [terrain, actor, gameMap] or [item, user, gameMap]. actor/user/tunneler/etc are just renames for the same basic entity type - creature and/or player (which is derived from creature). item/terrain/etc are all the 'sender' object, the things that can actually have procs attached to them, and obviously tightly coupled with the proc type in question. And gameMap is the game map.

From that we can extrapolate that any proc must be able handle those three elements in its trigger signature (either explicitly, or just *args), and therefore those parameters must be considered fundamental for all procs.

That allows a certain degree of automatic flexibility -- any proc that is only interested in the event itself and the sender can always count on the sender to be present as the first field, and doesn't have to concern itself with any additional parameters passed in. That is, any proc that is only interested in the fact that the trigger condition was raised, and isn't interested in the particulars of the event. This solves the problem of mixing trigger conditions for the weight proc.

Any proc that is interested in addressing specific aspects of an event (eg: modifying the damage done to an object) must be able to handle the parameter lists required of said event. For a proc that handles multiple trigger conditions, this can be handled via the triggerMaps, as each function that's pointed to can be customized to the needs of the given trigger. This makes it possible to mix trigger conditions when you *do* want to modify the results.

For anything that expects to modify values, they must be able to operate on a mutable object that's provided as a parameter. Designing around that need is something I'll leave for later.

So it looks like the solution to the issue revolves primarily around fix #1: define fixed starting parameters.

[Magnate]

Just popping in to say that it's very interesting that the thinking around naming has led us, via procs, to the discussion of ID. Let's remember that we need to design ID from the ground up, rather than as a by-product of discussing procs ;-)

I favour the approach outlined by Derakon: every property tracked separately as either known or not known.

[Kinematics]

Preparing/Doing

Current:

    ## Respond to the item being picked up.
    def onPickup(self, user, gameMap):
        self.triggerProcs(user, gameMap, 'onPickup')

Revised:

    ## Respond to the item being picked up.
    def onPickup(self, user, gameMap):
        cancelArgs = cancelEventArgs()
        self.triggerProcs(user, gameMap, 'prepareToPickup', cancelArgs)
        if cancelArgs.cancel:
            return
        self.triggerProcs(user, gameMap, 'onPickup')


# Create a class for this since it's going
# to be pretty commonly used
class cancelEventArgs():
    __init__():
        self.cancel = False

Need consistent naming in the trigger conditions. Is "preparingToAction"/"prepareToAction" and "onAction" workable? Or "onActioning" vs "onAction" (eg: "onPickingUp" vs "onPickup"/"onPickUp")? I'm inclined more towards the former, since it feels less awkward for a lot of the verbs we'll be using. "prepareToFire" vs "onFiring", before "onFire".

Should there be space for a message if an action is cancelled? For example, if you try to pick something up and it's too heavy for you to lift, something like:

class cancelEventArgs():
    __init__():
        self.cancel = False
        self.message = []


class someHeavyItemProc():
    def prepareToPickup(self, item, user, gameMap, cancel, *args, **kwargs):
        if user.Strength < 18:
            cancel.cancel = True
            cancel.message.append("%s is too heavy for you to lift!" % self.name)

class item():
    def onPickup(self, user, gameMap):
        cancelArgs = cancelEventArgs()
        self.triggerProcs(user, gameMap, 'prepareToPickup', cancelArgs)
        if cancelArgs.cancel:
            if cancelArgs.message:
                gui.message(cancel.message)
            return
        self.triggerProcs(user, gameMap, 'onPickup')

[Derakon]

I really think that we're better off having tiered Procs that are invoked in tier order, rather than trying to come up with separate trigger conditions for preparing, executing, and cleaning up afterwards. So we have e.g. a default tier of 1, and if you want to go beforehand then you set the tier to 0, and if you want to go afterwards, then you set it to 2. Floating point of course so you can insert whatever tiers you need.

To handle canceling the invocation chain, you could have an extra argument to indicate whether cancellation happens. I don't think having an extra return value for the Proc is a great idea since Proc return values are already used for other things, and I'd rather they be consistent. A third possibility is to set a property on the Proc itself, which the caller would check after invoking the Proc:

def invokeProcs(procs, *args):
    for proc in sorted(procs, key = lambda p: p.tier):
        proc.shouldCancel = False
        proc.trigger(*args)
        if proc.shouldCancel:
            break

The modify-the-Proc approach strikes me as rather hackish, but it bothers me to force every Proc to accept an extra argument (the cancelArgs thing) when the vast majority of them will not care about it. Hm.

Messaging should definitely be up to the Proc involved. Sometimes it will make sense to provide a message when a Proc chain must be cancelled; sometimes it won't.

[Kinematics]

Messaging should definitely be up to the Proc involved. Sometimes it will make sense to provide a message when a Proc chain must be cancelled; sometimes it won't.

Yeah, I was looking it over and realized it would be a lot easier that way. If it was done in the manner with my cancelArgs, then each one that was doing a prepare check could check if cancel was true, and if so just immediately bail out since any additional attempt to cancel would be redundant. That leaves the first one that cancels free to provide a message without worrying about spamming extra messages further down the line.

I really think that we're better off having tiered Procs that are invoked in tier order, rather than trying to come up with separate trigger conditions for preparing, executing, and cleaning up afterwards. So we have e.g. a default tier of 1, and if you want to go beforehand then you set the tier to 0, and if you want to go afterwards, then you set it to 2. Floating point of course so you can insert whatever tiers you need.

Hmm. Will that actually work?

I can see it such that each proc essentially encapsulates the "preparing" plus "doing" phases within itself. For example, the tooHeavyObject.trigger() first checks if you can even pick the item up, and if so bails out, giving a message and seeing proc.shouldCancel to True. Sending a message to the UI is safe(?) at that point, since any other 'preparing' phase procs will have either passed, or won't be run.

However it leaves open a potential issue: Suppose one of the procs sends a UI message when it -succeeds-? If that gets run before the one that cancels, you'll get the succeed message followed by the cancel message. Certainly you can avoid this by using the tiering method, putting the canceling action at a tier that goes before the others, but then what if the other proc needs that as well, since it has its own cancel message? Then you're in a race condition for which one gets executed first.

And while the above example is for displaying a message, it applies for any event that somehow modifies the game state. You need to be sure that any modifications that were done prior to the cancelation are thrown out.

That gets tricky unless you don't allow modification of any game state aside from a variable that can be passed from one proc to the next that will be used to do the actual state modification. Then if any of the procs cancel, you can just toss that variable out and not worry about it.

So, either you only allow 'state' modification of a provided variable that's easily discardable, or you need the multi-stage pipeline for preparing/doing to make sure nothing changes the global state until all of the procs have agreed it's ok.

The modify-the-Proc approach strikes me as rather hackish, but it bothers me to force every Proc to accept an extra argument (the cancelArgs thing) when the vast majority of them will not care about it. Hm.

Not all of them have to handle the extra argument; only the ones that care about cancelling an action you're about to do. 'Standard' proc triggers would still just get the normal [self.triggerProcs(user, gameMap, 'onPickup')]-type function call (which calls the function that loops through all the procs, calling each one with (item, user, gameMap)).

Of course if you use the non-pipelined approach then you don't have the easy separation there. However in that case you'll be required to provide some sort of state-holding var, regardless, so it might as well include its own cancel condition.

data = {"cancel": False, "someGameData": "stuff", etc}
proc.trigger(self, user, gameMap, data)

def trigger(self, sender, user, gameMap, data, *args, **kwargs):
    pass

However that requires setting up an agree-upon dict mapping for stuff that the trigger might need, and many won't really need anything else at all, aside from the basics already provided in sender/user/map.

The main trickiness is that those initial basics themselves already allow changing of game state without discardability. For example, onUse for a potion item may decrement the count on the sender object, completely bypassing the safety net of the data arg. You need to be sure that that can't happen if there's still any possibility of a cancellation.

[Derakon]

I can see it such that each proc essentially encapsulates the "preparing" plus "doing" phases within itself. For example, the tooHeavyObject.trigger() first checks if you can even pick the item up, and if so bails out, giving a message and seeing proc.shouldCancel to True. Sending a message to the UI is safe(?) at that point, since any other 'preparing' phase procs will have either passed, or won't be run.

The issue I have with this is that we'd be coupling "can I do X" with "do X", with the assumption that we will never want the two to be separate. But I could easily see e.g. a Proc that just does "perform a skill check, cancel with this message if failed" that would be attached to many actual "do X" Procs. That's part of why I prefer the tiering approach: we can separate out the conditional Procs from the actually-do-something Procs, and mix and match them as we like.

However it leaves open a potential issue: Suppose one of the procs sends a UI message when it -succeeds-? If that gets run before the one that cancels, you'll get the succeed message followed by the cancel message. Certainly you can avoid this by using the tiering method, putting the canceling action at a tier that goes before the others, but then what if the other proc needs that as well, since it has its own cancel message? Then you're in a race condition for which one gets executed first.

Surely a success message should be printed by the "do something" Proc, not the conditional Proc?

As for if two Procs would both cancel, the fact that there's a race condition doesn't bother me. The Procs will be sorted, so there's a consistent invocation order; the fact that it's arbitrary is a non-issue.

And while the above example is for displaying a message, it applies for any event that somehow modifies the game state. You need to be sure that any modifications that were done prior to the cancelation are thrown out.

That gets tricky unless you don't allow modification of any game state aside from a variable that can be passed from one proc to the next that will be used to do the actual state modification. Then if any of the procs cancel, you can just toss that variable out and not worry about it.

When I started writing Pyrel, a couple of people vaguely agitated for making the game state immutable (i.e. any permutations would be done by creating a new state). We didn't really take the time to hash out exactly how this would work, so I wasn't convinced, and didn't do it. This is one area where I could see it being handy. Unfortunately at this point I'm not really certain it'd be easy to retrofit the system.

Thus my advice would be simply "don't change state in cancellation Procs; always call all cancellation Procs before any state-changing Procs".

[Kinematics]

Thus my advice would be simply "don't change state in cancellation Procs; always call all cancellation Procs before any state-changing Procs".

I was trying to adapt my idea from separate phases to a single phase based on what you seemed to be saying, but it looks like we're back at separate phases (which I agree is a much easier way of going about things).

But I could easily see e.g. a Proc that just does "perform a skill check, cancel with this message if failed" that would be attached to many actual "do X" Procs. That's part of why I prefer the tiering approach: we can separate out the conditional Procs from the actually-do-something Procs, and mix and match them as we like.

Just as a note: separating the 'cancel' procs from the 'doing' procs is sort of an inherent tiering, but it's a logical one that's pretty much required for predictable behavior. In the abstract sense you could consider each category a tier, but you don't need to create tier values for that, just an understanding that there will always be those two phases for just about any trigger -- all cancelables get called, then all actionables get called.

Surely a success message should be printed by the "do something" Proc, not the conditional Proc?

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.

As for if two Procs would both cancel, the fact that there's a race condition doesn't bother me. The Procs will be sorted, so there's a consistent invocation order; the fact that it's arbitrary is a non-issue.

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.

When I started writing Pyrel, a couple of people vaguely agitated for making the game state immutable (i.e. any permutations would be done by creating a new state). We didn't really take the time to hash out exactly how this would work, so I wasn't convinced, and didn't do it. This is one area where I could see it being handy. Unfortunately at this point I'm not really certain it'd be easy to retrofit the system.

Immutability is a wonderful thing, but hard to do right, and not always what we want.

~ Getting into a long tangent on immutability. Will save that for another post.