1.9.13: a weapon with calculated damage changes on level-up

[octalot]

(Might be a bug in the add-on, might be a bug in the engine. Posting here as several other add-ons will need to be fixed too, if it's an add-on bug.)

I'm testing the 1.8 add-on SXCollection on 1.9.13. I haven't isolated a reasonably-minimal chunk of test code yet, but believe I have a delayed-variable-substitution problem:
The add-on gives units new weapons, where the damage and strikes are variables calculated at the time the unit gets the weapon.

In 1.9, on level-up the variables in the effect block seem to get substituted again. (non-AMLA level-up only)
Bug in the add-on, or unexpected behaviour in the engine?

The original code is around line 5001 of SXCollection/macros/SXCmacros.cfg. The nested macros are entered by a right-click menu item, added by SXC_SHOP_MENU.

Code: Select all

#define SXC_BUY_WEAPON ICON NAME ID RNG TYPE DMG STR IMAGE HITS MISSES
   ...
          {VARIABLE damage {DMG}}
          {VARIABLE_OP damage add ${RNG}_damage_$side_number||}
          {VARIABLE strikes {STR}}
          {VARIABLE_OP strikes add ${RNG}_strikes_$side_number||}
          [object]
            silent=yes
            duration=forever
            [effect]
              apply_to=new_attack
              name="sxc_{ID}_$weapons_index|"
              description="{NAME}"
              damage=$damage
              number=$strikes
              range={RNG}
              type={TYPE}
              icon={IMAGE}
            [/effect]
            [effect]
              apply_to=new_animation
              ...
            [/effect]
          [/object]

[Anonymissimus]

Looks like an unfortunate effect of the variable substitution delaying I made in the code handling objects. There are other usecases of [object] which require its wml block to not be variable-substituted when [object] is executed. ([effect][filter] with $this_unit and the new teleport ability)
A testcase could be an [object][effect]key=$var. It should cause $var to be substituted each time the object's effects are reapplied, which happens, for instance, when leveling.
An unintrusive way to solve the contrary requirements could be an [object]delayed_variable_substitution=yes|no key which would need to be set by one of the usecase groups. I'm unsure about the default, "no" matches the previous behavior, but it's "yes" in [event] and always delaying variable substitution as far as possible feels the "right" behavior to me, so your code actually makes use of a previous bug...

EDIT
I suppose that var in the testcase above is not allowed to be cleared or overwritten for as long as the unit exists/as the object's effects can be reapplied in order to keep the appllied effect the same, which is very unfortunate...

[Sapient]

Probably it would be best to go back to the 1.8 behavior and use $|this_unit.x etc for the 1.9 behavior.
Just my opinion...

[Anonymissimus]

Probably it would be best to go back to the 1.8 behavior and use $|this_unit.x etc for the 1.9 behavior.
Just my opinion...

If we do that, then [abilities][teleport] should be reverted to its old non-variable-substitution-using implementation as well. And [effect][filter] not working with $this_unit was a bug which should have been already present in 1.8 which I noticed along the way. And yes, $|this_unit did probably work.

related bugs:
https://gna.org/bugs/index.php?18893
https://gna.org/bugs/?19225

EDIT
fendrin says, [teleport] tag behavior (which was the start of this problem) can't be reverted to its old implementation without removing the [tunnel] feature as well (which could be used now as well). Using $| in ABILITY_TELEPORT is also not an option since that breaks usage in unit_types. So I currently see no way to solve the problem without breaking backwards compatibility in one way or another.