Practice/Rank-lock on skills and solution proposal

[Krogar]

A couple of days after the release of the CB version I found that I was unabled receive any practice in my axe skill anymore, and thus cannot rank up. Well, not a major problem - we\'re betatesting, after all. However, when I realised that a dwarf with a longsword looks like he\'s going for a pole vault competition I was actually looking into where the problem lies.

The problem:
It seems that at one point during development there has been a change to the ruleset (rpgrules.xml), and the training costs for some skills were obviously reduced.
With training already received on the character, this resulted in the training already received (Skill.y) being higher than the actual training requirements (Skill.yCost).
However, in Skill::Practice (src/server/bulkobjects/pscharacter.cpp) this case is unaccounted for:

Code: [Select]


bool Skill::Practice( unsigned int amount, unsigned int& actuallyAdded,psCharacter* user )
{
    bool rankup = false;
    // Practice can take place
    if ( yCost == y )
    {
    <....add practice, check for rankup...>

    }
    <...else...>
    return rankup;
}


The training received from a trainer (y) has to be equal to the training requirements that are calculated in the mathscript (yCost).
This shouldn\'t be a problem normally, as the training does not increase beyond the skill\'s requirement - unless the mathscript is changed i.e. for balancing reasons, which can result in skills becoming un-rankable again.

Proposed solution:
As a solution I suggest in adding a little safety check to Skill::CalculateCosts (src/server/bulkobjects/pscharacter.cpp):

Code: [Select]


void Skill::CalculateCosts(psCharacter* user)
{            
    <...cut: scriptvar init, script exec...>

    // Get the output
    yCost = (int)yCostVar->GetValue();
    zCost = (int)zCostVar->GetValue();

    // added: Sanity check for modified cost requirements :
    if(y > yCost) y = yCost;
    if(z > zCost) z = zCost;
}

While the z/zCost may not seem problematic so far, I think it\'s a good idea to put it in anyway to prevent unwanted side effects, like wrongly rendered statbars in pawsskillwindow.cpp.
(also, it may have been sufficient to run this check over all skills in psCharacter::LoadSkills or Skills::Calculate - but I think it really belongs into CalculateCosts, since y>yCost may lead to problems in most cases).

This change should not only fix current character skill-locks (like, my own ;) ), but also avoid future problems with unpracticable/unrankable skills caused by changes/balance tweaks in the scripts.