Bug in /CharInfo

NealThorpayt · Post by **NealThorpayt** » Mon Mar 17, 2003 1:38 pm

Greetings Constructs,

Just thought I would mention that there is a bug in /charinfo.

I believe this is caused by the change in the bank structure. As anyone who purchased the LoY expansion, or read the release announcements is aware, they changed the number of bank slots from 8 to 16. The code structure for CHARINFO reads:

Code: Select all

typedef struct _CHARINFO
...
PITEMINFO Bank[8];
...

I am not certain if this is the exact cause of the problem. But, as you can see, it will need to be changed to accomodate the extra 8 slots in the bank.

To fix this problem with /charinfo termporarily, comment out the following in EQLib.cpp -- CharInfo

Code: Select all

for (index=0;index<8;index++) if (pCharInfo->Bank[index]) {
	BOOL Found = FALSE;
	PITEMDB ItemDB = gItemDB;
	while (ItemDB) {
		if (ItemDB->ID == pCharInfo->Bank[index]->ItemNumber) {
			Found = TRUE;
		}
		ItemDB = ItemDB->pNext;
	}
	if (!Found) {
		PITEMDB Item = (PITEMDB)malloc(sizeof(ITEMDB));
		Item->pNext = gItemDB;
		Item->ID = pCharInfo->Bank[index]->ItemNumber;
		strcpy(Item->szName,pCharInfo->Bank[index]->Name);
		DebugSpew("New Item found - %d: %s",Item->ID,Item->szName);
		gItemDB = Item;
	}
	if (pCharInfo->Bank[index]->Type == ITEMTYPE_PACK) {
		LONG bindex;
		for (bindex=0;bindex<pCharInfo->Bank[index]->Container.Slots;bindex++) if (pCharInfo->Bank[index]->Container.Contents[bindex]) {
			PITEMDB ItemDB = gItemDB;
			Found=FALSE;
			while (ItemDB) {
				if (ItemDB->ID == pCharInfo->Bank[index]->Container.Contents[bindex]->ItemNumber) {
					Found = TRUE;
				}
				ItemDB = ItemDB->pNext;
			}
			if (!Found) {
				PITEMDB Item = (PITEMDB)malloc(sizeof(ITEMDB));
				Item->pNext = gItemDB;
				Item->ID = pCharInfo->Bank[index]->Container.Contents[bindex]->ItemNumber;
				strcpy(Item->szName,pCharInfo->Bank[index]->Container.Contents[bindex]->Name);
				DebugSpew("New Item found - %d: %s",Item->ID,Item->szName);
				gItemDB = Item;
			}
		}
	}
}

I will continue to look into this problem.

Question: Who decoded the original CHARINFO structure?
Comment: Perhaps they would be able to decode the new structure faster than I.

End of line...

Mckorr · Post by **Mckorr** » Tue Mar 18, 2003 9:42 am

Would changing

Code: Select all

PITEMINFO Bank[8];

to

Code: Select all

PITEMINFO Bank[16];

fix the problem? Or do we need a conditional, based on the availability (or lack thereof) of the LOY expansion?

I mean, looks to me like this is a simple array of Bank[index] and a FOR..NEXT loop, and changing all the instances of 8 to 16 in this block would handle the problem.

You'd also need to change

Code: Select all

for (index=0;index<8;index++) if (pCharInfo->Bank[index]) {

to

Code: Select all

for (index=0;index<16;index++) if (pCharInfo->Bank[index]) {

|| Napolion || · Post by **|| Napolion ||** » Wed Mar 19, 2003 11:42 am

Dont think you can change the

Code: Select all

for (index=0;index<8;index++) if (pCharInfo->Bank[index]) {

unless you can detect if the user have LoY install. Some only still have 8 slots to use in bank.

Mckorr · Post by **Mckorr** » Wed Mar 19, 2003 2:18 pm

Hmmm, not sure how we would do that, considering that information is stored server side.

Well, since it's "bring your own compiler" those with LOY should be able to go ahead and make that change before they compile, and those without won't need to worry about it.

What I wanted to know, though, was whether making that change would screw things up in /charinfo or not.

Revenge · Post by **Revenge** » Wed Mar 19, 2003 4:06 pm

I'm having a hard time accepting the concept of SoE using two different and incompatable bank variables.

I think we'll find the truth of the matter is that every client has 16 bank slots, and that the extra 8 are just simply not visible in the user interface of people who dont have LoY.

Anyone know enough about the structure to prove me wrong?

NealThorpayt · Post by **NealThorpayt** » Thu Mar 20, 2003 1:01 am

Greetings Constructs,

I completed the investigation into the cause and solution to the /charinfo bug.

First, the new _CHARINFO struct code in MQ.h:

Code: Select all

#define		NUM_BANK_SLOTS			16
typedef struct _CHARINFO {
	BYTE		Unknown0000;
	BYTE		Unknown0001;
	CHAR		Name[64];
	CHAR		Lastname[70];
	DWORD		Unknown0136;
	DWORD		Race;
	DWORD		Class;
	DWORD		Gender;
	DWORD		Level;
	DWORD		Exp;
	DWORD		Face;
	DWORD		Mana;
	DWORD		BaseHP;
	DWORD		Stunned;
	DWORD		BaseSTR;
	DWORD		BaseSTA;
	DWORD		BaseCHA;
	DWORD		BaseDEX;
	DWORD		BaseINT;
	DWORD		BaseAGI;
	DWORD		BaseWIS;
	BYTE Unknown0204[172];
	BYTE		Language[32];
	BYTE Unknown0408[496];
	SPELLBUFF	Buff[21];
	BYTE Unknown1240[1704];
	DWORD		SpellBook[256];
	BYTE Unknown3968[1024];
	DWORD		MemorizedSpells[8];
	BYTE Unknown5024[56];
	DWORD		Plat;
	DWORD		Gold;
	DWORD		Silver;
	DWORD		Copper;
	DWORD		BankPlat;
	DWORD		BankGold;
	DWORD		BankSilver;
	DWORD		BankCopper;
	BYTE Unknown5964[32];
	DWORD		Skill[125];
	BYTE Unknown5644[112];
	DWORD		AutoSplit;
	BYTE		Unknown5760[96];
	struct _SPAWNINFO*	pSpawn;
	PITEMINFO	Inventory[30];
	PITEMINFO	Cursor;
	BYTE Unknown5984[60];
	SPELLBUFF ShortBuff[6];
	BYTE Unknown6140[264];
	DWORD ZoneBoundId;
	DWORD ZoneBirthId;
	DWORD ZoneOtherId[3];
	FLOAT ZoneBoundX;
	FLOAT ZoneBirthX;
	FLOAT ZoneOtherX[3];
	FLOAT ZoneBoundY;
	FLOAT ZoneBirthY;
	FLOAT ZoneOtherY[3];
	FLOAT ZoneBoundZ;
	FLOAT ZoneBirthZ;
	FLOAT ZoneOtherZ[3];
	BYTE Unknown6484[3548];
	DWORD GuildID;
	BYTE Unknown10036[15];
	WORD		Anon;
	BYTE GuildStatus;
	BYTE Unknown10054[452];
	DWORD AAExp;
	BYTE Unknown10510[428];
	DWORD AAPoints;
	BYTE Unknown10942[2542];
	PITEMINFO	Bank[NUM_BANK_SLOTS];
} CHARINFO, *PCHARINFO;

If you look at the code, I have updated the unknown byte offsets. Also, any item that I was able to verify (i.e. able to confirm with data in my characters) is tabbed over.

Second, the change to the CharInfo code in EQLib.cpp:

The old code was

Code: Select all

for (index=0;index<8;index++) if (pCharInfo->Bank[index]) {

The new code is

Code: Select all

for (index=0;index <NUM_BANK_SLOTS ;index++) if (pCharInfo->Bank[index]) {

This fixes the crash and memory problems with the /CharInfo command.

End of line...

Mckorr · Post by **Mckorr** » Thu Mar 20, 2003 3:19 pm

Problem.

Made your changes, executed a /charinfo command, got booted all the way out of EQ. Tried it the first time in full screen mode, and had to reset the computer. Second time in windowed mode, and EQ crashed and exited all the way back to the desktop.

Perhaps there needs to be a modification to the /charinfo code structure itself to accomodate these changes?

Prior to these changes /charinfo worked for me, not that I use it all that much.

And yes, I do have LOY.

NealThorpayt · Post by **NealThorpayt** » Thu Mar 20, 2003 5:25 pm

Greetings Constructs,

My apologies to anyone who tried my changes and had EQ crash. All I can tell you is that prior to my changes, EQ was dereferencing invalid bank pointers in the bank pointer array in the _CHARINFO struct. After the changes, it no longer was referencing bad pointers. It is curious that at least one person was able to run the /CharInfo command prior to the changes.

Please note that there was a change to the EQLib.cpp file in the CharInfo function that changed the bank slot index from a constant '8' to a defined NUM_BANK_SLOTS which I had set to 16. This should make very little difference to the resulting action of the code if left at 8.

Prior to my posting the changes, I tested it against 5 different characters. Each one having a different set and combination/position of items in the bank. All worked fine. I even checked if the position of the array of bank pointers could have been referenced by a pointer itself (much like the _CHARINFO struct is). However, I was unable to find any pointer in memory as such.

I suppose the only prudent course of action at this time is to roll back the changes until such a time as others can evaluate them.

Once again, my apologies to anyone that tried and failed.

End of line...

dont_know_at_all · Post by **dont_know_at_all** » Thu Mar 20, 2003 5:34 pm

Uh, these changes look valid to me. I have added them to the CVS depot.

Mckorr · Post by **Mckorr** » Thu Mar 20, 2003 5:51 pm

My debugspew.log shows MQ acquiring birth and bind points correctly, then an item list... then it stops, at what I'm assuming is the point where EQ crashes. From the items found it's definately finding items in the bank, but crashes immediately afterwards. I believe it is even finding things in the extra 8 slots.

Question: on characters you tested this on, did they have anything in the expanded bank slots?

Also, perhaps there is an error causing MQ to try to read a 17th slot? A simple math error in the code someplace?

Don't have time to look now, taking the wife out for sushi... but if you get the time check that 17th slot idea....

insanitywiz · Post by **insanitywiz** » Thu Mar 20, 2003 6:16 pm

Mmm, sushi. Benihana dollar sushi for me tonight!

OT yes, but important, I mean, cmon, it's sushi!

dont_know_at_all · Post by **dont_know_at_all** » Thu Mar 20, 2003 10:10 pm

I don't have anything in the bank slots, I don't have the expansion.

Cheese · Post by **Cheese** » Fri Mar 21, 2003 5:58 am

Neal beat me to posting it omg !

Mckorr · Post by **Mckorr** » Fri Mar 21, 2003 11:20 am

Ok, rolled back the changes in MQ.h, with the following exceptions:

1) Left in #define NUM_BANK_SLOTS 16
2) Kept PITEMINFO Bank[NUM_BANK_SLOTS];
3) Kept the change to EQLib.CPP

Seems to be working fine with just these changes. Did a /charinfo, got the appropriate replies in EQ, no crash.

So, seems like the crash is in the ASM code changes made by NealThorpayt, but not in the bank slot changes.

Appropriate section of DebugSpew.log is as follows:

HookDInput - Patching
HookCommands - Patching
TakeControlOfCommandList - Adding our commands
CChatHook::Detour(MacroQuest is active.)
MacroQuest is active.
DetermineWhoFirst: '/charinfo' will try MQ's commands first
CChatHook::Detour(You are bound in Butcherblock Mountains at 2610.00, -251.00, 84.00)
CChatHook::Detour(You were born in Butcherblock Mountains at 2610.00, -251.00, 84.00)
New Item found - 18766: Recruitment Letter
New Item found - 1378827265:
New Item found - 0:
DetermineWhoFirst: '/sit' will try EQ's commands first
DetermineWhoFirst: '/camp' will try EQ's commands first
CChatHook::Detour(It will take you about 30 seconds to prepare your camp.)
CChatHook::Detour(It will take about 25 more seconds to prepare your camp.)
CChatHook::Detour(It will take about 20 more seconds to prepare your camp.)
CChatHook::Detour(It will take about 15 more seconds to prepare your camp.)
CChatHook::Detour(It will take about 10 more seconds to prepare your camp.)
CChatHook::Detour(It will take about 5 more seconds to prepare your camp.)
CChatHook::Detour(You abandon your preparations to camp.)

Not sure what those empty/erroneous items found are (brand new character on new server just to check /charinfo). I did notice that the item list doesn't have any of the other newbie items (food/drink, sword). From previous crashes this would indicate that it has something to do with the way this section of code is reading/determining Items.

NealThorpayt · Post by **NealThorpayt** » Fri Mar 21, 2003 5:00 pm

Greetings Constructs,

I have checked an rechecked my changes. I have tried many higher level characters of various races, classes and zones (not that any of the should matter). And, I have tested many brand new characters. I have tested combinations of items in bank slots that range from empty to fulll and various type of containers as well. All test cases work fine.

Mckorr, I was wondering, did you make the changes to your new source code? Or, did you first make the changes to the released source code? I am not using your modified code. I am using the standard release.

If you look at your debug file, the output shows two items that are invalid. I suspect you are not crashing because you haven't hit the right memory condition. But, you are not retrieving the right item data either.

Question: What do you mean by 'ASM' code? Do you mean assembly code?
Note: I only made changes to the basic struct of _CHARINFO and the loop count. No, 'ASM' code was modified.

End of line...

Bug in /CharInfo

Bug in /CharInfo

bank size

Solution to /CharInfo bug...