[CF-Devel] patch for spell system

[Mark Wedel]

david delbecq wrote:
>
     
     
     >
     
      I wrote a big modification on the way spells are used in the code. This is
     
     >
     
      only internal change and does
     
     >
     
      not affect the gameplay. The chages include:
     
     
 I'd be nice if this was brought up before the code was done.  It seems like
this comes up often.  The reason I say that is this entry from the TODO list:

- IDEA: Make spells be objects.  One object for each spell you know.  Same
  system for the monsters.  For now let the objects be invisible.
  Then later create a spellbook into which the spells can be "put".
  Thus knowing a spell consists of having the spell-object in some
  spellbook.  With this, more properties of the spell (level, cost, etc)
  from the current array to the spell object.  Alternatively, spellbooks
  have the spell objects that are then put in the players inventory when
  the spell is learnt.  scrolls, wands, potions would just be objects with
  some spell object as an inventory.
  As a further extension, make general spell casting code, which can look
  at the spell object and come up with effects.  For example, right now
  the cones could be pretty generalized - all that is really different
  is the attack form (could be determined by attack type), area of effect
  (some other value in the object), damage, spell cast, level, etc.  In
  other words, the spell objects could pretty much provide all the information
  needed for some of the more general functions (cast cone, fire_bolt, etc.)

 That above change is not completely orthagonal to your code, but certainly your
code did a lot of work that isn't really necessary.

  In the above example, since all spells are just archetypes, tuning them is
just a matter of changing the archetype, not a configuration file.

 If the code is abstract enough, it means that new spells are just a matter of
modifying the arch on the map.  Eg, if different paramaters control damage,
attacktype, size of spell (length of code, expansion of fireball, etc), making a
'mongo fireball' spell just means taking the large fireball and increase the
area of effect value.

 This approach means you don't even have spell numbers.  Given your code below,
these archetypes would just have the function that should be called in one of
their fields.  This also simplifies some areas of the code - since knowing a
spell is having that spell object in your inventory, it means the loading and
storing of the spells for the player file are no longer need - just the normal
object loading code is used.

 This also meaks some things easier to examine - rather than needing to look up
a spell number to see what spell that spellbook may contain, you just look at
the object inside the spellbook.  I think this may make it easier for mapmakers.

 Given that is the eventual goal, may comments below:

>
     
     
     >
     
       *  The spells internals have been separated from the rest of the code and are
     
     >
     
      only accessed using
     
     >
     
      functions in a file called spell_library.c. There should be no more access to
     
     >
     
      the spells[] table or the
     
     >
     
      spellarch[] table
     
     >
     
       *  The spells are now saved in a list and not in a table. This allow to have,
     
     >
     
      for example, spells numbers
     
     >
     
      from 0 to 203 then spells from 500 to 504, then 600 to 602 and so on. There
     
     >
     
      may be holes in the list.
     
     >
     
      This should help creating new spells since you get a set of number, keep them
     
     >
     
      even if someone else is
     
     >
     
      creating newspells at the same time. This also open the door for people
     
     >
     
      wanting to allow players
     
     >
     
      creating spells (get a free number and keep it)
     
     >
     
       *   All spells have their internals saved in a file called spellbook. This
     
     >
     
      file may be tuned by system
     
     >
     
      administrator, the same way the old file spell_params did, but in a larger and
     
     >
     
      more human readable way.
     
     >
     
      At the bottom of this mail, there is some example line from the file
     
     
 Those three points would be non issues if spells are objects. 

>
     
        *  The spells are casted using a callback in the new structure. This allow
     
     >
     
      plugins to reference spells in
     
     >
     
      the server. However the hooks for referencing a spell from a plugin has not
     
     >
     
      been written yet :(
     
     
 I think a similar mechanism if spells are objects in place, eg, the spell would
have a callback field that is used when the spell is cast.

>
     
        *   The file spellbook in the shared crossfire dir is created automagically
     
     >
     
      by the server if not available.
     
     >
     
      This is done using thed old spells[] structure and the old spell_param file.
     
     
 Reasonable.  I'm not sure how many people actually modify the spells.  The
spell_params file was really put in place so that spells could be tweaked
without needing to recompile the server.  IF spells are objects, that is not
relevant, but I will certainly admit that going to a spell object system will
result in some legacy code (eg, need to support the old spell number system for
some amount of time for things like wands and scrolls, but that could be made
easier by the spell archetypes having a spell number that is used for that
purpose).  Likewise, player loading would need to convert the known_spell ..
values into spell objects.

>
     
         *   The new structure also opens the door for a new kind of spell: the
     
     >
     
      localised spells. This will be done
     
     >
     
      using a command fire_at <deltax><deltay> instead of fire <direction> so we
     
     >
     
      could create spells that aim
     
     >
     
      a position and not a direction
     
     
 The support for this mainly seems to be in the need to say that this is an
aimed spell vs directional - that of course could have been done in the old
system just by adding another value to the spell field.

 While I think this change is a good idea, I think the bigger problem is the
actual implementation of this (eg, the player need some way to say fire at
x,y).  I think that is the bigger thing to do - some spells should probably be
in both categories, eg, directional and aimed at some specific spot.

 I say that because for almost all spells combat spells, you want to be able to
fire it off quickly in some direction.  However, if your with a party or other
considerations, being able to aim some spell at a specific point would be nice.

 Given that, I think the callback should have something like 'direction,
offset_x, offset_y' parameter.  If the offsets are set, and the spell is
aimable, those are used.  Otherwise, direction is used.

 Thus, for example, if I fire a spell north, direction would be 1, the offsets
0, so it is a normal spell.  If the player tries to aim a spell that is not
aimable, he gets some error message. 

 I say that because I think to add such functionality is to some extent a client
issue to issue something like a 'fireat <x><y>' command.  How the player does
that in the client is certainly a client issue, but I would guess it would
probably involve clicking on some space with the mouse, which is certainly
something slower to do.

 Note that the fireat command could also be used for the bow attacks, and
monsters should also be able to used the aimed spells (which would make a group
of spellcasting monsters much nastier, as the ones in back could cast spells at
the player).  The aimed fire code does need to do some sanity checking of course
- you can't fire through walls  or otherwise blocked terrain.
 
>
     
      I plan to cvs the code next week. But since it is a big change, i send a patch
     
     >
     
      for the moment.
     
     
 It would be nice if people actually discussed what they were working on before
writing the code.

 A few comments:

 I dislike code/functions like:
 
+ /* callback name: int sp_cast_raise_dead
+  *       used by: raise dead, resurrection
+  */
+ int sp_cast_raise_dead (object *op,object *caster,int dir,spell_data *sp,int
ability,SpellTypeFrom item,char *stringarg)
+   {
+   return cast_raise_dead_spell(op,dir,sp->type, NULL);

 I'd much rather that the target function (cast_raise_dead_spell) get modified
to use the same parameters - This extra function generally makes code more
confusing.  Using such wrappers is generally easy to do, but if done enough,
results in a complete mess of code.
 
 
>
     
      -- A part of the spellbook file --
     
     >
     
     
     >
     
      spell magic bullet
     
     >
     
      ################################
     
     >
     
      # Core server Spell N°0
     
     >
     
      # a level 1 wizard spell
     
     >
     
      #
     
     >
     
      id= 0
     
     >
     
      level= 1
     
     >
     
      sp= 1
     
     >
     
      charges= 99
     
     >
     
      time= 2.000000
     
     >
     
      scrolls= 0
     
     >
     
      scroll_chance= 0
     
     >
     
      book_chance= 10
     
     >
     
      range?= 1
     
     >
     
      defensive?= 0
     
     >
     
      cleric?= 0
     
     >
     
      onself?= 0
     
     >
     
      targeting?= 0
     
     >
     
      path= 0x0010
     
     
 I'd also much rather have values stored in such config files to be text when
appropriate, eg, have PATH_HEALING or PATH_.. instead of someone needing to go
through the docs to find the right value.

 Yes, I know that this isn't done in lots of places right now, but we need to
start somewhere.