Tradeskill Bug--Quick Fix

[nuttheawsome]

I am kind of new to this engine so if what im about to explain has already been covered then my apologies!

A friend and I got together and started a server a couple of weeks ago. While testing everything out we noticed a bug in the tradeskills. If you try to combine a recipe that has the exact same ingredients, but a different container type you get a message: "These items cannot be combined in this container type."

Just in case anyone reading doesn't understand the problem, here is an example:

There are two recipes: Boots of Justice and Boots of the Northern Wolf.
Both of these recipes have the exact same ingredients, the only difference is they use two different containers. If you walk through the TradeSkills.cpp you'll notice what is going on.

The code does two checks, first a basic check to find the recipe and if the database returns multiple results it calculates a unique number based on the ingredient id's. This works fine usually, but in the case above, you still get multiple recipes returned because they use the EXACT same ingredients, so instead of validating again, it just returns false thinking you have a duplicate recipe in the table.

Ive fixed this by adding in a third check. Now if it returns multiple recipes a second time, we validate the container type and if the player is using the correct container type it will validate properly.(Or return false if the recipe is truely not unique or the correct container isn't being used)

Here is the code:(It starts around line 900 in the tradeskills.cpp)

	//updated this section to fix the tradeskill bug of multiple recipies, but different containers.
	/*
	if (qcount != 1) {
		if(qcount > 1) {
			LogFile->write(EQEMuLog::Error, "Combine error: Recipe is not unique!");
		
		
		}
		//else, just not found i guess..
		return(false);
	}*/
	
	if(qcount < 1)
		return(false);
	
	
	if(qcount > 1)
	{
		//The recipe is not unique, so we need to compare the container were using.
		uint32 containerId = container->GetID();

		qlen = MakeAnyLenString(&query,"SELECT tre.recipe_id FROM tradeskill_recipe_entries as tre WHERE tre.recipe_id IN (%s)"
		" GROUP BY tre.item_id HAVING tre.item_id = %u;",buf2,containerId);
		
		if (!RunQuery(query, qlen, errbuf, &result)) {
			LogFile->write(EQEMuLog::Error, "Error in GetTradeRecipe, re-query: %s", query);
			safe_delete_array(query);
			LogFile->write(EQEMuLog::Error, "Error in GetTradeRecipe, error: %s", errbuf);
			return(false);
		}
		safe_delete_array(query);

		
		uint32 resultRowTotal = mysql_num_rows(result); 
		
		//If all the recipes were correct, but no container type matched, drop out.
		if(resultRowTotal == 0 || resultRowTotal > 1){
			LogFile->write(EQEMuLog::Error, "Combine error: Recipe is not unique OR incorrect container is being used!");
			return(false);
		}
	
	}

I am not the best at writing MYSQL queries. So this may be able to be fixed with just a more robust query on the second check, but this fix does the trick(as far as I can tell)

Hope it helps someone here...

-Nut

[cavedude]

Awesome, that bug has annoyed me for some time! Thanks for the fix, I'll try it out on PEQ the next time it reboots.

[nuttheawsome]

No problem. Just post back here if you have any problems adding it and ill just post up my entire tradeskills.cpp file.

[cavedude]

I have found one problem with I think is this piece of code. It looks like it has created a new zone crash. I've never seen it before, and since the only thing that has changed in tradeskills is this, I'm opt to think this is the culprit. It's not a common crash by any means (it has happened a total of 3 times on PEQ, which in the grand scheme of things, considering the amount of players and how often they combine isn't a lot) but it's a crash nonetheless. Here's the debug output:

#0  0x081a6946 in ZoneDatabase::GetTradeRecipe(ItemInst const*, unsigned char, unsigned, DBTradeskillRecipe_Struct*) (this=0x82c5f20, container=0x83e7c28,
    c_type=16 '\020', some_id=0, spec=0x0) at Item.h:295
#1  0x081a457d in Object::HandleCombine(Client*, NewCombine_Struct const*, Object*) (user=0x8864fb0, in_combine=0x87fc9f8, worldo=0x83e7b20)
    at tradeskills.cpp:140
#2  0x08189351 in Client::Handle_OP_TradeSkillCombine(EQApplicationPacket const*) (this=0x1, app=0x0) at client_packet.cpp:4225
#3  0x0817d410 in Client::HandlePacket(EQApplicationPacket const*) (
    this=0x8864fb0, app=0x8818b40) at client_packet.cpp:392
#4  0x080d59f5 in Client::Process() (this=0x8864fb0) at client_process.cpp:581
#5  0x080bb3b3 in EntityList::MobProcess() (this=0x829c220) at entity.cpp:442
#6  0x080e0239 in main (argc=0, argv=0xbfffe514) at net.cpp:479

Just to eliminate the possibility I didn't merge it correctly, could you post a diff? However, I did try out both of the recipes you mentioned, and both combined correctly, and I have heard of no other issues with recipes. So, from a functionality standpoint it certainly is working. I also won't rule out a db error, but I have no idea of how to trace them back.

[nuttheawsome]

Sorry for the delay. Ive been moving for a few days, so my internet hasn't been up until today.

Anyway. I find it odd that its crashing the zone server..hmm. I haven't notice any crashes on the build over here. We run a much smaller player base though.

Here is the diff:

--- tradeskills.cpp   7 Jun 2007 07:03:46 -0000   1.18
+++ tradeskills.cpp   14 May 2008 23:52:27 -0000
@@ -885,7 +885,7 @@
        " WHERE tre.recipe_id IN (%s)"
        " GROUP BY tre.recipe_id HAVING sum(tre.componentcount) = %u"
        "  AND sum(tre.item_id * tre.componentcount) = %u", buf2, count, sum);
-      
+
       if (!RunQuery(query, qlen, errbuf, &result)) {
          LogFile->write(EQEMuLog::Error, "Error in GetTradeRecipe, re-query: %s", query);
          safe_delete_array(query);
@@ -896,14 +896,52 @@
    
       qcount = mysql_num_rows(result);
    }
+   
+   //updated this section to fix the tradeskill bug of multiple recipies, but different containers.
+   /*
    if (qcount != 1) {
       if(qcount > 1) {
          LogFile->write(EQEMuLog::Error, "Combine error: Recipe is not unique!");
+      
+      
       }
       //else, just not found i guess..
       return(false);
+   }*/
+   
+   if(qcount < 1)
+      return(false);
+   
+   
+   if(qcount > 1)
+   {
+      //The recipe is not unique, so we need to compare the container were using.
+      uint32 containerId = container->GetID();
+
+      qlen = MakeAnyLenString(&query,"SELECT tre.recipe_id FROM tradeskill_recipe_entries as tre WHERE tre.recipe_id IN (%s)"
+      " GROUP BY tre.item_id HAVING tre.item_id = %u;",buf2,containerId);
+      
+      if (!RunQuery(query, qlen, errbuf, &result)) {
+         LogFile->write(EQEMuLog::Error, "Error in GetTradeRecipe, re-query: %s", query);
+         safe_delete_array(query);
+         LogFile->write(EQEMuLog::Error, "Error in GetTradeRecipe, error: %s", errbuf);
+         return(false);
+      }
+      safe_delete_array(query);
+
+      
+      uint32 resultRowTotal = mysql_num_rows(result); 
+      
+      //If all the recipes were correct, but no container type matched, drop out.
+      if(resultRowTotal == 0 || resultRowTotal > 1){
+         LogFile->write(EQEMuLog::Error, "Combine error: Recipe is not unique OR incorrect container is being used!");
+         return(false);
+      }
+   
    }
    
+
+
    row = mysql_fetch_row(result);
    uint32 recipe_id = (uint32)atoi(row[0]);
    mysql_free_result(result);

Success, CVS operation completed

If this isn't what you needed, just let me know(Im a little new to using CVS and such)

Ill take some time and retrace the code and see if I can find any problems or paths that aren't being handled and see if I can get to the bottom of this.

[Scorpious2k]

Before I move this into the live code, is there any problem with it? Cavedude? Was this the cause oif the zone crash and if so do we have the fix?

[Angelox]

Originally Posted by Scorpious2k

Before I move this into the live code, is there any problem with it? Cavedude? Was this the cause oif the zone crash and if so do we have the fix?

All I see at the download section is EQEmu-0.7.0-1111 on top (nothing newer) - am I missing something, or it just hasn't updated yet?

[Scorpious2k]

Originally Posted by Angelox

All I see at the download section is EQEmu-0.7.0-1111 on top (nothing newer) - am I missing something, or it just hasn't updated yet?

I'm not sure how that works. I mentioned before that it seems slow and doesn't seem to update everyday. I am currently at 1115 (just updated) ... I don't know why the list isn't at the very LEAST at the point where I was last night: 1113

EDIT: here is a list of what I have in that isn't in that list yet (from the changelog):

==06/19/2008
Scorpious2k (Knightly): Correction of divine intervention text
Scorpious2k (LordKahel): Support for defensive Instinct and Reflexive Mastery AA

==06/18/2008
Scorpious2k (Derision): Fix for flee runspeed - linear flee speed reduction as HP drops
Scorpious2k (Derision): Rule to prevent mobs from fleeing if they are being helped by other NPCs
Scorpious2k (haecz): Distance check for corpse dragging
Scorpious2k (haecz): Distance check for taunt
Scorpious2k (greggg230): Merchant price faction/charisma fix
Scorpious2k (greggg230): Faction will now show on /con for agnostic players
Scorpious2k (BatCountry): Correction of a zone crash caused by reloading rules
Scorpious2k (Congdar): Eliminated array index error/zone crash in spells

==06/17/2008
Scorpious2k (TheLieka): Ban by IP
Scorpious2k (cavedude/TheLieka): Ability to limit melee guys from being bound in certain zones. This changes the canbind
column of the zone table. Value 0 means noone can bind, value 1 means only casters can bind, value 2 means
anyone can bind in the zone (ie cities).

Required SQL:
CREATE TABLE `Banned_IPs` (
`ip_address` VARCHAR(32) NOT NULL,
PRIMARY KEY (`ip_address`)
)
ENGINE = InnoDB;

Optional SQL:
Insert into rule_values values (0, 'World:UseBannedIPsTable', 0);
Update zone set canbind = 2 where zoneidnumber in (1,2,3,8,9,10,19,23,24,29,40,41,42,45,49,52,54,55, 60,61,62,67,75,82,83,106,155);