COMMITTED: Implemented Augmention Sealer from Inventory

[trevius]

Finally had a chance to review this code a bit. I think it is a good idea, but not sure I follow the code enough to commit it myself.

I am not really sure why you check 10 slots in this function (and a few other spots):

bool ItemInst::WillAugmentCauseLoreCollision(ItemInst* newAugment)

Max aug slots on an item is currently 5 slots and if anything, should be coded to use the defined value for max aug slots, "MAX_AUGMENT_SLOTS". The only thing I can think of that would run through 10 slots would be bags. I don't really know the goal of this function. I would think you could just use CheckLoreConflict(), but maybe I am missing something here. Aug Lore checks should be done against the entire inventory, not just per item. I haven't looked too in-depth into it, though.

Also, I don't think the DB query you have is required. You should be able to pull that info directly from shared memory, I think.

There were a few other things I am not quite clear on yet, but I don't really want to go into each one.

As it is currently, I think this could probably be done in a different and maybe more user-friendly way. With the limitations of how this is coded (such as you can't select which aug slot to remove), I think it might be a bit too much of a hack to make it into the SVN. Even though it would be another hack, I think this could be done by tricking the client into thinking it is interacting with a real aug pool. It would probably take more work and innovation to make it work as I am thinking, but I think it may be possible to trick the client into using a bag as a real aug pool. I can imagine how to do part of it (may only work for SoF+ clients though), but the part I am not sure of offhand is how it would actually be saved in the inventory in the case of a crash.

Sorry, I don't mean to shoot down your submission, but if something like this was added, I just think it would have to be done a bit differently.

[l0stmancd]

Hi Trevius,

Thanks for taking the time to review this.

The lore conflict may simply be confusion on my part. I was under the impression that a lore augment was simply an augment that could not be in an item twice. The client itself and other server checks have already determined if a user can hold two of an augment (in hand, etc) by the time the user has them. This check was simply to guarantee there would not be another of the same lore item (or lore group) in the item they are augmenting. If the game should not allow two of the same lore augment -anywhere, in any augmentation slot, on any item - then we should definitely remove the check lore augment code and move to the CheckLoreConflict code. Mind you - this may be unnecessary since by the time the user has two of these same lore augments, the CheckLoreConflict should already have been called (which is why it does not exist in the augmentation procedures as they are in trunk).

Regarding the database query? You are absolutely correct. Can remove this function and switch to the following code snippet.

				for(uint8 ctr = 0; ctr < MAX_AUGMENT_SLOTS; ctr++) {
					ItemInst* augToRemove = toBeAuged->GetAugment(ctr);
					
					if (augToRemove) {
						if (idOfAssumedSolvent == augToRemove->AugDistiller) {
							slotToRemove = ctr;	
							break;
						}
					}
				}

Holding off on fixing those and resubmitting until I understand your last note.

If it is possible to change this to work as a hand held augmentation pool (with item select, etc) this would be a good fix as long as it would handle zone server crashing. I am afraid, though, I am not familiar with how you would do that. If you do not have time to look into this, would you mind giving a two sentence explanation that I can take and dig into?

I agree that this fix is not perfect in so far as you could not select which augment to remove (just the first one it finds of a given type that has been inserted) but it would fix an issue that is causing problems now. That said, given that this would be an optional rule and does not seem to pollute the code surrounding tradeskills / augmentation I do not believe this would be hard to revert out if there was a better solution in the future. I will gladly spend the time to fix any issues you see / think can be improved here if you would still consider the concept for submission. If it would be easier, I can create a set of much smaller diffs that show the code progression more obviously.

Let me know - Took off work until Tuesday of next week and will gladly spend that time working on this.

[trevius]

Well, I had another chance to look into this a bit further to find a way to do this that I think would be good enough to make it into the SVN and I think I found it!

I had a feeling that there may be some existing way to let aug pool windows work from inventory and was about to start experimenting with some ideas. First, I figured I would look into what it is that causes the different existing containers like Collapsible Spit to bring up a tradeskill window. The answer is simple; bagtype. So, I checked the wiki page here:

http://www.eqemulator.net/wiki/wikka...uDBSchemaitems

Under Bagtype there, it lists each type. One of them happens to be:

53 = Augmentation Sealer (for applying augments)

So, searching the DB for an item with that bagtype yielded this:

66180 - Augmentation Sealer

Checking that item out looks like it should be fairly simple to implement a fully functional aug pool that works from inventory.

I don't really have the time to work on the code for it, but I think implementing support for bagtype 53 to act as an aug pool in the inventory would be perfectly acceptable.

You won't need a rule for setting item ID of the item, just need to have it only work from bagtype 53 items.

Also, I don't think you really need to worry about doing things that aren't already done in the current aug pool code. Stuff like the extra lore conflict checking shouldn't be necessary. Though, since the existing aug pool code probably isn't perfect, I don't think some adjustments here and there would be bad if they make sense to do.

I am still a fairly novice to intermediate coder myself. I don't know how best to put it, but if I am finding issues with your code like the previous stuff, it concerns me a bit. So, if you do make another rewrite and submit it here, I will have to make sure I fully understand everything in it 100% before I will consider committing it. Otherwise, I may have to defer to one of the more experienced devs to commit it if it passes their inspection.

[l0stmancd]

Hi Trevius,

I see what you mean by the bagtype change. Just started working on it here. Changing the bagtype to 53 has given a lot of things by default but there still needs to be a code change. Looking into this now.

Thanks!

[l0stmancd]

This is actually a pretty clean change. Still working through it but I think the only really odd part is in the slot numbering. The client is sending slots for the "container" that appear to be shifted. IE: Sending slot 30 for container slot 8 in inventory (29). Consistent in SoD inventory, havent tested bank yet...

Will see if we are handling this in a consistent way in the code already...

[trevius]

It is normal for all SoF+ clients to send shifted slot IDs for all slots over slot 21, since Power Source replaced the Titanium Ammo slot and bumped them all up by 1. This is all handled directly in the <patch>.cpp files near the top such as in this file for SoD:

http://code.google.com/p/projecteqem...atches/SoD.cpp

I made slot converting functions that adjusts Titanium slots to later clients slots and visa versa.

So, even though you see it sending slot 30 (cursor slot in Titanium), when it is in slot 29, it is actually converted back to slot 29 before it is actually handled in any way.

If something is making it into the handling without being converted, it is most likely due to us missing an encode/decode in that .cpp file for that particular packet type. It is pretty easy to add encodes/decodes if needed, we just need to know which packet it is that isn't in there already.

[l0stmancd]

Hey folks,

Rewrote the code submission based on Trevius's recommendation. Turns out it was a fairly simple change with a -lot- less code than before.

The only "interesting" part of it was that I had to add decode functions to the expansion patch files to allow us to shift the slot numbers to the titanium slot numbers. This is done following the same logic as the tradeskill decode functions for the same reason. As I mentioned above, the slot numbers were incorrect between Titanium and SoD due to them using different slightly different slot numbers.

Tested this with ItemID 66180 and also a custom container I created for it - works fine.

Tests ran:
1. Validated that I could still augment slots without issue in aug pool.
2. Validated that I could still remove augments without issue in aug pool.
3. Validated that I could still delete augments without issue in aug pool..
4. Validated that I could agument slots with bags of type 53.
5. Validated that I could remove augments with bags of type 53 (and select which one to remove).
6. Validate that I could delete augments with bags of type 53 (and select which one to remove).
7. Validated that after a successful augmentation, deaugmentation, or augment deletion that the items in your tradeskill container are wiped and the new items are still pushed onto your cursor.
8. Validated all of the above on both Titanium and SoD clients.
9. Validated that items are not lost on zone crash.

You can use the following item ids to test this: 69312, 60332, 47013, 66180

No new logic was added - for the most part this is using the exact same logic that the augmentation pool is using.

Hah - just noticed you responded to my statement - yeah, was testing other things before posting it. Thanks man...

Index: common/patches/HoT.cpp
===================================================================
--- common/patches/HoT.cpp	(revision 1798)
+++ common/patches/HoT.cpp	(working copy)
@@ -3105,6 +3105,16 @@
 	FINISH_DIRECT_DECODE();
 }
 
+DECODE(OP_AugmentItem) {
+	DECODE_LENGTH_EXACT(structs::AugmentItem_Struct);
+	SETUP_DIRECT_DECODE(AugmentItem_Struct, structs::AugmentItem_Struct);
+
+	emu->container_slot = HoTToTitaniumSlot(eq->container_slot);
+	emu->augment_slot = eq->augment_slot;
+
+	FINISH_DIRECT_DECODE();
+}
+
 DECODE(OP_TradeSkillCombine) {
 	DECODE_LENGTH_EXACT(structs::NewCombine_Struct);
 	SETUP_DIRECT_DECODE(NewCombine_Struct, structs::NewCombine_Struct);
Index: common/patches/HoT_ops.h
===================================================================
--- common/patches/HoT_ops.h	(revision 1798)
+++ common/patches/HoT_ops.h	(working copy)
@@ -114,5 +114,6 @@
 D(OP_ChannelMessage)
 D(OP_DeleteItem)
 D(OP_ZoneChange)
+D(OP_AugmentItem)
 #undef E
 #undef D
Index: common/patches/SoD.cpp
===================================================================
--- common/patches/SoD.cpp	(revision 1798)
+++ common/patches/SoD.cpp	(working copy)
@@ -2771,6 +2771,16 @@
 	FINISH_DIRECT_DECODE();
 }
 
+DECODE(OP_AugmentItem) {
+	DECODE_LENGTH_EXACT(structs::AugmentItem_Struct);
+	SETUP_DIRECT_DECODE(AugmentItem_Struct, structs::AugmentItem_Struct);
+
+	emu->container_slot = SoDToTitaniumSlot(eq->container_slot);
+	emu->augment_slot = eq->augment_slot;
+
+	FINISH_DIRECT_DECODE();
+}
+
 DECODE(OP_TradeSkillCombine) {
 	DECODE_LENGTH_EXACT(structs::NewCombine_Struct);
 	SETUP_DIRECT_DECODE(NewCombine_Struct, structs::NewCombine_Struct);
Index: common/patches/SoD_ops.h
===================================================================
--- common/patches/SoD_ops.h	(revision 1798)
+++ common/patches/SoD_ops.h	(working copy)
@@ -104,5 +104,6 @@
 D(OP_LoadSpellSet)
 D(OP_ApplyPoison)
 D(OP_DeleteItem)
+D(OP_AugmentItem)
 #undef E
 #undef D
Index: common/patches/SoF.cpp
===================================================================
--- common/patches/SoF.cpp	(revision 1798)
+++ common/patches/SoF.cpp	(working copy)
@@ -2269,6 +2269,16 @@
 	FINISH_DIRECT_DECODE();
 }
 
+DECODE(OP_AugmentItem) {
+	DECODE_LENGTH_EXACT(structs::AugmentItem_Struct);
+	SETUP_DIRECT_DECODE(AugmentItem_Struct, structs::AugmentItem_Struct);
+
+	emu->container_slot = SoFToTitaniumSlot(eq->container_slot);
+	emu->augment_slot = eq->augment_slot;
+
+	FINISH_DIRECT_DECODE();
+}
+
 DECODE(OP_TradeSkillCombine) {
 	DECODE_LENGTH_EXACT(structs::NewCombine_Struct);
 	SETUP_DIRECT_DECODE(NewCombine_Struct, structs::NewCombine_Struct);
Index: common/patches/SoF_ops.h
===================================================================
--- common/patches/SoF_ops.h	(revision 1798)
+++ common/patches/SoF_ops.h	(working copy)
@@ -88,5 +88,6 @@
 D(OP_InspectAnswer)
 D(OP_ApplyPoison)
 D(OP_DeleteItem)
+D(OP_AugmentItem)
 #undef E
 #undef D
Index: common/patches/Underfoot.cpp
===================================================================
--- common/patches/Underfoot.cpp	(revision 1798)
+++ common/patches/Underfoot.cpp	(working copy)
@@ -3000,6 +3000,16 @@
 	FINISH_DIRECT_DECODE();
 }
 
+DECODE(OP_AugmentItem) {
+	DECODE_LENGTH_EXACT(structs::AugmentItem_Struct);
+	SETUP_DIRECT_DECODE(AugmentItem_Struct, structs::AugmentItem_Struct);
+
+	emu->container_slot = UnderfootToTitaniumSlot(eq->container_slot);
+	emu->augment_slot = eq->augment_slot;
+
+	FINISH_DIRECT_DECODE();
+}
+
 DECODE(OP_TradeSkillCombine) {
 	DECODE_LENGTH_EXACT(structs::NewCombine_Struct);
 	SETUP_DIRECT_DECODE(NewCombine_Struct, structs::NewCombine_Struct);
Index: common/patches/Underfoot_ops.h
===================================================================
--- common/patches/Underfoot_ops.h	(revision 1798)
+++ common/patches/Underfoot_ops.h	(working copy)
@@ -113,5 +113,6 @@
 D(OP_ChannelMessage)
 D(OP_DeleteItem)
 D(OP_PetCommands)
+D(OP_AugmentItem)
 #undef E
 #undef D
Index: zone/tradeskills.cpp
===================================================================
--- zone/tradeskills.cpp	(revision 1798)
+++ zone/tradeskills.cpp	(working copy)
@@ -45,16 +45,48 @@
 		LogFile->write(EQEMuLog::Error, "Client or AugmentItem_Struct not set in Object::HandleAugmentation");
 		return;
 	}
+	
+	ItemInst* container = NULL;
 
-	if(!worldo)
-	{
+	if (worldo) {
+		container = worldo->m_inst;
+	} 
+	else {
+		// Check to see if they have an inventory container type 53 that is used for this.
+		Inventory& user_inv = user->GetInv();
+		ItemInst* inst = NULL;
+		
+		inst = user_inv.GetItem(in_augment->container_slot);
+		if (inst) {
+			const Item_Struct* item = inst->GetItem();
+			if (item && inst->IsType(ItemClassContainer) && item->BagType == 53) {
+				// We have found an appropriate inventory augmentation sealer
+				container = inst;
+
+				// Verify that no more than two items are in container to guarantee no inadvertant wipes.
+				uint8 itemsFound = 0;
+				for (uint8 i=0; i<10; i++){
+					const ItemInst* inst = container->GetItem(i);
+					if (inst) {
+						itemsFound++;
+					}
+				}
+
+				if (itemsFound != 2) {
+					user->Message(13, "Error:  Too many/few items in augmentation container.");
+					return;
+				}
+			}
+		}
+	}
+
+	if(!container) {
 		LogFile->write(EQEMuLog::Error, "Player tried to augment an item without a world object set.");
 		return;
 	}
 	
 	ItemInst *tobe_auged, *auged_with = NULL;
 	sint8 slot=-1;
-	ItemInst* container = worldo->m_inst;
 
 	if (!container) {
 		user->Message(13, "Error: This item is not a container!");
@@ -78,18 +110,14 @@
 		}
 	}
 
+	bool deleteItems = false;
+
 	// Adding augment
 	if (in_augment->augment_slot == -1) {
 		if (((slot=tobe_auged->AvailableAugmentSlot(auged_with->GetAugmentType()))!=-1) && (tobe_auged->AvailableWearSlot(auged_with->GetItem()->Slots))) {
 			tobe_auged->PutAugment(slot,*auged_with);
 			user->PushItemOnCursor(*tobe_auged,true);
-			container->Clear();
-			EQApplicationPacket* outapp = new EQApplicationPacket(OP_ClearObject, sizeof(ClearObject_Struct));
-			ClearObject_Struct *cos = (ClearObject_Struct *)outapp->pBuffer;
-			cos->Clear = 1;
-			user->QueuePacket(outapp);
-			safe_delete(outapp);
-			database.DeleteWorldContainer(worldo->m_id, zone->GetZoneID());
+			deleteItems = true;
 		} else {
 			user->Message(13, "Error: No available slot for augment");
 		}
@@ -104,14 +132,31 @@
 		user->PushItemOnCursor(*tobe_auged,true);
 		if (old_aug)
 			user->PushItemOnCursor(*old_aug,true);
-		container->Clear();
-		EQApplicationPacket* outapp = new EQApplicationPacket(OP_ClearObject, sizeof(ClearObject_Struct));
-		ClearObject_Struct *cos = (ClearObject_Struct *)outapp->pBuffer;
-		cos->Clear = 1;
-		user->QueuePacket(outapp);
-		safe_delete(outapp);
-		database.DeleteWorldContainer(worldo->m_id, zone->GetZoneID());
+		deleteItems = true;
 	}
+
+	if (deleteItems) {
+		if (worldo) {
+			container->Clear();
+			EQApplicationPacket* outapp = new EQApplicationPacket(OP_ClearObject, sizeof(ClearObject_Struct));
+			ClearObject_Struct *cos = (ClearObject_Struct *)outapp->pBuffer;
+			cos->Clear = 1;
+			user->QueuePacket(outapp);
+			safe_delete(outapp);
+			database.DeleteWorldContainer(worldo->m_id, zone->GetZoneID());
+		}
+		else {
+			// Delete items in our inventory container... 
+			for (uint8 i=0; i<10; i++){
+				const ItemInst* inst = container->GetItem(i);
+				if (inst) {
+					user->DeleteItemInInventory(Inventory::CalcSlotId(in_augment->container_slot,i),0,true);
+				}
+			}
+			// Explicitly mark container as cleared.
+			container->Clear();
+		}
+	}
 }
 
 // Perform tradeskill combine