Angband.oook.cz
Angband.oook.cz
AboutVariantsLadderForumCompetitionComicScreenshotsFunniesLinks

Go Back   Angband Forums > Angband > Variants

Reply
 
Thread Tools Display Modes
Old March 12, 2013, 01:41   #11
Kinematics
Apprentice
 
Join Date: Feb 2013
Posts: 71
Kinematics is on a distinguished road
The ability to override an existing proc, though, is more difficult. Replacing a proc entirely is easy, but suspending it and putting it back when the current one goes away takes a little more finesse.

The standard logic for processing procs is to iterate over the list and allow each one to execute. However you may sometimes only want a single one to execute (eg: only use the single highest resistance value; the previous example of making a creature look like an item; etc).

It could be done if you abstract out the proc management instead of leaving it as a simple list that other code can directly manipulate.

Than you could have:

Code:
funcs = item.myProcs.getProcFunctions(trigger)
for f in funcs:
    f(~params)

procs.getProcFunctions(trigger):
    validProcFuncs = [p.getFunction(trigger) for p in self.procs]
    validProcFuncs = [p for p in validProcFuncs if p]
    maxPriority = max(validProcFuncs, key = lambda p: p.priority)
    return [p for p in validProcFuncs if p.priority == maxPriority]
It inherently introduces another layer of abstraction to do this, though it could be argued as a good thing, even without the priority-handling code.

However even that allows for multiple returns of the highest priority procs, so we're not out of the woods yet.

Note: we actually already have a situation where this comes into play with normal vs unique items. We get around it right now because the procs aren't actually entered as procs in the object data; only the name of the proc is listed in the nameInfo struct, and then the proc is determined from that. However, logically, nameInfo shouldn't have a "proc" entry, and the proc should be explicitly entered as such in the item record.

Or suppose we had a mimic that got hit with petrification, thus leading to two procs at priority 1 (as opposed to the default 0). We're back at the situation of two procs activating when we only want to allow one. You can somewhat dance around that by varying the priority values, but I think it would be better to handle it explicitly.

We can probably assume that, in such a situation, we always want the most recently applied, highest priority effect. Or, in computing terms, we want this to be a stack. Except we also want it sorted by priority.

We can also control the addition and removal of procs more precisely if we have an abstracted class for handling that.
Code:
procs.addProc(proc):
    nextHigherPriorityIndex = next((p for p, q in enumerate(self.procs) if q > proc.priority), None)
    if nextHigherPriorityIndex:
        self.procs.insert(proc, nextHigherPriorityIndex)
    else
        self.procs.append(proc)
Then the procs will be listed in order of both least-recent to most-recent, and low-priority to high-priority. You could then check if the top of the stack (last one in the list) considers itself 'singular'. If so, only return that item. Otherwise, return the list as normal.

However that seems more like something you'd need to know the logic of at the calling code level (eg: when wanting to get the item name) rather than internally to the procs, so probably better to have the encapsulating class provide two functions: getAllProcs, or getTopProc.
Kinematics is offline   Reply With Quote
Old March 12, 2013, 02:19   #12
Derakon
Prophet
 
Derakon's Avatar
 
Join Date: Dec 2009
Posts: 8,921
Derakon is on a distinguished road
Rather than try to teach each Proc how to handle being overridden and come up with a sensible way to order the Procs (which makes defining each individual Proc slightly more complicated), perhaps instead we could have meta-Procs that operate on the Proc list immediately prior to triggering. We would add the following atttributes to the base Proc class:

* isMetaProc: a boolean, determines if the Proc is invoked normally or as part of the self-examination process (see below)
* isActive: a boolean, if False then the Proc is not triggered.

These would default to False and True respectively, if not specified otherwise in the data file.

When we go to trigger Procs for a given trigger condition, first we look up all the MetaProcs with that trigger. Each of them is called (in undetermined order) with the list of Procs as an additional argument to its trigger function, and is allowed to mutate the list as it sees fit. Procs can be disabled by modifying their isActive boolean. If you want to modify a Proc's behavior, then you can disable it and insert a new Proc that does something different. Et cetera. Once all of the MetaProcs have been invoked, the normal Procs in the now-modified list are invoked.

(Note that a MetaProc can e.g. create a new Proc, and also spawn a timered event to remove it from the list later without requiring the MetaProc's trigger to be called again. This should suffice to handle any bookkeeping cleanup.)

Of course this system does not allow for recursion -- MetaMetaProcs. In practice I doubt this will be needed, but if it is then we can just replace the isMetaProc boolean with a metaLayer integer, and invoke Procs in order based on that value. Still, eliminate complexity where it is not needed, hence why we start with booleans.
Derakon is offline   Reply With Quote
Old March 12, 2013, 02:24   #13
Derakon
Prophet
 
Derakon's Avatar
 
Join Date: Dec 2009
Posts: 8,921
Derakon is on a distinguished road
Quote:
Originally Posted by Magnate View Post
No. I have wanted to move rings and amulets to affix-based generation for as long as I've thought about affixes. This is just a handy incentive to get started. It is true that this doesn't necessarily mean that consumables also need to move to the same system, but I can see advantages beyond the naming issue.
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.
Derakon is offline   Reply With Quote
Old March 12, 2013, 03:27   #14
Kinematics
Apprentice
 
Join Date: Feb 2013
Posts: 71
Kinematics is on a distinguished road
Went to write this procs wrapper class, and found that it seemed to work better a bit different than I originally thought. The class as a whole is pretty simple.

This moves the code to run a proc's triggers into the procs class, exposing just two functions: runAll() and runTop(). runTop() encapsulates the logic of running the most recently added, highest priority proc.

runAll() returns a list of all the results, while runTop() just returns the result of the function that was run.

Code:
class Procs(list):
    # Add a proc to the procs list.
    # Inserted according to priority value, and with most
    # recent entry last.
    def addProc(self, proc):
        nextHigherPriorityIndex = next((p for p, q in enumerate(self) if q > proc.priority), None)
        if nextHigherPriorityIndex:
            self.insert(proc, nextHigherPriorityIndex)
        else:
            self.append(proc)


    # Get the first proc that matches a provided predicate
    def getProc(self, predicate):
        proc = next((q for p, q in enumerate(self) if predicate(q)), None)
        return proc


    # Get all procs that match a provided predicate
    def getProcs(self, predicate):
        return [p for p in self if predicate(p)]


    # Remove the specified proc.
    def removeProc(self, proc):
        self.remove(proc)


    # Run all procs that match the provided trigger, using the
    # specified arguments list.
    # Returns a list of the results of all proc calls.
    def runAll(self, trigger, *args):
        results = []
        for proc in self:
            procFunc = proc.triggerFunction(trigger)
            if procFunc:
                results.append(procFunc(args))
        return results


    # Run the most recent, highest priority proc that matches
    # the provided trigger.
    # Returns the result of the proc.
    def runTop(self, trigger, *args):
        for proc in self[::-1]:
            procFunc = proc.triggerFunction(trigger)
            if procFunc:
                return procFunc(args)

On your ideas:

onActive would be a simple way to switch other procs off and only have a single one run, however I'm not sure how it would determine which other proc to turn back on if/when the active one is removed.

isMetaProc allows for some significant power, but this time I'm the one saying, do you really need it?

Though I suppose my Procs class is like a tiny version of that. It handles maintaining an order for procs to run in (so higher priority procs will always run after lower priority ones, and recent ones after older ones), and allows you to select a mode for running procs (one or all). I can't think of anything offhand that would require moving to the next stage of allowing complete free-form rearranging of proc run order, or cherry-picking which ones to run.

Although.. I think I added a bit of that into the stats code, with the filter option (eg: get all temporary stat mods). Though you can do something a bit similar with Procs.getProcs(predicate). Or add a simple runProc() function to go with the getProc/getProcs functions to allow custom runs.


Code:
    # Run the specified proc, if it's one of our procs.
    def runProc(self, proc, trigger, *args):
        if proc in self:
            procFunc = proc.triggerFunction(trigger)
            if procFunc:
                return procFunc(args)


    # Run all the procs in the provided lists that are
    # one of our procs.
    def runProcs(self, procList, trigger, *args):
        results = []
        for proc in procList:
            if proc in self:
                procFunc = proc.triggerFunction(trigger)
                if procFunc:
                    results.append(procFunc(args))
        return results

Done this way, you can do meta procs without a special flag. Just pass in the procs class as one of the parameters to another proc.
Kinematics is offline   Reply With Quote
Old March 12, 2013, 05:38   #15
Kinematics
Apprentice
 
Join Date: Feb 2013
Posts: 71
Kinematics is on a distinguished road
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.
Kinematics is offline   Reply With Quote
Old March 12, 2013, 09:36   #16
Magnate
Angband Devteam member
 
Join Date: May 2007
Location: London, UK
Posts: 5,057
Magnate is on a distinguished road
Send a message via MSN to Magnate Send a message via Yahoo to Magnate
Quote:
Originally Posted by Kinematics View Post
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.
__________________
"3.4 is much better than 3.1, 3.2 or 3.3. It still is easier than 3.0.9, but it is more convenient to play without being ridiculously easy, so it is my new favorite of the versions." - Timo Pietila
Magnate is offline   Reply With Quote
Old March 12, 2013, 13:31   #17
Magnate
Angband Devteam member
 
Join Date: May 2007
Location: London, UK
Posts: 5,057
Magnate is on a distinguished road
Send a message via MSN to Magnate Send a message via Yahoo to Magnate
Quote:
Originally Posted by Derakon View Post
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 ??"
__________________
"3.4 is much better than 3.1, 3.2 or 3.3. It still is easier than 3.0.9, but it is more convenient to play without being ridiculously easy, so it is my new favorite of the versions." - Timo Pietila
Magnate is offline   Reply With Quote
Old March 13, 2013, 04:09   #18
Kinematics
Apprentice
 
Join Date: Feb 2013
Posts: 71
Kinematics is on a distinguished road
Created a separate branch to fix up some of the things I noted in the earlier overview. Holding, pending review of main grammar branch.

Quote:
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 is offline   Reply With Quote
Old March 13, 2013, 05:18   #19
Kinematics
Apprentice
 
Join Date: Feb 2013
Posts: 71
Kinematics is on a distinguished road
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:
Code:
"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:
Code:
    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.

Code:
"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.

Code:
"procs": [{"name": "adjust encumbrance", "triggerCondition": ["onPickup", "onDrop", "onThrow", "onFire", "onBreak", "onDestroy", "onSteal", "onOverflow"]}]
Though maybe you should break that into logical unique components.

Code:
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 is offline   Reply With Quote
Old March 13, 2013, 06:21   #20
Kinematics
Apprentice
 
Join Date: Feb 2013
Posts: 71
Kinematics is on a distinguished road
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:
Code:
    ## 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.
Code:
"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:
Code:
    ## 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 is offline   Reply With Quote
Reply


Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump

Similar Threads
Thread Thread Starter Forum Replies Last Post
Pyrel dev log, part 4 Magnate Variants 116 March 11, 2013 19:53
Pyrel dev log, part 3 Derakon Variants 108 November 12, 2012 23:30
Affixes in Pyrel Magnate Development 0 October 13, 2012 13:53
Pyrel dev log, part 2 Derakon Variants 126 September 11, 2012 23:03
Pyrel dev log Derakon Variants 64 June 8, 2012 11:58


All times are GMT +1. The time now is 07:20.


Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2019, vBulletin Solutions Inc.