Ball lightning doesn't move after casting

Regression was introduced by 54e11c21cf, the starvation bug fix. Found by git bisect.

Since I was curious, I poked around a little:

Ball lightning is spell type SP_MOVING_BALL, which calls spell_util.cpp:fire_arch_from_position() when cast.

Note that spell_util.cpp:fire_arch_from_position() line 696-698 sets both a duration and a food value when casting.

Note that ball lightning also sets the is_used_up flag inside ball_lightning.arc

The fix for the starvation bug will remove an object that has the 'used up' flag set, when there is also a duration set. This causes the ball lightning to be removed immediately after casting,

This also affects poison fog, negative energy ball, and I assume divine shock (although I'm having trouble test the last one)

Proposed fix is inside fire_arch_from_position(), and removes setting the duration field of certain spells.

--- a/server/spell_util.cpp
+++ b/server/spell_util.cpp
@@ -693,9 +693,9 @@ int fire_arch_from_position(object *op, object *caster, int16_t x, int16_t y, in
         }

         tmp->stats.dam = spell->stats.dam+SP_level_dam_adjust(caster, spell);
-        tmp->duration = spell->duration+SP_level_duration_adjust(caster, spell);
-        /* code in time.c uses food for some things, duration for others */
-        tmp->stats.food = tmp->duration;
+        /* code in time.c uses food for some things, duration for others,
+         * but can't use both at the same time since commit 54e11c21cf */
+        tmp->stats.food = spell->duration+SP_level_duration_adjust(caster, spell);
         tmp->range = spell->range+SP_level_range_adjust(caster, spell);
         tmp->attacktype = spell->attacktype;
         if (object_get_owner(op) != NULL)

Some testing and notes:

fire_arch_from_position() is called for spell subtypes SP_BULLET, SP_MAGIC_MISSILE, SP_MOVING_BALL. My proposed fix does not change any behaviour for the spell subtype SP_BULLET, but will change any other subtypes (anything other than SP_BULLET).

fire_arch_from_position() is called inside the swarm (SP_SWARM spell subtype) logic, which currently only spawn SP_BULLET or SP_MAGIC_MISSILE subtypes.

fire_arch_from_position() is also called for alchemy failure, and spawns a SP_MED_FIREBALL arch, which is mapped to spell_medium_fireball, subtype 5 / SP_BULLET.

This means there are a number of spells to test, [x] indicates tested and passed, [ ] is untested:

• SP_MOVING_BALL (35):
  • Ball Lightning
  • ball moves around, is that enough?
  • Poison Fog
  • cloud moves around, is that enough?
  • Divine Shock
  • Negative Energy Ball
  • works the same as ball lightning
    SP_MAGIC_MISSILE / (11)
  • [x] Magic Missile
    SP_SWARM (36)
  • Bullet storm -> SP_BULLET
  • Bullet swarm -> SP_BULLET
  • Cause many wounds -> SP_BULLET
  • Frost nova -> SP_BULLET
  • meteor swarm -> SP_BULLET
  • missile swarm -> SP_MAGIC_MISSILE

More notes:
- SP_BULLET spells decrement range, and does not use duration.
- fire_arch_from_position() Sets a fixed range of 50.
- SP_MAGIC_MISSILE spells decrement range, as it calls SP_BULLET logic.
- fire_arch_from_position() uses the caster level to determine range.
- SP_MOVING_BALL spells don't decrement anything, therefore use food to remove the object.
- SP_SWARM spells decrement duration, but that only for keeping the swarm alive. The projectiles created are either SP_BULLET or SP_MAGIC_MISSILE, which both use range

Conclusion:

It seems that SP_MOVING_BALL and SP_MAGIC_MISSILE do not use duration to determine how long the spell lives. Therefore it is safe to update fire_arch_from_position() to only set the food value instead of setting both food and duration. This ensures that that process_object() in time.c will use the food value to determine the time to live, and not immediately remove the spell.

I've pushed the workaround to cph/fix-moving-ball-spells so it doesn't get lost in the shuffle.

Also happens to divine shock, just tested. Does not happen with meteor swarm.