Allow decreasing lower tier skills

[Iturio]

Sometimes (couldn't find a reproduceable cause) it's not possible to remove spent points, even if there aren't any depending talents or still enough points spent. Most of the time it occurs if any talent from the next level gets improved. For example: You can spent 10 points on the first level and as soon as you put 1 point into any talent from the second level, you can't reduce any points from the first level (even as the second level should only require 5 points on the first level).

Tested with Chrome: Version 74.0.3729.131 (Official Build) (64-Bit)

[Iturio]

I guess it's related to the "skill/level", which is shown at the bottom. You need to remove the talents in the same (reversed) order as you spent them. Was this intended (maybe it's just a confusing requirement for your app)?

[hseager]

@Iturio Thanks for the feedback! Yeah this was intentional as I had some issues with the logic around decreasing higher levels when removing lower level talents. To get the first version out I only allowed the removal of talents at the highest tier of that tree.

A lot of feedback from the reddit thread was around this so I'm going to have another look when I get a chance!

[hseager]

I've had a few attempts at this and I'm still having trouble working out how to calculate which skills need to be reset when reducing lower tier skills without any bugs. Any help on how this logic should work would help, although I'll keep looking into it

[Malorh]

Since I have opened a few tickets on issues I will just comment on this.

I hope I have understood your comment correctly. I understand it as "which points can legally be removed?"

I do not code in anything but C# and MatLab (lol, I know ;) but really; MatLab does make some pretty figures for scientific papers and presentations), so I chose not to start reading your code.

Just so we agree on syntax: "lower (level) tier" is the ones up highest visually in the trees, e.g. the lowers tier is tier one and is available at lvl 10. Similarly, "higher (level) tiers" are those available later.

If I had to implement it, I would probably do the following, when a user tries to remove points in a tier:

1. Are there any points in the later tiers than the one you are trying to remove a point in?
   • no => nothing to test for
   • yes => do everything below
2. For each tier up to the second to last tier (higher tiers) with points, starting in the second to last tier, working down to lower tiers: do the following
   1. Sum up the number of spend talent points in the current and previous tiers
   2. Is this sum minus one (the one point the user is trying to remove) greater than or equal to five (five, since that is the requirement pr tier) times the current tier (the i'th tier)?
      • no => do not allow removing the point
      • yes => so far so good, but now repeat this "for each" loop for a lower tier
3. If the point can legally be removed, update the table. Also update the level list (i.e. which points the user takes in the levels): All points are shifted to one earlier level.

NB: Technically, the order of tiers you test is irrelevant. I just think that there is a higher chance to catch an illegal action if you start from the higher tiers. I guess this completely irrelevant since we are talking so few operations anyway.

Example 1: You have seven points in tier one, six points in tier two, and two points in tier three (total: 7+6+2 = 15). Let's say we want to remove a point in tier one (which should be legal): We start in tier "three minus one" (i.e. the second to last with points): The sum is "6+7-1 = 12". Test: Is 12 greater than i'th tier (which is "2") times 5? 12 >=? 2x5. Yes/True. Good. Let's continue the iteration: We are now in tier one. The sum is "7-1 = 6". Test: Is 6 greater than the i'th tier (which is "1") times 5? 6 >=? 1x5. Yes/True. There are no more lower tiers, we need to test for. All tests have passed => the point can legally be removed. The point should then be removed in the table and in the level list below the table.

Example 2: Tier one has five points, tier two has five points, tier three has one point. Try to remove a point in tier two: The sum is "5+5-1 = 9". Test: 9 >=? 2*5? False => it is illegal to remove this point. There is no need to test other tiers, since we already have one tier with a test resulting in false/illegality.

In order to do this, you need to have some counter telling you the highest tier in which you have points, but I guess you already have that in some way. If not, I guess it is easy to add.

I do not claim this is necessarily the prettiest solution, but I think it should work :)

[Mikewa33]

This is in react but I got the logic fixed. I didn't implement the skill selected at each level so that may still be a problem but this logic makes sure the user is allowed a valid decrease

isValidDecrease = () => {
    if(this.props.skill.currentRank === 0){
      return false;
    }
    if(this.hasAdjacentSkillRequirement()){
      return false;
    }
    if (this.props.skill.position[0] === this.props.currentSkillTier) {
      return true;
    }

    let isValid = true;
    let firstTierTotal = 0;
    let secondPlusTierTotal = 0;
    this.props.tree.skills.forEach((skill) => {

      if (skill.requirements)
      {
        if (skill.requirements.specPoints >= this.props.tree.skillPoints - 1 && skill.currentRank !== 0 ) {
          secondPlusTierTotal = secondPlusTierTotal + skill.currentRank;
          isValid = false;
        }
      }
      else if(skill.position[0] === 1){
        firstTierTotal = firstTierTotal + skill.currentRank;
      }
    });

    if(secondPlusTierTotal > 0 && firstTierTotal <= 5) {
      return false;
    }

    return isValid 
  }

[hseager]

@Mikewa33 Thanks for the code. I've just tried it out and although it does allow you to decrease lower levels, it allows you to spec out the following which is invalid:

I've tested in the old vanilla client and in this example you would have to have 10 points in the first 2 tiers before you can add them to the 3rd. This is where I've been having a lot of trouble in my previous attempts.

[Mikewa33]

My bad I caught that and forgot to update the comment. Try out this. It should fix the issue. It is a little heavier on the loops but it makes sure that each skill is still valid

isValidDecrease = () => {
    if (this.props.skill.currentRank === 0) {
      return false;
    }
    if (this.hasAdjacentSkillRequirement()) {
      return false;
    }
    if (this.props.skill.position[0] === this.props.currentSkillTier) {
      return true;
    }
    if (this.props.skill.requirements) {
      if (this.props.skill.requirements.skill) {
        if (this.props.skill.requirements.skill.skillPoints !==  this.props.tree.skills[this.props.skill.requirements.skill.id].currentRank) {
        return false;
        }
      }
    }

    let isValid = true;
    let firstTierTotal = 0;
    let secondPlusTierTotal = 0;
    let tiers = [];

    // This checks to see if it can deselected and won't break any requirements of lower tier skills
    this.props.tree.skills.forEach((skill) => {
      tiers[skill.position[0] - 1] = (tiers[skill.position[0] - 1] || 0) + skill.currentRank
      if (skill.requirements) {
        if (skill.requirements.specPoints >= this.props.tree.skillPoints - tiers[skill.position[0] - 1] && skill.currentRank !== 0 ) {
          secondPlusTierTotal = secondPlusTierTotal + skill.currentRank;
          isValid = false;
        }
      }
      else if(skill.position[0] === 1){
        firstTierTotal = firstTierTotal + skill.currentRank;
      }
    });

    if(secondPlusTierTotal > 0 && firstTierTotal < 5) {
      return false;
    }

    return isValid 
  }

[hseager]

@Mikewa33 I've tried out the code above and there are a few issues where it doesn't reset the higher tier skills when you decrease a lower tier.