Opened 8 years ago

Last modified 6 years ago

#1041 assigned task

Refactor effects to allow variables

Reported by: magnate Owned by: magnate
Milestone: v4 Keywords: cleanup
Cc:

Description

Inspired by #1038, the effects framework could be tidied up a lot if effects were of the form:

TYPE: ELEMENT: DAM[: RAD]

e.g. BOLT: ACID: 5d8 or BREATH: CHAOS: 200: 2

instead of ACID_BOLT2 etc.

This would also make #992 a lot easier.

Change History (10)

comment:1 Changed 8 years ago by magnate

  • Milestone changed from 3.1.3 to Future
  • Status changed from new to confirmed

Postponing at Takkaria's request on IRC.

(16:06:39) takkaria: magnate: arr, don't do anything like that with effects yet please
(16:08:05) takkaria: magnate: eventually I would like to add a mini scripting language to specify effects... but it's a while off yet and I'm cautious about doing it before 3.2 or so

comment:2 Changed 7 years ago by magnate

  • Type changed from change to task

comment:3 Changed 6 years ago by takkaria

OK, so I'd like to see a proposal on how to move forward with this. The way things are done at the moment was only ever considered a temporary stopgap to make things just a little bit cleaner and less duplicated than before and was not meant as the final word.

The question is what to do from here. I'll put forward a bunch of considerations which I think are important.

  1. The effects framework was made in the first place to standardise certain values game-wide. So, for example, a staff or scroll of identify use the same effect, confusion/poison/etc from items have the same duration, healing effects with the same have the same effect (I'm aware this is incomplete), and so they are defined in the same place.

While many of these considerations may no longer apply, I think the idea that you have effects defined in one place and invoked in another is quite important, so that if you want to tweak 'cure light wounds', you get to tweak it everywhere without lots of effort, and it's one way of ensuring that effects with the same name have the same effect (important for making the game easier to understand).

  1. Adding an extra random_value variable that gets sent to all effects so that some can use it is not helpful because power ratings and object descriptions need to vary with effects (though these already don't take into effect the power boost when using devices). It also means that effects are not defined in one place and used in another, but are sort-of defined in code and sort-of in the edit files.
  1. I think it would also screw up parsing, since there are many things we might want to parameterise, and amending the 'E:' line to include various different kinds of parameterised effects will have horrible results. e.g.
    E:BOLT:FIRE:5d8
    E:HEAL:60
    E:HASTE:75+1d75
    
    would requie the parser to read in the first field, and based on its contents, parse the second (and maybe third) field differently. It would make a lot more sense, I think, to define an effect "FIRE_BOLT_5d8" or "HASTE_75-150" and then invoke those effects on different items. I think these considerations lead to the idea that we should have an effect.txt file which specifies effects such that each effect has its own parsing instruction, e.g.
    EFFECT:FIRE_BOLT_5d8
    NEEDS:DIRECTION
    BOLT:FIRE:5d8
    
    EFFECT:CURE_LIGHT
    HEAL:20
    CLEAR_TIMED:BLIND
    DEC_TIMED:CUT:20
    DEC_TIMED:CONFUSED:20
    
    however, this is far from problematic, since not all effects will be possible to make fully parameterisable. Even moderate steps in this direction woud increase editability, though. With an approach like this, generating accurate effect descriptions would be possible (or they could be provided in the edit file).
  1. There is currently an issue with using the 'n' command to repeat the reading of enchant scrolls (or other effects which require item selection): the item selected does not get remembered as part of the command. If we're going to rework item effects, I think we should be doing so with an eye to fixing this problem. This would require the definition of effects to include whether they needed an object and what kind of object it would have to be, in the same way that some effects require a direction.

comment:4 Changed 6 years ago by takkaria

I don't think any of these are insurmountable and I'd welcome some debate to hash out a good proposal to move forward with, even if it's not the best possible plan. I get the feeling what I'm looking for and what the bug was originally filed in aid of are not quite the same.

Note that advantage of having effects.txt separate is that we can make sure that all the different effects (HEAL, CLEAR_TIMED, INC_TIMED, etc.) are being passed the right parameters, and tell the user at parse-time if things are wrong. This reduces the change of introducing game-crashing bugs with badly edited edit files. Note that the syntax is pretty much a miniature scripting language (imagine the sub-effects as function calls and colons as commas...). This is pretty much what I had in mind when I was talking about a mini scripting language.

comment:5 Changed 6 years ago by magnate

Ok, continuing from #1427, I think we have two conceptual issues with effects:

  1. We are conflating simple effects with a single outcome (heal some hp, id an object, set a timed status) from compound effects with multiple outcomes (CLW, RAGE_BLESS_RESIST, etc.). I would prefer that the effects framework was composed entirely of single effects, and that compound effects were achieved with multiple E: lines in object.txt (and similarly the multiple effects of spells or GF_ types could be handled this way too).
  1. We seem to disagree about the right way to deal with the variable quantities of an effect. I think there should be one EF_HEAL, that takes a parameter "amount" (which can be fixed or random, thanks to the random_value struct). I agree with you that any adjustments to this parameter (e.g. for clev or device skill or whatever) should be made before the effect is called. But it seems crazy duplication to me to define a different effect for each different amount of healing.

These two issues are related, because passing multiple variables into compound effects would get really ugly. One effect, one variable. (And a mode bitflag, to encode stuff like NEEDS_AIM | NEEDS_OBJECT etc.)

comment:6 Changed 6 years ago by magnate

Sorry, forgot to add: I think this is the way to deal with multiple side effects of GF_ types: one line in list-side-effects for each effect, and a GF_ type can have as many lines as necessary. Each line has only one random value, which is the variable passed to the effect. Different GF_ types can therefore call different effects with different strengths. I don't think it answers the question of whether project() needs more than one variable passed into it, but I'll do some more thinking about that.

comment:7 Changed 6 years ago by takkaria

OK, so re 1: I think the code should be implementing so-called 'simple effects' (heal x%, heal x HP, inc_timed, clear_timed, etc.), but I think there's value in also having a higher-level grouping of those effects into effect definitions (in an external file) like CLW, CSW, etc. so that every place where a spell/object uses CLW, it doesn't re-specify the same stuff over and over (having four different places to update if you want to tweak one effect isn't a good, I think). This, instead of having multiple E lines in the object file, though in effect it is exactly the same, it just prevents duplication. How does that sound?

comment:8 Changed 6 years ago by magnate

Sounds good. I don't really understand why it's important that the CLW spell, potion and activation are all identical (since they're not at the moment and that seems fine), but I agree that a file of higher-level effect groupings is the right way to do that.

Perhaps this answers the second point as well: the single effects could (if you agree) take a variable, but the compound effects wouldn't take separate variables for each effect - they'd use pre-defined values (which could be computed from a passed-in caster level or device skill etc.).

comment:9 Changed 6 years ago by magnate

  • Milestone changed from Future to 4.0

Initial assignment to v4 per http://trac.rephial.org/roadmap

comment:10 Changed 6 years ago by magnate

  • Owner set to magnate
  • Status changed from confirmed to assigned

Ok, I've started work on this, but it's too big for my brain so I'm trying to break it down. Progress will be documented on the NewEffects page.

Note: See TracTickets for help on using tickets.