MantisBT - VCMI
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0002796VCMIMechanics - Otherpublic2017-09-19 07:072018-03-12 21:08
Reporterhkoehler 
Assigned Tohkoehler 
PrioritynormalSeverityminorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version1.next 
Target Version1.nextFixed in Version 
Summary0002796: Merging of secondary skill bonuses does not check limiters/propagators/updaters
DescriptionIn 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.

Steps To ReproduceExample 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"
            }
        }
    }
}
Additional InformationThe approach of updating existing bonus isn't reliable. Instead, existing bonuses from secondary skill should be stripped, and replaced by new bonuses.
TagsNo tags attached.
Relationships
Attached Files
Issue History
Date ModifiedUsernameFieldChange
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.