Issue information

Issue ID
#7471
Status
Fixed
Severity
High
Started
ToiletMaster
Jul 4, 2013 14:56
Last Post
Harly Davidson
Oct 15, 2013 18:11
Confirmation
Yes (4)
No (0)

ToiletMaster - Jul 4, 2013 14:56

hi there!

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!

Ind - Jul 4, 2013 19:27

[quote]
1. The message that used to show you require to use 50 for the 1st job is missing.[/quote]was that something a npc said? to my knowledge you require at most 40 o-o 50 is optional (I'm not acknowledging this was changed, I currently have no idea) -- also when/where was this displayed?.
Will try to test #2

This post has been edited by Ind on Jul 4, 2013 19:30

Ind - Jul 4, 2013 19:33

[quote name="ToiletMaster" timestamp="1372949819"]
2. It seems that upon reset, you're able to immediately add your skills to 3rd job page without going through 1st and second.[/quote]How can this be done? I tried reseting and the client doesn't even show the lvl up icon in the 2nd or 3rd class skill pages

kyeme - Jul 4, 2013 19:38

Maybe related:
Using this script:
[code=auto:0]new_zone01,136,121,4 script wwewewe 105,{ jobchange 4018; set baselevel, 90; set joblevel, 60; set skillpoint, 117; end; }[/code]
You can bypass the 1st job skills

This post has been edited by kyeme on Jul 5, 2013 2:07

ToiletMaster - Jul 5, 2013 1:36

I'm not sure where to get that info, but previously before I updated, and when i just resetted my skill points. It just displays in the chat log there. Similar to dispbottom script command.

If i increased 2nd/3rd job skills immediately, it'll state. Please use finish '50' Skill points in 1st tab. (Note that this is wrong as it should be 49) which i would require to use up 49 skills points in order
to reach the second tab.

Imagine my level is 150/50 at the moment. Upon resetting, i should get, 49+69+49 am i correct? which comes back to 167 skill points.

However, upon updating, the message dissapears and I can increase the skill points bypassing the restriction.

Please let me know if you require further information thanks!

Ind - Jul 6, 2013 1:51

What method did you use to reset the skills?

malufett - Jul 6, 2013 4:38

are you using GM account? using skillall or bypassing job per job change can ruin your per class skill restriction..

:meow:

ToiletMaster - Jul 6, 2013 4:43

Ind,

Using a script to reset it. To be more specific it would be Euphy's All-in-one's NPC. with small modifications but none tampering the resetting part.


Malufett,

Nope, I'm aware that GM account is able to bypass however right now my players are able to bypass the restrictions at the moment.
I'm currently using a normal account at the moment and it managed to bypass all restrictions.

Ind - Jul 21, 2013 19:31

Fixed in [url="https://github.com/HerculesWS/Hercules/commit/d680d0ccd5207a0f858b93d2c28997148bcc5f55"]https://github.com/HerculesWS/Hercules/commit/d680d0ccd5207a0f858b93d2c28997148bcc5f55[/url]
Thank you ToilerMaster

ToiletMaster - Jul 22, 2013 1:28

[quote name="Ind" timestamp="1374435071"]
Fixed in [url="https://github.com/HerculesWS/Hercules/commit/d680d0ccd5207a0f858b93d2c28997148bcc5f55"]https://github.com/HerculesWS/Hercules/commit/d680d0ccd5207a0f858b93d2c28997148bcc5f55[/url]
Thank you ToilerMaster[/quote]

Thank you so much Ind!

malufett - Jul 22, 2013 10:58

[quote name="Ind" timestamp="1374435071"]
Fixed in [url="https://github.com/HerculesWS/Hercules/commit/d680d0ccd5207a0f858b93d2c28997148bcc5f55"]https://github.com/HerculesWS/Hercules/commit/d680d0ccd5207a0f858b93d2c28997148bcc5f55[/url]
Thank you ToilerMaster[/quote]
sir Ind you didn't fix anything here you just revised the message info system I originally added and make it cap to 40, 50 & 70..
the main problem resides here 'pc_calc_skilltree_normalize_job' this function is the one that restrict and recompute total skill point if its ok for base, trans and 3rds..so I suggest to revert it..and try to fix 'pc_calc_skilltree_normalize_job' where I failed to fixed since I think and based on my test only gm characters can have this problem until now..:D

this holds the total skillpoint that a character previous allocated to its skill tree..[code=auto:0] sd->change_level_2nd sd->change_level_3rd [/code]
therefore problem may exist if the character is not properly or bypass the hierarchical sequence of having a job...
and yes this problem also exist in aegis when you make play of the job changer script..:P

and officially when you reset your skill the basis of the minimum skill points to be used before the next tab is the previous job level you have that is why I used 'sd->change_level_2nd' & 'sd->change_level_3rd'..
for example:[quote]

swordsman(jlvl50) turns to knight(jlvl 50) and when I reset skill it ask me to full fill my 1st tab with 49 before adding my knight tab skill..
same with
swordsman(jlvl40) turns to knight(jlvl 50) and when I reset skill it ask me to full fill my 1st tab with 39 before adding my knight tab skill..
[/quote]

therefore having a fix of 40, 50 & 70 is wrong..:D
:meow:

Beret - Jul 24, 2013 15:25

Bump.

Ind - Jul 24, 2013 16:04

malufett and I have been discussing the issue, its being worked on

mofo - Jul 26, 2013 8:04

looking forward to the fix. i almost made a duplicate topic, but remembered this.

bump :)

Beret - Jul 26, 2013 22:30

Any solution to this until it is fixed ?

ToiletMaster - Jul 28, 2013 15:06

a temporary solution would be good as well haha

more or less another bump for this >_<

Beret - Jul 29, 2013 15:56

bump.

Beret - Jul 31, 2013 18:32

Something new ?

jTynne - Aug 2, 2013 23:05

Anything yet? o.o

Myth - Aug 3, 2013 10:41

i also have the same problem here [url="http://herc.ws/board/topic/1743-skill-points/"]http://herc.ws/board/topic/1743-skill-points/[/url]

ToiletMaster - Aug 3, 2013 13:58

Still waiting for something to happen haha

Samuel - Aug 3, 2013 14:16

this will happen when we set in player.conf

// 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: [b]yes[/b]

Wildcard - Aug 3, 2013 17:36

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.
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 [url="https://github.com/HerculesWS/Hercules/commit/d680d0ccd5207a0f858b93d2c28997148bcc5f55"]https://github.com/H...28997148bcc5f55[/url] 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 ( [url="http://herc.ws/board/tracker/issue-7154-equipment-based-skills-skill-891-for-example-persisting-through-resetsremoving-gears/"]http://herc.ws/board/tracker/issue-7154-equipment-based-skills-skill-891-for-example-persisting-through-resetsremoving-gears/[/url] ).

Hope I helped :D

This post has been edited by Wildcard on Aug 3, 2013 17:38

malufett - Aug 3, 2013 17:59

oh..thanks you gave me another idea to look into it.. ;)

:meow:

Beret - Aug 6, 2013 14:09

Bump, this is important.

Ind - Aug 7, 2013 12:19

[quote name="Wildcard" timestamp="1375551389"]
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 :D[/quote]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

malufett - Aug 7, 2013 13:05

[quote name="Ind" timestamp="1375877943"][quote name="Wildcard" timestamp="1375551389"]

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 :D[/quote]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[/quote]

sir I don't think so it seems you missed something in here
@status.c[code=auto:0] 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 } [/code]
:meow:

Ind - Aug 7, 2013 15:34

[quote name="malufett" timestamp="1375880712"]

[quote name="Ind" timestamp="1375877943"]

[quote name="Wildcard" timestamp="1375551389"]
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 :D[/quote]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

[/quote]sir I don't think so it seems you missed something in here
@status.c[code=auto:0]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 } [/code] :meow:

[/quote]oooh interesting :D thank you.

Ind - Aug 7, 2013 16:23

I've fixed the SKILL_FLAG_REPLACED_LV_0 that was pointed out (and adjusted it to use the vars until such time that portion is reverted as intended), I'm not sure whether this fixes the actual bug though (so I'll need help to confirm) [url="https://github.com/HerculesWS/Hercules/commit/46a6fdb7374f5fe9301b9d23289f563c6f7fb4f9"]https://github.com/HerculesWS/Hercules/commit/46a6fdb7374f5fe9301b9d23289f563c6f7fb4f9[/url]

Wildcard - Aug 7, 2013 17:27

[quote name="Ind" timestamp="1375877943"][quote name="Wildcard" timestamp="1375551389"]

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 :D[/quote]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[/quote]

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 :D

Michieru - Aug 7, 2013 22:49

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.

malufett - Aug 8, 2013 4:23

[quote name="Michieru" timestamp="1375915797"]
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.[/quote]
sir Ind this is because you didn't revert yet your previous attempt fix for this([url="https://github.com/HerculesWS/Hercules/commit/d680d0ccd5207a0f858b93d2c28997148bcc5f55"]https://github.com/HerculesWS/Hercules/commit/d680d0ccd5207a0f858b93d2c28997148bcc5f55[/url])..revert it then try to check if it works..

since this part of the code is in charge for the checking if possible to add skill point
[code=auto:0] 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); }[/code]
cause in your previous commit its like you remove its functionality and also with the skill tree normalization function...

:meow:

Ind - Aug 8, 2013 19:39

[quote name="malufett" timestamp="1375935809"]

[quote name="Michieru" timestamp="1375915797"]
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.[/quote]sir Ind this is because you didn't revert yet your previous attempt fix for this([url="https://github.com/HerculesWS/Hercules/commit/d680d0ccd5207a0f858b93d2c28997148bcc5f55"]https://github.com/HerculesWS/Hercules/commit/d680d0ccd5207a0f858b93d2c28997148bcc5f55[/url])..revert it then try to check if it works..

since this part of the code is in charge for the checking if possible to add skill point[code=auto:0]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); }[/code]cause in your previous commit its like you remove its functionality and also with the skill tree normalization function...

:meow:

[/quote]I tried as you proposed (like this: [attachment=546:malu-testfix.patch] ) 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?

Ind - Aug 8, 2013 19:54

[quote name="Ind" timestamp="1375990748"]

[quote name="malufett" timestamp="1375935809"]

[quote name="Michieru" timestamp="1375915797"]
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.[/quote]sir Ind this is because you didn't revert yet your previous attempt fix for this([url="https://github.com/HerculesWS/Hercules/commit/d680d0ccd5207a0f858b93d2c28997148bcc5f55"]https://github.com/HerculesWS/Hercules/commit/d680d0ccd5207a0f858b93d2c28997148bcc5f55[/url])..revert it then try to check if it works..

since this part of the code is in charge for the checking if possible to add skill point[code=auto:0]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); }[/code]cause in your previous commit its like you remove its functionality and also with the skill tree normalization function...

:meow:

[/quote]I tried as you proposed (like this: [attachment=546:malu-testfix.patch] ) 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?

[/quote]As I understand the previous code doesn't work because:
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)

malufett - Aug 9, 2013 3:07

[quote]
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)[/quote]
therefore problem relies on 'pc_calc_skilltree_normalize_job'
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
[code=auto:0]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 [/code]

stances I found to produce the bug
1.bypass hierarchical job sequence.
2. more than skill point than usual.

:meow:

This post has been edited by malufett on Aug 9, 2013 5:12

malufett - Aug 9, 2013 4:53

[quote name="Ind" timestamp="1375991648"][quote name="Ind" timestamp="1375990748"]

[quote name="malufett" timestamp="1375935809"]

[quote name="Michieru" timestamp="1375915797"]

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.[/quote]sir Ind this is because you didn't revert yet your previous attempt fix for this([url="https://github.com/HerculesWS/Hercules/commit/d680d0ccd5207a0f858b93d2c28997148bcc5f55"]https://github.com/HerculesWS/Hercules/commit/d680d0ccd5207a0f858b93d2c28997148bcc5f55[/url])..revert it then try to check if it works..

since this part of the code is in charge for the checking if possible to add skill point[code=auto:0] 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); }[/code]cause in your previous commit its like you remove its functionality and also with the skill tree normalization function...

:meow:


[/quote]I tried as you proposed (like this: [attachment=546:malu-testfix.patch] ) 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?


[/quote]As I understand the previous code doesn't work because:
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)[/quote]
sir can you show me your @stats thanks...
[quote]
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.[/quote]
try this

[code=auto: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; } [/code]


:meow:

Michieru - Aug 9, 2013 7:57

Thanks Malu it's looks to works fine =)

Myth - Aug 9, 2013 9:16

[code=: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; } [/code]
is this the solution of this bug malu?

Michieru - Aug 9, 2013 10:54

yes this is the solution

ToiletMaster - Aug 11, 2013 7:35

Bump for this to be committed in since the solution has been found?

Beret - Aug 13, 2013 4:30

Bump.

jTynne - Aug 14, 2013 2:42

Still have an issue, specifically with the Rune Knight skills.

A Job level 27 Rune Knight, who changed at job 50 to Lord Knight, and job 70 to Rune Knight, should have 26 skill points once arriving to the third class skill tree. Implementing the fix above by malufett prevents players from putting points directly into third job after only putting 1-2 points into second job, however, it now states that three more skill points than should be required, must go into the trans skill tree for Rune Knight. Haven't run into this issue again on other third jobs yet, but I've not done extensive testing.

malufett - Aug 14, 2013 4:01

[quote name="jTynne" timestamp="1376448152"]
Still have an issue, specifically with the Rune Knight skills.

A Job level 27 Rune Knight, who changed at job 50 to Lord Knight, and job 70 to Rune Knight, should have 26 skill points once arriving to the third class skill tree. Implementing the fix above by malufett prevents players from putting points directly into third job after only putting 1-2 points into second job, however, it now states that three more skill points than should be required, must go into the trans skill tree for Rune Knight. Haven't run into this issue again on other third jobs yet, but I've not done extensive testing.
[/quote]
SS please for its base, trans and 3rd..or what is its total skill point after reset..cause I can't reproduce your situation..:D
:meow:

mofo - Aug 14, 2013 6:28

Has the fix been committed yet?

jTynne - Aug 14, 2013 10:53

After applying your code @ malufett, here are just a few of the support tickets I've received since:

Hello,after this reset,my Mechanic get an bug with skill tree.My problem until now is with skill Cart Boost.I use it and appear message about thor seed, i think some error with skill tree of Genetic. I use npc reset and reput the skills,and use Cart Boost,but same thing.

hi. with the recent maintenance, my skills were forced reset. i know it was part of the maintenance for the skill allocation bug. but now there\'s a new problem. i put skills on my RK and now my RK got maxed out 3rd job skills. i think you can jump to then next skill tree w/o using all of the previous skill points. im not sure.

I noticed that all players were given a free skill reset, after the 3rd job skill allocation bug was apparently fixed. But it doesn\'t seem to be fixed completely.

My RK has 3 points in Increase HP Recovery even after the reset, and my Warlock has 3 points in Increase SP Recovery. And I can\'t add further points to those skills, it gives the \"Please allocate \'x\' skill points in the 1st tab\" error.

I noticed I was wearing Swordsman and Mage Essence Halloween clips on those characters when they were reset, and thought that\'s what was causing it. So I took all my equips off on both characters, and they still show 3 points invested in those skills. Even after a clean reset, with all my equips off, it shows 3 points invested in the aforementioned skills.

In conclusion, it\'s possible that the Halloween clips may be clashing with skills and causing this bug.

Another thing is, on reset, technically we should get 49+69+49 = 167 points, but we\'re only getting 166.

Thanks for the efforts you\'re putting into resolving this. I hope the information I have provided is of some use.

An addition to this is, many players are experiencing different errors with skills. A ranger had trouble with Attention Concentrate, but it was fixed for him by taking off the equips, then adding skills. It did not work for me. Another Warlock could start adding skills in 3rd job tree directly, without having to go through 1st and 2nd jobs.

After the forced skill reset today, as I was pointing the skill points back on my AB (Enlighten) I noticed that the skill points are a bit more permissice now than should be. I was working off a skill simulator and didn\'t notice I forgot to put points into some 1st job skills; yet the skill window allowed me to put points into 2nd job skills already. I didn\' put 3rd skills yet at that point, so not sure if you could abuse this to devote more points towards third job skills.
For reference, after I figured this out I made sure to put the missed points into 1st jobs as should be and did not abuse it to put more points into 2nd job skills. But the potential for intentional or unintentional abuse is there, so I thought I would notify you about this.

As seen on the character \"Distraught\" on this account, I was able to put 2nd class skill points after a mere 30 into 1st job.

malufett - Aug 14, 2013 12:13

[quote]
My problem until now is with skill Cart Boost.I use it and appear message about thor seed[/quote]
its just a problem in your msgstringtable..

may I know the script on the clips?...

and can you do this..only test this in your test server so that you can execute this easier..
@pc.c[code=auto:0] int pc_calc_skilltree(struct map_session_data *sd) { int i,id=0,flag; int c=0; nullpo_ret(sd); i = pc->calc_skilltree_normalize_job(sd); c = pc->mapid2jobid(i, sd->status.sex); +ShowWarning("Only skill tree of %d is now ok for skill upgrade.\n", c); [/code]
and after compiling now do this step:
1. Create character or pick one character that has a problem
2. Reset skill
3. Monitor the emu screen and check the result if match
eg. I'm 3rd RK and after skill reset you should see[code=auto:0] [Warning]: Only skill tree of 4002 is now ok for skill upgrade. [/code]
and if you accomplished the first job it should show[code=auto:0] [Warning]: Only skill tree of 4008 is now ok for skill upgrade. [/code]

if doesn't match tell me so we can run another set of test...

:meow:

Ind - Aug 14, 2013 12:52

[quote name="malufett" timestamp="1376482380"]

[quote]
My problem until now is with skill Cart Boost.I use it and appear message about thor seed[/quote]its just a problem in your msgstringtable..

may I know the script on the clips?...

and can you do this..only test this in your test server so that you can execute this easier..
@pc.c[code=auto:0]int pc_calc_skilltree(struct map_session_data *sd) { int i,id=0,flag; int c=0; nullpo_ret(sd); i = pc->calc_skilltree_normalize_job(sd); c = pc->mapid2jobid(i, sd->status.sex); +ShowWarning("Only skill tree of %d is now ok for skill upgrade.\n", c); [/code]and after compiling now do this step:
1. Create character or pick one character that has a problem
2. Reset skill
3. Monitor the emu screen and check the result if match
eg. I'm 3rd RK and after skill reset you should see[code=auto:0][Warning]: Only skill tree of 4002 is now ok for skill upgrade. [/code]and if you accomplished the first job it should show[code=auto:0][Warning]: Only skill tree of 4008 is now ok for skill upgrade. [/code]if doesn't match tell me so we can run another set of test...

:meow:

[/quote]I found the problem, working on it, lies on pc_calc_skillpoint (good news the problem can self-fix since this is recalculated everytime, once the algorithm is adjusted), its counting guild skills towards, e.g.[code=auto:0][Debug]: pc_calc_skillpoint: Started [Debug]: pc_calc_skillpoint: 9 for NV_BASIC [Debug]: pc_calc_skillpoint: 1 for HT_SKIDTRAP [Debug]: pc_calc_skillpoint: 1 for HT_LANDMINE [Debug]: pc_calc_skillpoint: 1 for HT_ANKLESNARE [Debug]: pc_calc_skillpoint: 1 for HT_SHOCKWAVE [Debug]: pc_calc_skillpoint: 1 for HT_SANDMAN [Debug]: pc_calc_skillpoint: 1 for HT_FLASHER [Debug]: pc_calc_skillpoint: 1 for HT_FREEZINGTRAP [Debug]: pc_calc_skillpoint: 1 for HT_BLASTMINE [Debug]: pc_calc_skillpoint: 1 for HT_CLAYMORETRAP [Debug]: pc_calc_skillpoint: 1 for HT_REMOVETRAP [Debug]: pc_calc_skillpoint: 1 for GD_APPROVAL [Debug]: pc_calc_skillpoint: 1 for GD_KAFRACONTRACT [Debug]: pc_calc_skillpoint: 1 for GD_GUARDRESEARCH [Debug]: pc_calc_skillpoint: 3 for GD_GUARDUP [Debug]: pc_calc_skillpoint: 10 for GD_EXTENSION [Debug]: pc_calc_skillpoint: 5 for GD_LEADERSHIP [Debug]: pc_calc_skillpoint: 5 for GD_GLORYWOUNDS [Debug]: pc_calc_skillpoint: 5 for GD_SOULCOLD [Debug]: pc_calc_skillpoint: 5 for GD_HAWKEYES [Debug]: pc_calc_skillpoint: 1 for GD_BATTLEORDER [Debug]: pc_calc_skillpoint: 3 for GD_REGENERATION [Debug]: pc_calc_skillpoint: 1 for GD_RESTORE [Debug]: pc_calc_skillpoint: 1 for GD_EMERGENCYCALL [Debug]: pc_calc_skillpoint: 1 for GD_DEVELOPMENT [Debug]: pc_calc_skillpoint: 1 for RA_RESEARCHTRAP [Debug]: pc_calc_skillpoint: 1 for RA_VERDURETRAP [Debug]: pc_calc_skillpoint: returning 64 [/code]

Ind - Aug 14, 2013 14:02

Fixed in [url="https://github.com/HerculesWS/Hercules/commit/0b05acb2fb1f64ec500b9b2ebc9cc9af08f36724"]https://github.com/HerculesWS/Hercules/commit/0b05acb2fb1f64ec500b9b2ebc9cc9af08f36724[/url]
Thank you all for all your help and information. If there are any new issures regarding this please create a new bug report, this one is 3 pages long and a mess.
Thank you again

Harly Davidson - Oct 8, 2013 4:16

Hello This issue resolve?

ToiletMaster - Oct 8, 2013 6:33

[quote name="Harly Davidson" timestamp="1381205787"]
Hello This issue resolve?[/quote]

It's been fixed quite sometime already.

Harly Davidson - Oct 15, 2013 18:11

this issue fix?