Bug: Me.Skill[name]

[SwiftyMUSE]

When echoing the value of a skill (by name) it does not correctly get the skill value when the name starts with a numeric character. The code looks at the first character of the string and assumes it is by number if it starts with a numeric character. Skills (and possibly other items) by name don't work for values such as '1H Blunt', '1H Slashing', '2H Blunt', etc.

[Drumstix42]

/skills ["skillname"] (Skills are listed in Appendix C)
Lists your skills, or your skill level when /skills "skillname" is used

/skills
Lists all of your skills with skill level

/skills pottery
Returns your pottery skill

Try that out.

[SwiftyMUSE]

/skills ["skillname"] (Skills are listed in Appendix C)
Lists your skills, or your skill level when /skills "skillname" is used

/skills
Lists all of your skills with skill level

/skills pottery
Returns your pottery skill

Try that out.

Thats all good, but 1) MQ2DataTypes.cpp is still wrong and, 2) You can't use that to verify skills in a macro.

[Caladine]

This isn't broken. I don't know exactly what you're typing, but you're typing something wrong as the TLO works just fine for me.

Code: Select all

/echo ${Me.Skill[1H Blunt]}

Returns the proper value for my characters 1h blunt skill.

[s16z]

Try some more of them. I find 1h blunt works fine, 1h slashing returns 1h blunt skill, 2h blunt returns 1h slashing skill.

[Caladine]

Yeah, 2h blunt is returning 255. Guess I'll take a look.

[Drumstix42]

1H Blunt works for me. 2H Blunt doesn't not (returns 255 like said)

[Caladine]

I see where the problem is, gimme a sec to figure out how to fix it.

EDIT: Fixed
MQ2DataTypes.cpp
Lines 1650-1676

Code: Select all

	case Skill:
		if (Index[0])
		{
			[color=red]if ((Index[0]>='0' && Index[0]<='9') && ((Index[1]>='0' && Index[1]<='9') || Index[1]==0))[/color]
			{
				// numeric
				unsigned long nSkill=atoi(Index)-1;
				if (nSkill<0x64)
				{
					Dest.DWord=pChar->Skill[nSkill];
					Dest.Type=pIntType;
					return true;
				}
			}
			else
			{
				// name
				for (DWORD nSkill=0;szSkills[nSkill];nSkill++)
					if (!stricmp(Index,szSkills[nSkill]))
					{
						Dest.DWord=pChar->Skill[nSkill];
						Dest.Type=pIntType;
						return true;
					}
			}
		}
		return false;

Changes highlighted in red. It was changed from

Code: Select all

if (Index[0]>='0' && Index[0]<='9')

The reason why this fails is pretty easy. Lax was looking for a skill value by a number, and forgot that some skill strings will start with a number. Solution: Check to see if the next character is another number or a null before looking up the skill by skill number.

[SwiftyMUSE]

Not an elegant solution, too say the least. We need to identify a way to check if the entire string sent is a number. We also need to look at all other parts of the code that use similar logic as they too may fail. I will investigate this further.

[MacroFiend]

This may be out in left field but what if Index was converted to a Long and compared to itself. If they aren't equal, then it is a string. If they are equal, then it is a num. Basically it is doing a NaN check on itself.

Just an idea. Not sure if that would be better or worse than the way it is corrected here.

[Caladine]

You'll have to bear with me, as most of my programming experience is with embedded devices and microcontrollers, so I generally look for a solution requiring the least number of cycles to execute over most others. Going with that, what I posted is the least amount of extra work to get it to return a proper value for a skill that actually exists. It'll still return bogus values if you typo a numeric.

MacroFiend's idea is probably the best in terms of getting the code to execute as intented with the least amount of added code. What we really want to be doing is iterating over the entire string checking each character.

Code: Select all

bool result=FALSE;
for(int j=0;index[j]!=0;j++){
  if(!(result=(index[j]>='0' && index[j]<='9')))
    break;
}

Will do that, and leave whether or not the string was numeric in result. I could swear there was a library somewhere with that as a function already, but I can't seem to find it. Anyways, that's less work than converting to a long, then back to an string and comparing index with its twice converted cousin.

Checking for a numeric string seems to be what to do, as the string parser section of the code here will handle bad strings.

[Lax]

This is why I made the "IsNumber" function, you guys tried too hard

I just havent gone through everything and changed them all to use it.

[Caladine]

Heh, I figured a function like that had to be defined somewhere, just too useful not to be.