Issue Information
-
#007471
-
4 - High
-
Fixed
Issue Confirmations
-
Yes (4)No (0)
i'm facing 2 issues here upon updating (note that my last pull was last tuesday)
1. The message that used to show you require to use 50 for the 1st job is missing.
2. It seems that upon reset, you're able to immediately add your skills to 3rd job page without going through 1st and second.
Please confirm thanks!
// When set to yes, forces skill points gained from 1st class to be put into 1st class
// skills, and forces novice skill points to be put into the basic skill. (Note 1)
player_skillup_limit: yes
Ever since granted skills use SKILL_FLAG_PERMANENT, and thus are counted in pc_calc_skillpoint() towards total used skillpoints, whereas they should not.
Thus probably on all afflicted servers jobchange_level and/or jobchange_level_3rd are too high, thus it makes you use skill points where it shouldnt. I didnt test this but I'm fairly confident I'm right. So basically you will probably want to revert https://github.com/H...28997148bcc5f55 as malufett pointed out (it's basically a re-implementation of an already existing function with hardcoded job levels), and re-visit that job skill issue ( http://herc.ws/board...removing-gears/ ).
Hope I helped
Edited by Wildcard, 03 August 2013 - 05:38 PM.
first HIIIIIIIII been so long oo <3, I've gone over the references you made and, in 1c9fdc0d flag 3 causes those skills to use SKILL_FLAG_PERM_GRANTED not SKILL_FLAG_PERMANENT and as I glanced it does not count towards skill pointsPretty sure this was caused in 1c9fdc0d3bbe2993ac1e8a31045ccbc60b8fbe26.
Ever since granted skills use SKILL_FLAG_PERMANENT, and thus are counted in pc_calc_skillpoint() towards total used skillpoints, whereas they should not.
Hope I helped
first HIIIIIIIII been so long oo <3, I've gone over the references you made and, in 1c9fdc0d flag 3 causes those skills to use SKILL_FLAG_PERM_GRANTED not SKILL_FLAG_PERMANENT and as I glanced it does not count towards skill points
Pretty sure this was caused in 1c9fdc0d3bbe2993ac1e8a31045ccbc60b8fbe26.
Ever since granted skills use SKILL_FLAG_PERMANENT, and thus are counted in pc_calc_skillpoint() towards total used skillpoints, whereas they should not.
Hope I helped
sir I don't think so it seems you missed something in here
@status.c
if(!battle_config.skillfree) { int j; for(j = 0; j < MAX_PC_SKILL_REQUIRE; j++) { int k; if((k=skill_tree[c][i].need[j].id)) { int idx2 = skill_tree[c][i].need[j].idx; if (sd->status.skill[idx2].id == 0 || sd->status.skill[idx2].flag == SKILL_FLAG_TEMPORARY || sd->status.skill[idx2].flag == SKILL_FLAG_PLAGIARIZED) k = 0; //Not learned. else if (sd->status.skill[idx2].flag >= SKILL_FLAG_REPLACED_LV_0) //Real lerned level k = sd->status.skill[idx2].flag - SKILL_FLAG_REPLACED_LV_0; else k = pc->checkskill2(sd,idx2); if (k < skill_tree[c][i].need[j].lv) { f = 0; break; } } } if( sd->status.job_level < skill_tree[c][i].joblv ) f = 0; // job level requirement wasn't satisfied }
oooh interesting thank you.sir I don't think so it seems you missed something in here
first HIIIIIIIII been so long oo <3, I've gone over the references you made and, in 1c9fdc0d flag 3 causes those skills to use SKILL_FLAG_PERM_GRANTED not SKILL_FLAG_PERMANENT and as I glanced it does not count towards skill points
Pretty sure this was caused in 1c9fdc0d3bbe2993ac1e8a31045ccbc60b8fbe26.
Ever since granted skills use SKILL_FLAG_PERMANENT, and thus are counted in pc_calc_skillpoint() towards total used skillpoints, whereas they should not.
Hope I helped
@status.cif(!battle_config.skillfree) { int j; for(j = 0; j < MAX_PC_SKILL_REQUIRE; j++) { int k; if((k=skill_tree[c][i].need[j].id)) { int idx2 = skill_tree[c][i].need[j].idx; if (sd->status.skill[idx2].id == 0 || sd->status.skill[idx2].flag == SKILL_FLAG_TEMPORARY || sd->status.skill[idx2].flag == SKILL_FLAG_PLAGIARIZED) k = 0; //Not learned. else if (sd->status.skill[idx2].flag >= SKILL_FLAG_REPLACED_LV_0) //Real lerned level k = sd->status.skill[idx2].flag - SKILL_FLAG_REPLACED_LV_0; else k = pc->checkskill2(sd,idx2); if (k < skill_tree[c][i].need[j].lv) { f = 0; break; } } } if( sd->status.job_level < skill_tree[c][i].joblv ) f = 0; // job level requirement wasn't satisfied }
first HIIIIIIIII been so long oo <3, I've gone over the references you made and, in 1c9fdc0d flag 3 causes those skills to use SKILL_FLAG_PERM_GRANTED not SKILL_FLAG_PERMANENT and as I glanced it does not count towards skill points
Pretty sure this was caused in 1c9fdc0d3bbe2993ac1e8a31045ccbc60b8fbe26.
Ever since granted skills use SKILL_FLAG_PERMANENT, and thus are counted in pc_calc_skillpoint() towards total used skillpoints, whereas they should not.
Hope I helped
Hi :3
indeed it's been a while~
I don't have an up-to-date server set up anymore so I cannot test any of this, but I think item and script granted skills use flag 0, and thus get counted? If I have the time to set everything up again I may take a look
sir Ind this is because you didn't revert yet your previous attempt fix for this(https://github.com/H...28997148bcc5f55)..revert it then try to check if it works..Sir Ind this is not fix the bug. Now it's ask 1more skill points for class 1 and 2 more skills points for class 2.
since this part of the code is in charge for the checking if possible to add skill point
if( sd->status.skill_point > 0 && sd->status.skill[index].id && sd->status.skill[index].flag == SKILL_FLAG_PERMANENT && //Don't allow raising while you have granted skills. [Skotlex] sd->status.skill[index].lv < skill->tree_get_max(skill_id, sd->status.class_) ) { sd->status.skill[index].lv++; sd->status.skill_point--; if( !skill_db[index].inf ) status_calc_pc(sd,0); // Only recalculate for passive skills. else if( sd->status.skill_point == 0 && (sd->class_&MAPID_UPPERMASK) == MAPID_TAEKWON && sd->status.base_level >= 90 && pc->famerank(sd->status.char_id, MAPID_TAEKWON) ) pc->calc_skilltree(sd); // Required to grant all TK Ranger skills. else pc_check_skilltree(sd, skill_id); // Check if a new skill can Lvlup clif->skillup(sd,skill_id); clif->updatestatus(sd,SP_SKILLPOINT); if( skill_id == GN_REMODELING_CART ) /* cart weight info was updated by status_calc_pc */ clif->updatestatus(sd,SP_CARTINFO); if (!pc->has_permission(sd, PC_PERM_ALL_SKILL)) // may skill everything at any time anyways, and this would cause a huge slowdown clif->skillinfoblock(sd); }cause in your previous commit its like you remove its functionality and also with the skill tree normalization function...
I tried as you proposed (like this: malu-testfix.patch 3.51K 7 downloads ) and it allowed the original bug to happen again (was able to put skills in 3rd class skill tree w/o having spent the points accordingly ), is there something I am missing when testing?sir Ind this is because you didn't revert yet your previous attempt fix for this(https://github.com/H...28997148bcc5f55)..revert it then try to check if it works..
Sir Ind this is not fix the bug. Now it's ask 1more skill points for class 1 and 2 more skills points for class 2.
since this part of the code is in charge for the checking if possible to add skill pointif( sd->status.skill_point > 0 && sd->status.skill[index].id && sd->status.skill[index].flag == SKILL_FLAG_PERMANENT && //Don't allow raising while you have granted skills. [Skotlex] sd->status.skill[index].lv < skill->tree_get_max(skill_id, sd->status.class_) ) { sd->status.skill[index].lv++; sd->status.skill_point--; if( !skill_db[index].inf ) status_calc_pc(sd,0); // Only recalculate for passive skills. else if( sd->status.skill_point == 0 && (sd->class_&MAPID_UPPERMASK) == MAPID_TAEKWON && sd->status.base_level >= 90 && pc->famerank(sd->status.char_id, MAPID_TAEKWON) ) pc->calc_skilltree(sd); // Required to grant all TK Ranger skills. else pc_check_skilltree(sd, skill_id); // Check if a new skill can Lvlup clif->skillup(sd,skill_id); clif->updatestatus(sd,SP_SKILLPOINT); if( skill_id == GN_REMODELING_CART ) /* cart weight info was updated by status_calc_pc */ clif->updatestatus(sd,SP_CARTINFO); if (!pc->has_permission(sd, PC_PERM_ALL_SKILL)) // may skill everything at any time anyways, and this would cause a huge slowdown clif->skillinfoblock(sd); }cause in your previous commit its like you remove its functionality and also with the skill tree normalization function...
As I understand the previous code doesn't work because:I tried as you proposed (like this: malu-testfix.patch 3.51K 7 downloads ) and it allowed the original bug to happen again (was able to put skills in 3rd class skill tree w/o having spent the points accordingly ), is there something I am missing when testing?
sir Ind this is because you didn't revert yet your previous attempt fix for this(https://github.com/H...28997148bcc5f55)..revert it then try to check if it works..
Sir Ind this is not fix the bug. Now it's ask 1more skill points for class 1 and 2 more skills points for class 2.
since this part of the code is in charge for the checking if possible to add skill pointif( sd->status.skill_point > 0 && sd->status.skill[index].id && sd->status.skill[index].flag == SKILL_FLAG_PERMANENT && //Don't allow raising while you have granted skills. [Skotlex] sd->status.skill[index].lv < skill->tree_get_max(skill_id, sd->status.class_) ) { sd->status.skill[index].lv++; sd->status.skill_point--; if( !skill_db[index].inf ) status_calc_pc(sd,0); // Only recalculate for passive skills. else if( sd->status.skill_point == 0 && (sd->class_&MAPID_UPPERMASK) == MAPID_TAEKWON && sd->status.base_level >= 90 && pc->famerank(sd->status.char_id, MAPID_TAEKWON) ) pc->calc_skilltree(sd); // Required to grant all TK Ranger skills. else pc_check_skilltree(sd, skill_id); // Check if a new skill can Lvlup clif->skillup(sd,skill_id); clif->updatestatus(sd,SP_SKILLPOINT); if( skill_id == GN_REMODELING_CART ) /* cart weight info was updated by status_calc_pc */ clif->updatestatus(sd,SP_CARTINFO); if (!pc->has_permission(sd, PC_PERM_ALL_SKILL)) // may skill everything at any time anyways, and this would cause a huge slowdown clif->skillinfoblock(sd); }cause in your previous commit its like you remove its functionality and also with the skill tree normalization function...
You click to level a 3rd-class-skill on the client, client sends lv-up requests in necessity order e.g. in a ranger if you reset skills and try to level verdure trap straight away: it sends 12 requests in order:
1 HT_SKIDTRAP
2 HT_FLASHER
3 HT_FREEZINGTRAP
4 HT_SANDMAN
5 HT_LANDMINE
6 HT_ANKLESNARE
7 HT_BLASTMINE
8 HT_SHOCKWAVE
9 HT_REMOVETRAP
10 HT_CLAYMORETRAP
11 RA_RESEARCHTRAP
12 RA_VERDURETRAP
which fullfills the skill-tree requirements of the previous and makes sd->status.skill[idx].id be non-0, it then skips the previous code since it passes on the first check (which is why I had previously moved the check to before the .id check)
therefore problem relies on 'pc_calc_skilltree_normalize_job'which fullfills the skill-tree requirements of the previous and makes sd->status.skill[idx].id be non-0, it then skips the previous code since it passes on the first check (which is why I had previously moved the check to before the .id check)
the skilltree system should work like this as it was intended before
first 'pc_calc_skilltree_normalize_job' computes and return the class in which you are able to put skills then pass it to 'pc_calc_skilltree' in which it assign the flag to skills if possible to skill up according the class passed by 'pc_calc_skilltree_normalize_job' and the flag assign is now can be use in 'pc_skillup' to check if ok to skill up...
as example above it should be
1 HT_SKIDTRAP - ok 2 HT_FLASHER - ok 3 HT_FREEZINGTRAP - ok 4 HT_SANDMAN - ok 5 HT_LANDMINE - ok 6 HT_ANKLESNARE - ok 7 HT_BLASTMINE - ok 8 HT_SHOCKWAVE - ok 9 HT_REMOVETRAP - ok 10 HT_CLAYMORETRAP - ok should stop here if 'pc_calc_skilltree_normalize_job' will return JOBL_UPPER or JOBL_2_1 11 RA_RESEARCHTRAP - fail then show message 12 RA_VERDURETRAP - fail then show message
stances I found to produce the bug
1.bypass hierarchical job sequence.
2. more than skill point than usual.
sir can you show me your @stats thanks...As I understand the previous code doesn't work because:
I tried as you proposed (like this: malu-testfix.patch 3.51K 7 downloads ) and it allowed the original bug to happen again (was able to put skills in 3rd class skill tree w/o having spent the points accordingly ), is there something I am missing when testing?
sir Ind this is because you didn't revert yet your previous attempt fix for this(https://github.com/H...28997148bcc5f55)..revert it then try to check if it works..
Sir Ind this is not fix the bug. Now it's ask 1more skill points for class 1 and 2 more skills points for class 2.
since this part of the code is in charge for the checking if possible to add skill pointif( sd->status.skill_point > 0 && sd->status.skill[index].id && sd->status.skill[index].flag == SKILL_FLAG_PERMANENT && //Don't allow raising while you have granted skills. [Skotlex] sd->status.skill[index].lv < skill->tree_get_max(skill_id, sd->status.class_) ) { sd->status.skill[index].lv++; sd->status.skill_point--; if( !skill_db[index].inf ) status_calc_pc(sd,0); // Only recalculate for passive skills. else if( sd->status.skill_point == 0 && (sd->class_&MAPID_UPPERMASK) == MAPID_TAEKWON && sd->status.base_level >= 90 && pc->famerank(sd->status.char_id, MAPID_TAEKWON) ) pc->calc_skilltree(sd); // Required to grant all TK Ranger skills. else pc_check_skilltree(sd, skill_id); // Check if a new skill can Lvlup clif->skillup(sd,skill_id); clif->updatestatus(sd,SP_SKILLPOINT); if( skill_id == GN_REMODELING_CART ) /* cart weight info was updated by status_calc_pc */ clif->updatestatus(sd,SP_CARTINFO); if (!pc->has_permission(sd, PC_PERM_ALL_SKILL)) // may skill everything at any time anyways, and this would cause a huge slowdown clif->skillinfoblock(sd); }cause in your previous commit its like you remove its functionality and also with the skill tree normalization function...
You click to level a 3rd-class-skill on the client, client sends lv-up requests in necessity order e.g. in a ranger if you reset skills and try to level verdure trap straight away: it sends 12 requests in order:
1 HT_SKIDTRAP
2 HT_FLASHER
3 HT_FREEZINGTRAP
4 HT_SANDMAN
5 HT_LANDMINE
6 HT_ANKLESNARE
7 HT_BLASTMINE
8 HT_SHOCKWAVE
9 HT_REMOVETRAP
10 HT_CLAYMORETRAP
11 RA_RESEARCHTRAP
12 RA_VERDURETRAP
which fullfills the skill-tree requirements of the previous and makes sd->status.skill[idx].id be non-0, it then skips the previous code since it passes on the first check (which is why I had previously moved the check to before the .id check)
try thisSir Ind this is not fix the bug. Now it's ask 1more skill points for class 1 and 2 more skills points for class 2.
diff --git a/src/map/pc.c b/src/map/pc.c index 6dea877a0b57e58c78f072bff7447e609504ae5d..032eed8f149e16feb57d87aca9a22ac9d152acb0 100644 --- a/src/map/pc.c +++ b/src/map/pc.c @@ -6219,73 +6219,6 @@ int pc_skillup(struct map_session_data *sd,uint16 skill_id) { if( !(index = skill->get_index(skill_id)) ) return 0; - if( battle_config.skillup_limit ) { - /* [Ind/Hercules] */ - if( (sd->class_&JOBL_2) && (sd->class_&MAPID_UPPERMASK) != MAPID_SUPER_NOVICE ){ - while(1) { - int c, i = 0, k = 0, pts = 0, pts_second = 0, id = 0; - bool can_skip = false; - - c = sd->class_ & MAPID_BASEMASK; - - k = pc_class2idx(pc_mapid2jobid(c, sd->status.sex)); - - for(i = 0; i < MAX_SKILL_TREE && (id=skill_tree[k][i].id) > 0 ; i++){ - int inf2 = skill->get_inf2(id), idx = skill_tree[k][i].idx; - - if( skill_id == id ) { - can_skip = true; - break;/* its oki we can skip */ - } - - if ( inf2&INF2_QUEST_SKILL || (inf2&(INF2_WEDDING_SKILL|INF2_SPIRIT_SKILL)) || id == NV_BASIC ) - continue; - - if( sd->status.skill[idx].id && sd->status.skill[idx].flag == SKILL_FLAG_PERMANENT ) - pts += pc_checkskill(sd, id); - } - - if( can_skip ) break; - - if( pts < sd->change_level_2nd ) { - clif->msg_value(sd, 0x61E, sd->change_level_2nd - pts); - return 0; - } - - if( sd->class_&JOBL_THIRD ) { - bool is_trans = sd->class_&JOBL_UPPER? true : false; - - c = is_trans ? (sd->class_ &~ JOBL_THIRD)/* find fancy way */ : sd->class_ & MAPID_UPPERMASK; - - k = pc_class2idx(pc_mapid2jobid(c, sd->status.sex)); - - for(i = 0; i < MAX_SKILL_TREE && (id=skill_tree[k][i].id) > 0 ; i++){ - int inf2 = skill->get_inf2(id), idx = skill_tree[k][i].idx; - - if( skill_id == id ) { - can_skip = true; - break;/* its oki we can skip */ - } - - if ( inf2&INF2_QUEST_SKILL || (inf2&(INF2_WEDDING_SKILL|INF2_SPIRIT_SKILL)) || id == NV_BASIC ) - continue; - - if( sd->status.skill[idx].id && sd->status.skill[idx].flag == SKILL_FLAG_PERMANENT ) - pts_second += pc_checkskill(sd, id); - } - - if( can_skip ) break; - - if( pts_second - pts < sd->change_level_3rd ) { - clif->msg_value(sd, 0x61F, sd->change_level_3rd - (pts_second - pts)); - return 0; - } - } - break; - } - } - } - if( sd->status.skill_point > 0 && sd->status.skill[index].id && sd->status.skill[index].flag == SKILL_FLAG_PERMANENT && //Don't allow raising while you have granted skills. [Skotlex] @@ -6306,7 +6239,21 @@ int pc_skillup(struct map_session_data *sd,uint16 skill_id) { clif->updatestatus(sd,SP_CARTINFO); if (!pc->has_permission(sd, PC_PERM_ALL_SKILL)) // may skill everything at any time anyways, and this would cause a huge slowdown clif->skillinfoblock(sd); + } else if( battle_config.skillup_limit ){ + int pts = 0, i, id; + for(i = 0; i < MAX_SKILL_TREE && (id=skill_tree[pc_class2idx(sd->status.class_)][i].id) > 0 ; i++) { + int inf2 = skill->get_inf2(id); + if ( inf2&INF2_QUEST_SKILL || (inf2&(INF2_WEDDING_SKILL|INF2_SPIRIT_SKILL)) || id == NV_BASIC ) + continue; + if( sd->status.skill[id].id && sd->status.skill[id].flag == SKILL_FLAG_PERMANENT ) + pts += pc_checkskill(sd, id); + } + if( pts < sd->change_level_2nd ) + clif->msg_value(sd, 0x61E, (sd->change_level_2nd-1)-pts); + else if( pts < (sd->change_level_3rd + sd->change_level_2nd) ) + clif->msg_value(sd, 0x61F, sd->change_level_3rd - (pts - sd->change_level_2nd)); } + return 0; }
diff --git a/src/map/pc.c b/src/map/pc.c index 6dea877a0b57e58c78f072bff7447e609504ae5d..032eed8f149e16feb57d87aca9a22ac9d152acb0 100644 --- a/src/map/pc.c +++ b/src/map/pc.c @@ -6219,73 +6219,6 @@ int pc_skillup(struct map_session_data *sd,uint16 skill_id) { if( !(index = skill->get_index(skill_id)) ) return 0; - if( battle_config.skillup_limit ) { - /* [Ind/Hercules] */ - if( (sd->class_&JOBL_2) && (sd->class_&MAPID_UPPERMASK) != MAPID_SUPER_NOVICE ){ - while(1) { - int c, i = 0, k = 0, pts = 0, pts_second = 0, id = 0; - bool can_skip = false; - - c = sd->class_ & MAPID_BASEMASK; - - k = pc_class2idx(pc_mapid2jobid(c, sd->status.sex)); - - for(i = 0; i < MAX_SKILL_TREE && (id=skill_tree[k][i].id) > 0 ; i++){ - int inf2 = skill->get_inf2(id), idx = skill_tree[k][i].idx; - - if( skill_id == id ) { - can_skip = true; - break;/* its oki we can skip */ - } - - if ( inf2&INF2_QUEST_SKILL || (inf2&(INF2_WEDDING_SKILL|INF2_SPIRIT_SKILL)) || id == NV_BASIC ) - continue; - - if( sd->status.skill[idx].id && sd->status.skill[idx].flag == SKILL_FLAG_PERMANENT ) - pts += pc_checkskill(sd, id); - } - - if( can_skip ) break; - - if( pts < sd->change_level_2nd ) { - clif->msg_value(sd, 0x61E, sd->change_level_2nd - pts); - return 0; - } - - if( sd->class_&JOBL_THIRD ) { - bool is_trans = sd->class_&JOBL_UPPER? true : false; - - c = is_trans ? (sd->class_ &~ JOBL_THIRD)/* find fancy way */ : sd->class_ & MAPID_UPPERMASK; - - k = pc_class2idx(pc_mapid2jobid(c, sd->status.sex)); - - for(i = 0; i < MAX_SKILL_TREE && (id=skill_tree[k][i].id) > 0 ; i++){ - int inf2 = skill->get_inf2(id), idx = skill_tree[k][i].idx; - - if( skill_id == id ) { - can_skip = true; - break;/* its oki we can skip */ - } - - if ( inf2&INF2_QUEST_SKILL || (inf2&(INF2_WEDDING_SKILL|INF2_SPIRIT_SKILL)) || id == NV_BASIC ) - continue; - - if( sd->status.skill[idx].id && sd->status.skill[idx].flag == SKILL_FLAG_PERMANENT ) - pts_second += pc_checkskill(sd, id); - } - - if( can_skip ) break; - - if( pts_second - pts < sd->change_level_3rd ) { - clif->msg_value(sd, 0x61F, sd->change_level_3rd - (pts_second - pts)); - return 0; - } - } - break; - } - } - } - if( sd->status.skill_point > 0 && sd->status.skill[index].id && sd->status.skill[index].flag == SKILL_FLAG_PERMANENT && //Don't allow raising while you have granted skills. [Skotlex] @@ -6306,7 +6239,21 @@ int pc_skillup(struct map_session_data *sd,uint16 skill_id) { clif->updatestatus(sd,SP_CARTINFO); if (!pc->has_permission(sd, PC_PERM_ALL_SKILL)) // may skill everything at any time anyways, and this would cause a huge slowdown clif->skillinfoblock(sd); + } else if( battle_config.skillup_limit ){ + int pts = 0, i, id; + for(i = 0; i < MAX_SKILL_TREE && (id=skill_tree[pc_class2idx(sd->status.class_)][i].id) > 0 ; i++) { + int inf2 = skill->get_inf2(id); + if ( inf2&INF2_QUEST_SKILL || (inf2&(INF2_WEDDING_SKILL|INF2_SPIRIT_SKILL)) || id == NV_BASIC ) + continue; + if( sd->status.skill[id].id && sd->status.skill[id].flag == SKILL_FLAG_PERMANENT ) + pts += pc_checkskill(sd, id); + } + if( pts < sd->change_level_2nd ) + clif->msg_value(sd, 0x61E, (sd->change_level_2nd-1)-pts); + else if( pts < (sd->change_level_3rd + sd->change_level_2nd) ) + clif->msg_value(sd, 0x61F, sd->change_level_3rd - (pts - sd->change_level_2nd)); } + return 0; }is this the solution of this bug malu?