MantisBT - VCMI
View Issue Details
0002796VCMIMechanics - Otherpublic2017-09-19 07:072018-03-12 21:08
hkoehler 
hkoehler 
normalminoralways
resolvedfixed 
0.next 
0.next 
0002796: Merging of secondary skill bonuses does not check limiters/propagators/updaters
In CGHeroInstance.cpp, bonuses from secondary skills are merged as follows:

void CGHeroInstance::updateSkill(SecondarySkill which, int val)
{
    auto skillBonus = (*VLC->skillh)[which]->getBonus(val);
    for (auto b : skillBonus)
    {
        // bonuses provided by different levels of a secondary skill are aggregated via max (not + as usual)
        // different secondary skills providing the same bonus (e.g. ballistics might improve archery as well) are kept separate
        std::shared_ptr<Bonus> existing = getBonusLocalFirst(
            Selector::typeSubtype(b->type, b->subtype).And(
            Selector::source(Bonus::SECONDARY_SKILL, b->sid).And(
            Selector::valueType(b->valType))));
        if(existing)
            vstd::amax(existing->val, b->val);
        else
            addNewBonus(std::make_shared<Bonus>(*b));
    }
    CBonusSystemNode::treeHasChanged();
}

This will cause bugs when a skills grants multiple bonuses differing on values not checked.

Example that produces bug:

"core:firstAid" : {
    "base" : {
        "effects" : {
            "dura" : {
                "type" : "STACK_HEALTH",
                "valueType" : "PERCENT_TO_BASE",
                "val" : 300,
                "limiters": [
                    {
                        "type" : "CREATURE_TYPE_LIMITER",
                        "parameters" : [ "firstAidTent" ]
                    }
                ]
            }
        }
    },
    "basic" : {
        "description" : "{Basic First Aid}\n\nGrants manual control of the first aid, increases healing to 50 points, and increases tent durability.",
    },
    "advanced" : {
        "description" : "{Advanced First Aid}\n\nFirst aid tent heals 75 points, and troops have 10% more health.",
        "effects" : {
            "xtra" : {
                "type" : "STACK_HEALTH",
                "val" : 10,
                "valueType" : "PERCENT_TO_BASE"
            }
        }
    },
    "expert" : {
        "description" : "{Expert First Aid}\n\nFirst aid tent heals 100 points, and troops have 20% more health.",
        "effects" : {
            "xtra" : {
                "type" : "STACK_HEALTH",
                "val" : 20,
                "valueType" : "PERCENT_TO_BASE"
            }
        }
    }
}
The approach of updating existing bonus isn't reliable. Instead, existing bonuses from secondary skill should be stripped, and replaced by new bonuses.
No tags attached.
Issue History
2017-09-19 07:07hkoehlerNew Issue
2017-09-19 07:07hkoehlerStatusnew => assigned
2017-09-19 07:07hkoehlerAssigned To => hkoehler
2018-03-12 21:08hkoehlerNote Added: 0007499
2018-03-12 21:08hkoehlerStatusassigned => resolved
2018-03-12 21:08hkoehlerResolutionopen => fixed

Notes
(0007499)
hkoehler   
2018-03-12 21:08   
Fixed and merged.