vcmi / vcmi

Open-source engine for Heroes of Might and Magic III
https://vcmi.eu/
GNU General Public License v2.0
4.51k stars 489 forks source link

Water immunity ability is not available for Nymph or Water Elemental #2608

Open by003 opened 1 year ago

by003 commented 1 year ago

Describe the bug Nymph and Water Elemental have the ability of immune harmful water spells, but it is not available. in the code: "immuneToWater" : { "type" : "WATER_IMMUNITY", "subtype" : 2 }

They can still be effected by ice bolt and frost ring.

Did it work earlier? In early version it worked.

Screenshots image

image

Version

rilian-la-te commented 1 year ago

@krs0 now you need to use SPELL_DAMAGE_REDUCTION bonus with subtype spellSchool.water and val of 100.

WATER_IMMUNITY bonus with subtype 2 is no longer functional after 1.2 release.

dydzio0614 commented 1 year ago

https://wiki.vcmi.eu/List_of_all_bonus_types#FIRE_IMMUNITY.2CWATER_IMMUNITY.2C_EARTH_IMMUNITY.2C_AIR_IMMUNITY update needed I guess...

krs0 commented 1 year ago

@rilian-la-te Instead of writing: "waterImunity" : { "type" : "SPELL_DAMAGE_REDUCTION", "subtype" : "spellSchool.water", "val" : 100 },

cant we have a simple proxy that everyone understands like:

        "immuneToWater" : {
            "type" : "WATER_IMMUNITY"
        }

And in background this translates to spell_damage_reduction ...

DjWarmonger commented 1 year ago

Didn't recent secondary skill rework go in exactly opposite way? Bonuses which could be replaced by other bonuses - were replaced.

rilian-la-te commented 1 year ago

@DjWarmonger yes, I had exactly this goal in mind.

rilian-la-te commented 1 year ago

@krs0 there is a spell immunity (so, efreet are immune to bloodlust and other fire buffs etc), negative spell immunity (so, negative fire immune creatures can be bloodlusted, but cannot be cursed) and damage type immunity (nymph like - all buffs and debuffs work, only damage spells did not work).

And third case is handled by SPELL_DAMAGE_REDUCTION, not by XXX_IMMUNITY, because it is damage type, and then reduction of damage by 100% is immunity.

Actually it adds a fine use case - additive spell damage reduction (and achieving immunity if > 100). In H3, we does not have a creature with elemental damage reduction, it is a modding feature.

krs0 commented 1 year ago

So in the end I ended up with this:

            "iceBoltImmunity" :
            {
                "type" : "SPELL_IMMUNITY",
                "subtype" : "spell.iceBolt"
            },
            "frostRingImmunity" :
            {
                "type" : "SPELL_IMMUNITY",
                "subtype" : "spell.frostRing"
            }

that goes 100% hand in hand with: image

rilian-la-te commented 1 year ago

@krs0 it will work only if you do not care about modded spells.

krs0 commented 1 year ago

Can you give an example I do not understand :(

rilian-la-te commented 1 year ago

If I will add MyFancyIceSpell, which will be water analogue of Inferno, it will strike nymphs. If you will use SPELL_DAMAGE_REDUCTION (as I suggested earlier), then it will be ignored. Before 1.2 it was also ignored.

rilian-la-te commented 1 year ago

I will update wiki tomorrow, but for summary - subtype 2 for XXX_IMMUNITY gone for all schools and replaced by SPELL_DAMAGE_REDUCTION with val of 100 and appropriate subtype.

krs0 commented 1 year ago

I got it! Those are the only 2 water damaging spells! Correct description for creatures should be: Immune to water damaging spells and then your fancy Inferno for water would be included. Will change back to your suggestion. Ty for your explanations.

krs0 commented 1 year ago

Guess what other damage spell is in water magic school?... image So I think 1 by 1 listing damage spells will be the way. Or?

rilian-la-te commented 1 year ago

Magic missile? Let's wait for tomorrow or maybe this evening, I will check this in code (fragments with damage reduction).

krs0 commented 1 year ago

So there are 2 cases: immune to all damage spells from a school and immune to all damage spells except for Magic Arrow

krs0 commented 1 year ago

RDT_20230818_0815095618772778802634008

rilian-la-te commented 1 year ago

Fire elementals is immune to Magic Arrow, AFAIK.

And bonus subtype 2 was giving immunity for Magic Arrow too. So, this will require a new handling in code, I guess.

rilian-la-te commented 1 year ago

@krs0 is WoG damage spell immunity affects magic arrow?

krs0 commented 1 year ago

Do not know what specifically are you refering to, but either way we need the 2 cases vanilla H3. With and without magic arrow.

rilian-la-te commented 1 year ago

Why 2 cases? I think we should decide how damage immunity should react to multi-school spells. Now it reacts "by highest", so, if creature immune to damage from water school, it immune to all multi-school damage spells, which schools include water.

But we can check all immunity, not by highest, so, for immunity for multi damage spells, it should be immune for all types of damage this spell provides.

So, for example, if we add a lava spell now, and some creature will have 100% resist to fire, it will be immune to lava. If I will change damage logic, it will not be immune to lava, because it is not immune to earth.

Yes, there is no thing like "school damage immunity" in OH3, and fire immune creatures like Phoenix immune to all school, not only to damage.

rilian-la-te commented 1 year ago

@krs0 it was not working earlier AFAIK, so, I will fix it for develop once and for all. Maybe today or tomorrow.

dydzio0614 commented 1 year ago

there may be some corner cases, like conflux creatures reaction to magic arrow (some elementals were immune, some not, but I am not sure)

rilian-la-te commented 1 year ago

@dydzio0614 we need damage types for correct handling of this case. So, I will add them in next PR)

krs0 commented 1 year ago

@rilian-la-te is this solved? I could find only blanket school reduction, which means magic arrow included.

There is deprecated bonus found:
{
         // wake-of-gods.stackexperience
        "subtype" : 2,
         // wake-of-gods.stackexperience
        "type" : "WATER_IMMUNITY"
}
Trying to convert...
Please, use this bonus:
{
        "subtype" : 2,
        "type" : "SPELL_DAMAGE_REDUCTION",
        "val" : 100
}
rilian-la-te commented 1 year ago

@krs0 only spell-by-spell basis now. In develop it uses spell-by-spell already.