Acid breath damage

International board Development
#1

I am puzzled about the implementation of Acid Breath Damage. In CSpellHandler.cpp the following line makes 2 identical entries in spells array. (As I understood it).

spells.push_back(spells[SpellID::ACID_BREATH_DEFENSE]); //clone Acid Breath attributes for Acid Breath damage effect

What is the reasoning behind that? You have 2 entries pointing to the same thing.

Also in config/spell_info.json acidBreathDamage has a field called “immunities” that should actually be called “immunity” otherwise code silently ignores it.

#2
  • One part of effect is defense reduction
  • Second part of effect is damage

They both can now be configured, for example to alter damage dealt or defense reduction. Or have one and not the other.

Keep in mind, for example, that defense reduction is triggered always, while breath damage is not. I prefer to keep them as separate effects for sanity.

#3

But in code both are pointers to the same spell. They are 100% identical. Or am I not getting something?

#4

Well, this probably worked different before massive spell rewrite. Now all the spell properties are stored in config, anyway.

Of course it would make sense only if both vector entries are different, but even then, I’m not sure if that’s still necesary. You may take a look at it if you want otheriwse I’m not going to fix something that’s not broken.

#5

Actually it may be broken:

spells.push_back(spells[SpellID::ACID_BREATH_DEFENSE]);

And spells vector defined as this:

std::vector< ConstTransitivePtr<CSpell> > spells;

This means that you have two pointers to same CSpell structure instead of two identical structures. I suppose that code should have used copy constructor here instead of copying pointer.

Probably we should add schema to this file. Will fix.