Train context menu the oldest bug

[Xeevis]

This bug existed in RunUO as far as I remember not sure why noone repaired it yet even when its so obvious. Basically when player clicks on city vendor he can get trained in some skills. Normally when he's above possible training value this context entry is grayed out.

Problem is, even when he pays for full training there's still fat chance that this entry will remain white till player trains that skill for at least +0.1 ... this is caused by wrong double value in BaseCreature.cs. When dividing NPC skillbase by 3.0 it will yield result with many decimal places like 27.13526272 which is then compared to player skill 27.1 ... In game terms these two skill values are equal so no training can occur, but in math terms the first one is actually higher. So context still thinks that NPC can teach player in this particular skill, but he can't. I suggest that everyone applies this fix as it does get annoying to players (especially on no-skillcap shards).

Just change

double toTeach = skill.Base / 3.0;

into

double toTeach = Math.Truncate(skill.Base / 3.0 * 10) / 10;

And from this point if context entry is white player can train, if its gray he can't, period.

 

[mumuboy]

Another way of fixing this is to use BaseFixedPoint, which will eliminate the need for a double calculation altogether.

 

[Xeevis]

Thanks for your input, yeah you are right. Its shame that RunUO is using doubles for skills as they are only complicating things here.

 

[mumuboy]

I agree. Its not difficult to convert the whole system over to fixedpoint. Depending on a person's skill level, it may take a while though.

 

[Jeff]

All math operations in C# return as doubles... so why not use doubles (avoids truncating, casting, boxing, un-boxing). Not to mention double calculations are just as fast as float calculations on x86 architecture... but faster on x64 architecture... I think those 3 points alone justify using double.

 

[mumuboy]

I think Xeevis' point was that the SkillUpdate packet is sent as BaseFixedPoint (integer), and it might be less complicated (faster?) if all of RunUO used integers instead of doubles. I think though that skillgain and difficulty would still require double calculations, so I don't think its complicating things more.

EDIT: Also the underlying variable for the skills is a ushort already.

 

[Jeff]

I think Xeevis' point was that the SkillUpdate packet is sent as BaseFixedPoint (integer), and it might be less complicated (faster?) if all of RunUO used integers instead of doubles. I think though that skillgain and difficulty would still require double calculations, so I don't think its complicating things more.

EDIT: Also the underlying variable for the skills is a ushort already.

My point still stands... all Math operations in C# return as doubles... you would have to much unnecessary implicit and explicit boxing if you didn't use doubles...

 

[Eos]

Fixed in SVN 831, in case anyone is wondering. (Using BaseFixedPoint.)

 

[mumuboy]

Thanks!