eResistance: Some are 0-based, others are 1-based

[OnlineCop]

Elemental effects on weapons, spells, and items are sometimes 0-based and other times 1-based.

• s_spell::elem is 0-based, with 0 == eResistance::R_EARTH and 16 == eResistance::R_TOTAL_RES.
• s_item::elem is 1-based, with 0 == "no imbue", 1 == eResistance::R_EARTH, and 16 == eResistance::R_TIME.
• KFighter::welem is 1-based.

Within res.cpp for the s_item items[] array:

{ "Frozen Star",     1,  244, "",                       ..., eResistance::R_ICE,      ... }
{ "Death's Head",    1,  121, "Undead destroyer",       ..., eResistance::R_WHITE,    ... }
{ "Stun Hammer",     2,  99,  "",                       ..., eResistance::R_PARALYZE, ... }
{ "Thor's Hammer",   2,  232, "Thunder powered hammer", ..., eResistance::R_THUNDER,  ... }
{ "Aichasi Knife",   5,  3,   "Dark blade",             ..., eResistance::R_BLACK,    ... }
{ "Rod of Fire",     7,  18,  "Summon fire",            ..., eResistance::R_FIRE,     ... }
{ "Gloom Rod",       7,  1,   "Dark power",             ..., eResistance::R_BLACK,    ... }
{ "Crystal Rod",     7,  228, "Call lightning",         ..., eResistance::R_THUNDER,  ... }
{ "Pentha Staff",    8,  50,  "Staff of the Wind",      ..., eResistance::R_AIR,      ... }
{ "Maham Staff",     8,  211, "Staff of Water",         ..., eResistance::R_WATER,    ... }
{ "Krendar's Brew",  23, 0,   "Remove Silence",         ..., eResistance::R_AIR,      ... }
{ "Nidana Poultice", 23, 0,   "Restore Vision",         ..., eResistance::R_EARTH,    ... }
{ "Rune of Air",     25, 0,   "Air Element",            ..., eResistance::R_THUNDER,  ... }
{ "Rune of Fire",    25, 0,   "Fire Element",           ..., eResistance::R_BLACK,    ... }
{ "Rune of Water",   25, 0,   "Water Element",          ..., eResistance::R_WHITE,    ... }

These are all the weapons/runes/item with a non-zero s_item::elem value. In the example above, I changed the values from 1-based to 0-based and then assigned their enum values to make it more readable.

Most of the values are correct, but some clearly have the wrong values for the elemental effect (the "Rune of Air" should probably be R_AIR, and "Nidana Poultice" probably shouldn't be non-zero).

Within allstat.mon, the KFighter::welem values are assigned, and this is their "adjusted to 0-based" enum equivalents:

Demnas          38     ...  eResistance::R_EARTH,
Jolina          44     ...  eResistance::R_BLACK,
Blaze           48     ...  eResistance::R_FIRE,
ShadowHound     50     ...  eResistance::R_BLIND,
Troll           60     ...  eResistance::R_EARTH,
DreadBat        67     ...  eResistance::R_POISON,
Treeman         70     ...  eResistance::R_EARTH,
Swampman        71     ...  eResistance::R_EARTH,
Sapstrider      72     ...  eResistance::R_EARTH,
Scorpion        74     ...  eResistance::R_POISON,
SalamanderMage  77     ...  eResistance::R_FIRE,
Salamander      78     ...  eResistance::R_FIRE,
ElectricEel     79     ...  eResistance::R_THUNDER,
SeaSnake        81     ...  eResistance::R_POISON,
JellyBeast      84     ...  eResistance::R_THUNDER,
PinkOcto        87     ...  eResistance::R_SLEEP,
BlueOcto        88     ...  eResistance::R_WATER,
BrownOcto       89     ...  eResistance::R_EARTH,
Mumak           92     ...  eResistance::R_EARTH,
ForestTroll     95     ...  eResistance::R_EARTH,
DoomToad        96     ...  eResistance::R_POISON,
DeathRose       97     ...  eResistance::R_CHARM,
Manticore       99     ...  eResistance::R_POISON,
Harpy           100    ...  eResistance::R_CHARM,
Cockatrice      101    ...  eResistance::R_PETRIFY,

That is, this is what each of their non-spell attacks are imbued with.

1. The void infusion() function assigns to KFighter::welem (ftr.welem) with 0-based values, instead of 1-based.
   • This should be assigning 0 for "no imbue" otherwise +1 values for each.
2. The combat_skill() function assigns to KFighter::welem (tempa.welem) with the eResistance value + 1.
   • This is correct.
3. KCombat::attack_result() assigns attacker_weapon_element = tempa.welem (1-based) and later, sets c = attacker_weapon_element - 1 (0-based)to do all the evaluation, such as calling KMagic::res_adjust() with the now 0-based value.
   • This is correct.
4. Within item_effects(), s_item::elem (tmp = items[ti].elem) (1-based) is passed to KMagic::res_adjust() without subtracting 1.
   • This should be 0-based.
5. s_spell::elem is 0-based.
   1. KFighter::SetInfuse() takes s_spell::elem as a parameter from within skill_use(), although it appears to be used as a "how long should this effect last for" value.
   2. res_throw() and res_adjust() take s_spell::elem as a parameter from KMagic::beffect_all_enemies(), which expects the value to be 0-based.
      • This is correct.

[OnlineCop]

Looking at the s_spell magic[] array, these are the status effects. I sorted these first by s_spell::icon value (column 2) and then by the eResistance type (column 5).

{ "",              0,  "",                       ..., eResistance::R_EARTH,     ... },

{ "Repulse",       40, "Repel monsters",         ..., eResistance::R_TOTAL_RES, ... },
{ "Vision",        40, "See enemy power",        ..., eResistance::R_TOTAL_RES, ... },

{ "Warp",          41, "Warp out of danger",     ..., eResistance::R_TOTAL_RES, ... },

{ "Death",         43, "Eliminate life",         ..., eResistance::R_BLACK,     ... },
{ "Doom",          43, "Drain life",             ..., eResistance::R_BLACK,     ... },
{ "Blind",         43, "Blind one opponent",     ..., eResistance::R_BLIND,     ... },
{ "Confuse",       43, "Confuse one opponent",   ..., eResistance::R_CHARM,     ... },
{ "Stone",         43, "Petrify one enemy",      ..., eResistance::R_PETRIFY,   ... },
{ "Slow",          43, "Slow enemy reactions",   ..., eResistance::R_TIME,      ... },
{ "Absorb",        43, "Steal magic points",     ..., eResistance::R_TOTAL_RES, ... },
{ "Diffuse",       43, "Remove defenses",        ..., eResistance::R_TOTAL_RES, ... },
{ "Drain",         43, "Steal hit points",       ..., eResistance::R_TOTAL_RES, ... },
{ "Malison",       43, "Dehabilitate opponents", ..., eResistance::R_TOTAL_RES, ... },
{ "Nausea",        43, "Sicken opponents",       ..., eResistance::R_TOTAL_RES, ... },

{ "Tornado",       44, "Twister",                ..., eResistance::R_AIR,       ... },
{ "Whirlwind",     44, "Whirlwind attack",       ..., eResistance::R_AIR,       ... },
{ "Flood",         44, "Waves of water",         ..., eResistance::R_WATER,     ... },
{ "Tsunami",       44, "Tidal waves",            ..., eResistance::R_WATER,     ... },
{ "Fade",          44, "Positive energy attack", ..., eResistance::R_WHITE,     ... },
{ "Lumine",        44, "Holy blast",             ..., eResistance::R_WHITE,     ... },

{ "Cure1",         45, "Light cure",             ..., eResistance::R_TOTAL_RES, ... },
{ "Cure2",         45, "Mild cure",              ..., eResistance::R_TOTAL_RES, ... },
{ "Cure3",         45, "Strong cure",            ..., eResistance::R_TOTAL_RES, ... },
{ "Cure4",         45, "Full cure",              ..., eResistance::R_TOTAL_RES, ... },

{ "Full Life",     46, "Return to full life",    ..., eResistance::R_TOTAL_RES, ... },
{ "Life",          46, "Return to life",         ..., eResistance::R_TOTAL_RES, ... },
{ "Recovery",      46, "Recover all status",     ..., eResistance::R_TOTAL_RES, ... },
{ "Restore",       46, "Heal blind and poison",  ..., eResistance::R_TOTAL_RES, ... },

{ "Hasten",        47, "Speed boost",            ..., eResistance::R_TIME,      ... },
{ "Quicken",       47, "Speed boost for all",    ..., eResistance::R_TIME,      ... },
{ "Bless",         47, "Empower allies",         ..., eResistance::R_TOTAL_RES, ... },
{ "Holy Might",    47, "Power increase",         ..., eResistance::R_TOTAL_RES, ... },
{ "Regenerate",    47, "Regenerate hp",          ..., eResistance::R_TOTAL_RES, ... },
{ "Shell",         47, "Magic defense up",       ..., eResistance::R_TOTAL_RES, ... },
{ "Shield All",    47, "Defense boost to all",   ..., eResistance::R_TOTAL_RES, ... },
{ "Shield",        47, "Defense boost",          ..., eResistance::R_TOTAL_RES, ... },
{ "Through",       47, "Etherealness",           ..., eResistance::R_TOTAL_RES, ... },
{ "True Aim",      47, "Can't miss",             ..., eResistance::R_TOTAL_RES, ... },
{ "Wall",          47, "Improved magic defense", ..., eResistance::R_TOTAL_RES, ... },
{ "Divine Guard",  47, "Shield and Shell",       ..., eResistance::R_WHITE,     ... },

{ "Hold",          48, "Paralyze an opponent",   ..., eResistance::R_PARALYZE,  ... },
{ "Silence",       48, "Prevent spellcasting",   ..., eResistance::R_SILENCE,   ... },
{ "Sleep All",     48, "Put all to sleep",       ..., eResistance::R_SLEEP,     ... },
{ "Sleep",         48, "Put opponent to sleep",  ..., eResistance::R_SLEEP,     ... },

{ "Gloom",         49, "Negative energy attack", ..., eResistance::R_BLACK,     ... },
{ "Negatis",       49, "The Void",               ..., eResistance::R_BLACK,     ... },
{ "Earthquake",    49, "Earthquake attack",      ..., eResistance::R_EARTH,     ... },
{ "Tremor",        49, "Shake it up",            ..., eResistance::R_EARTH,     ... },
{ "Fire Blast",    49, "Mild fire attack",       ..., eResistance::R_FIRE,      ... },
{ "Flame Wall",    49, "Strong fire attack",     ..., eResistance::R_FIRE,      ... },
{ "Scorch",        49, "Weak fire attack",       ..., eResistance::R_FIRE,      ... },
{ "Blizzard",      49, "Snow and ice attack",    ..., eResistance::R_ICE,       ... },
{ "Frost",         49, "Snow and ice attack",    ..., eResistance::R_ICE,       ... },
{ "Plague",        49, "Disease attack",         ..., eResistance::R_POISON,    ... },
{ "Venom",         49, "Poison attack",          ..., eResistance::R_POISON,    ... },
{ "Virus",         49, "Viral infection",        ..., eResistance::R_POISON,    ... },
{ "Lightning",     49, "Lightning attack",       ..., eResistance::R_THUNDER,   ... },
{ "Shock",         49, "Shocking strike",        ..., eResistance::R_THUNDER,   ... },
{ "Thunder Storm", 49, "Thunderstorm",           ..., eResistance::R_THUNDER,   ... },
{ "X-Surge",       49, "Non-elemental attack",   ..., eResistance::R_TOTAL_RES, ... },

Most of these look correct, although a few seem to have the wrong eResistance effect for the s_spell::elem value. Or maybe I just don't fully understand what the spell is supposed to be doing, and I just need to adjust its s_spell::icon.

[OnlineCop]

Several of these had been discovered during PR #116, but as that was a massive rabbit hole that changed a lot of various files, I might need to break it down into smaller PRs.

[pedro-w]

It's tricky old code is that; when we were first trying to understand the code base we just skirted round that part

[OnlineCop]

I'm working on this currently.

I started to merge data/allstat.mon and data/resabil.mon into data/enemies.xml (since we already have tinyxml2), but it's been taking me longer than I like to get the import/export functions to work, so I'll probably just append the values to the end of one of them, and figure out how to refactor it all later.

Getting these to 0-based should also fix some of the "why does an enemy casting Doom somehow heal my party?" kind of odd bugs that I've seen cropping up.

[OnlineCop]

Okay, good-news, bad-news time: pull #141 is setting up the changes necessary to do the actual "let's fix the data!" portion of the eResistance values.

• Bad news: That's a big PR.
• Good news: I tried [really, really hard] to keep all commits focused on only 1 change at a time. Sometimes those span multiple files, but they were all related, such as "renaming a variable" or "adding comments".
• Bad news: The next PR which has the actual res.cpp changes loses all sorts of its current formatting (you may want a very wide monitor!).
• Good news: All of the Magic Numbers will be changed (as much as possible) into enum values, so the names should be a lot easier to understand what something is doing.
• More good news: I also lined up the columns (partly for my own sanity, and partly to help in mass edits) so that should make things easier to read (again, sorry for the horizontal scroll).
• Bad news: What's there currently should be 0-based (that's good news, though, right?), but there's a lot of data, and I keep spotting places where the item/spell type isn't exactly what the enum says the effect is going to be.

That last point will probably need community help to sift through everything before I merge that PR in.

[OnlineCop]

141 and #142 have been merged.

The lines in res.cpp are quite long, and I'm fine reformatting them after we do some thorough play-throughs to ensure that spells and items we cast/use are actually doing what we expect them to.

It's also possible that the code is in place to handle a lot of the "strong" weapons/armor that the game has logic for, but none of the maps actually provide the player with chests containing them, so they may never be encountered.