COMMITTED: Reagent Consumption

[Leere]

This should fix two problems. The first was reported over on PEQTGC (link). Basically items were consumed as their existence was checked, even if the spell would then fail due to missing some of the later listed ones. I've separated the check and the consumption. Also, all the missing items are now listed, not just the first one encountered.

The second problem was the reagent conservation focus check. It was possible for it to trigger for SE_SummonItem effects and the likes. (Focus check returned 0, the random was 0-100 and a <= check.)

Moving the check for the for-loop continue is probably pointless penny-pinching with memory lookups, I know, but since I was already modifying the code near them...

Index: spells.cpp
===================================================================
--- spells.cpp    (revision 1744)
+++ spells.cpp    (working copy)
@@ -921,20 +921,21 @@
     // first check for component reduction
     if(IsClient()) {
         int reg_focus = CastToClient()->GetFocusEffect(focusReagentCost,spell_id);
-        if(MakeRandomInt(0, 100) <= reg_focus) {
+        // reg_focus of 0 has to fail for SE_SummonItem spells
+        if(MakeRandomInt(1, 100) <= reg_focus) {
             mlog(SPELLS__CASTING, "Spell %d: Reagent focus item prevented reagent consumption (%d chance)", spell_id, reg_focus);
         } else {
             if(reg_focus > 0)
                 mlog(SPELLS__CASTING, "Spell %d: Reagent focus item failed to prevent reagent consumption (%d chance)", spell_id, reg_focus);
             Client *c = this->CastToClient();
-            int component, component_count, inv_slot_id;
-            for(int t_count = 0; t_count < 4; t_count++) {
+            int component, component_count, inv_slot_id, t_count;
+            bool missingreags = false;
+            for(t_count = 0; t_count < 4; t_count++) {
                 component = spells[spell_id].components[t_count];
+                if (component == -1)
+                    continue;
                 component_count = spells[spell_id].component_counts[t_count];
     
-                if (component == -1)
-                    continue;
-
                 // bard components are requirements for a certain instrument type, not a specific item
                 if(bard_song_mode) {
                     bool HasInstrument = true;
@@ -996,7 +997,10 @@
                 else {
                     if(c->GetInv().HasItem(component, component_count, invWhereWorn|invWherePersonal) == -1) // item not found
                     {
-                        c->Message_StringID(13, MISSING_SPELL_COMP);
+                        if (!missingreags) {
+                            c->Message_StringID(13, MISSING_SPELL_COMP);
+                            missingreags = true;
+                        }
     
                         const Item_Struct *item = database.GetItem(component);
                         if(item) {
@@ -1008,35 +1012,41 @@
                             strcpy((char*)&TempItemName, "UNKNOWN");
                             mlog(SPELLS__CASTING_ERR, "Spell %d: Canceled. Missing required reagent %s (%d)", spell_id, TempItemName, component);
                         }
+                    }
+                } // end bard/not bard ifs
+            } // end reagent check loop
+            if (missingreags) {
+                if(c->GetGM())
+                    c->Message(0, "Your GM status allows you to finish casting even though you're missing required components.");
+                else {
+                    InterruptSpell();
+                    return;
+                }
+            }
+            else {
+                for(t_count = 0; t_count < 4; t_count++) {
+                    component = spells[spell_id].components[t_count];
+                    if (component == -1)
+                        continue;
+                    component_count = spells[spell_id].component_counts[t_count];
 
-                        if(c->GetGM())
-                            c->Message(0, "Your GM status allows you to finish casting even though you're missing required components.");
-                        else {
-                            InterruptSpell();
-                            return;
-                        }
-                    
-                    }
-                    else
+                    mlog(SPELLS__CASTING_ERR, "Spell %d: Consuming %d of spell component item id %d", spell_id, component, component_count);
+                    // Components found, Deleteing
+                    // now we go looking for and deleting the items one by one
+                    for(int s = 0; s < component_count; s++)
                     {
-                        mlog(SPELLS__CASTING_ERR, "Spell %d: Consuming %d of spell component item id %d", spell_id, component, component_count);
-                        // Components found, Deleteing
-                        // now we go looking for and deleting the items one by one
-                        for(int s = 0; s < component_count; s++)
+                        inv_slot_id = c->GetInv().HasItem(component, 1, invWhereWorn|invWherePersonal);
+                        if(inv_slot_id != -1)
                         {
-                            inv_slot_id = c->GetInv().HasItem(component, 1, invWhereWorn|invWherePersonal);
-                            if(inv_slot_id != -1)
-                            {
-                                c->DeleteItemInInventory(inv_slot_id, 1, true);
-                            }
-                            else
-                            {    // some kind of error in the code if this happens
-                                c->Message(13, "ERROR: reagent item disappeared while processing?");
-                            }
+                            c->DeleteItemInInventory(inv_slot_id, 1, true);
                         }
+                        else
+                        {    // some kind of error in the code if this happens
+                            c->Message(13, "ERROR: reagent item disappeared while processing?");
+                        }
                     }
-                } // end bard/not bard ifs
-            } // end reagent loop
+                } // end reagent consumption loop
+            } // end missingreags if 
         } // end `focus did not help us`
     } // end IsClient() for reagents

[trevius]

I dunno why this was never committed, but I am bumping it so it can be committed soon.

[Leere]

Recreated locally for a new diff. I've actually tried to apply it to a reverted version too, and it worked. Not sure what I broke about the first version while copy and pasting.

Index: spells.cpp
===================================================================
--- spells.cpp    (revision 1944)
+++ spells.cpp    (working copy)
@@ -933,13 +933,14 @@
     // first check for component reduction
     if(IsClient()) {
         int reg_focus = CastToClient()->GetFocusEffect(focusReagentCost,spell_id);
-        if(MakeRandomInt(0, 100) <= reg_focus) {
+        if(MakeRandomInt(1, 100) <= reg_focus) {
             mlog(SPELLS__CASTING, "Spell %d: Reagent focus item prevented reagent consumption (%d chance)", spell_id, reg_focus);
         } else {
             if(reg_focus > 0)
                 mlog(SPELLS__CASTING, "Spell %d: Reagent focus item failed to prevent reagent consumption (%d chance)", spell_id, reg_focus);
             Client *c = this->CastToClient();
             int component, component_count, inv_slot_id;
+            bool missingreags = false;
             for(int t_count = 0; t_count < 4; t_count++) {
                 component = spells[spell_id].components[t_count];
                 component_count = spells[spell_id].component_counts[t_count];
@@ -1008,7 +1009,11 @@
                 else {
                     if(c->GetInv().HasItem(component, component_count, invWhereWorn|invWherePersonal) == -1) // item not found
                     {
-                        c->Message_StringID(13, MISSING_SPELL_COMP);
+                        if (!missingreags)
+                        {
+                            c->Message_StringID(13, MISSING_SPELL_COMP);
+                            missingreags=true;
+                        }
     
                         const Item_Struct *item = database.GetItem(component);
                         if(item) {
@@ -1020,35 +1025,42 @@
                             strcpy((char*)&TempItemName, "UNKNOWN");
                             mlog(SPELLS__CASTING_ERR, "Spell %d: Canceled. Missing required reagent %s (%d)", spell_id, TempItemName, component);
                         }
+                    }
+                } // end bard/not bard ifs
+            } // end reagent loop
 
-                        if(c->GetGM())
-                            c->Message(0, "Your GM status allows you to finish casting even though you're missing required components.");
-                        else {
-                            InterruptSpell();
-                            return;
-                        }
-                    
-                    }
-                    else
+            if (missingreags) {
+                if(c->GetGM())
+                    c->Message(0, "Your GM status allows you to finish casting even though you're missing required components.");
+                else {
+                    InterruptSpell();
+                    return;
+                }
+            }
+            else
+            {
+                for(int t_count = 0; t_count < 4; t_count++) {
+                    component = spells[spell_id].components[t_count];
+                    if (component == -1)
+                        continue;
+                    component_count = spells[spell_id].component_counts[t_count];
+                    mlog(SPELLS__CASTING_ERR, "Spell %d: Consuming %d of spell component item id %d", spell_id, component, component_count);
+                    // Components found, Deleteing
+                    // now we go looking for and deleting the items one by one
+                    for(int s = 0; s < component_count; s++)
                     {
-                        mlog(SPELLS__CASTING_ERR, "Spell %d: Consuming %d of spell component item id %d", spell_id, component, component_count);
-                        // Components found, Deleteing
-                        // now we go looking for and deleting the items one by one
-                        for(int s = 0; s < component_count; s++)
+                        inv_slot_id = c->GetInv().HasItem(component, 1, invWhereWorn|invWherePersonal);
+                        if(inv_slot_id != -1)
                         {
-                            inv_slot_id = c->GetInv().HasItem(component, 1, invWhereWorn|invWherePersonal);
-                            if(inv_slot_id != -1)
-                            {
-                                c->DeleteItemInInventory(inv_slot_id, 1, true);
-                            }
-                            else
-                            {    // some kind of error in the code if this happens
-                                c->Message(13, "ERROR: reagent item disappeared while processing?");
-                            }
+                            c->DeleteItemInInventory(inv_slot_id, 1, true);
                         }
+                        else
+                        {    // some kind of error in the code if this happens
+                            c->Message(13, "ERROR: reagent item disappeared while processing?");
+                        }
                     }
-                } // end bard/not bard ifs
-            } // end reagent loop
+                }
+            } // end missingreags/consumption
         } // end `focus did not help us`
     } // end IsClient() for reagents

Tested locally: Regular spell with no reags. Single reag spell (with reag not in inventory, and with reag in inventory). Multiple reag spell (with no reags in inventory, with only the first in inventory, with only the second in inventory, with all in inventory).

All cases behaved as expected.

[joligario]

I'll get working on this. Trying to figure out why it doesn't want to apply diff to my local version.

[Leere]

Hmm, it ate the tabs yet again. I have absolutely no idea what I'm doing wrong to get that behavior. They are in the diff file, they are in the message before I hit post, they are in the preview post version, but as soon as I hit post they seem to go bye bye.

Trying one more time.

Index: spells.cpp
===================================================================
--- spells.cpp	(revision 1944)
+++ spells.cpp	(working copy)
@@ -933,13 +933,14 @@
 	// first check for component reduction
 	if(IsClient()) {
 		int reg_focus = CastToClient()->GetFocusEffect(focusReagentCost,spell_id);
-		if(MakeRandomInt(0, 100) <= reg_focus) {
+		if(MakeRandomInt(1, 100) <= reg_focus) {
 			mlog(SPELLS__CASTING, "Spell %d: Reagent focus item prevented reagent consumption (%d chance)", spell_id, reg_focus);
 		} else {
 			if(reg_focus > 0)
 				mlog(SPELLS__CASTING, "Spell %d: Reagent focus item failed to prevent reagent consumption (%d chance)", spell_id, reg_focus);
 	    	Client *c = this->CastToClient();
 	    	int component, component_count, inv_slot_id;
+	    	bool missingreags = false;
 		    for(int t_count = 0; t_count < 4; t_count++) {
 				component = spells[spell_id].components[t_count];
 				component_count = spells[spell_id].component_counts[t_count];
@@ -1008,7 +1009,11 @@
 				else {
 					if(c->GetInv().HasItem(component, component_count, invWhereWorn|invWherePersonal) == -1) // item not found
 					{
-						c->Message_StringID(13, MISSING_SPELL_COMP);
+						if (!missingreags)
+						{
+							c->Message_StringID(13, MISSING_SPELL_COMP);
+							missingreags=true;
+						}
 	
 						const Item_Struct *item = database.GetItem(component);
 						if(item) {
@@ -1020,35 +1025,42 @@
 							strcpy((char*)&TempItemName, "UNKNOWN");
 							mlog(SPELLS__CASTING_ERR, "Spell %d: Canceled. Missing required reagent %s (%d)", spell_id, TempItemName, component);
 						}
+					}
+				} // end bard/not bard ifs
+			} // end reagent loop
 
-						if(c->GetGM())
-							c->Message(0, "Your GM status allows you to finish casting even though you're missing required components.");
-						else {
-							InterruptSpell();
-							return;
-						}
-					
-					}
-					else
+			if (missingreags) {
+				if(c->GetGM())
+					c->Message(0, "Your GM status allows you to finish casting even though you're missing required components.");
+				else {
+					InterruptSpell();
+					return;
+				}
+			}
+			else
+			{
+				for(int t_count = 0; t_count < 4; t_count++) {
+					component = spells[spell_id].components[t_count];
+					if (component == -1)
+						continue;
+					component_count = spells[spell_id].component_counts[t_count];
+					mlog(SPELLS__CASTING_ERR, "Spell %d: Consuming %d of spell component item id %d", spell_id, component, component_count);
+					// Components found, Deleteing
+					// now we go looking for and deleting the items one by one
+					for(int s = 0; s < component_count; s++)
 					{
-						mlog(SPELLS__CASTING_ERR, "Spell %d: Consuming %d of spell component item id %d", spell_id, component, component_count);
-						// Components found, Deleteing
-						// now we go looking for and deleting the items one by one
-						for(int s = 0; s < component_count; s++)
+						inv_slot_id = c->GetInv().HasItem(component, 1, invWhereWorn|invWherePersonal);
+						if(inv_slot_id != -1)
 						{
-							inv_slot_id = c->GetInv().HasItem(component, 1, invWhereWorn|invWherePersonal);
-							if(inv_slot_id != -1)
-							{
-								c->DeleteItemInInventory(inv_slot_id, 1, true);
-							}
-							else
-							{	// some kind of error in the code if this happens
-								c->Message(13, "ERROR: reagent item disappeared while processing?");
-							}
+							c->DeleteItemInInventory(inv_slot_id, 1, true);
 						}
+						else
+						{	// some kind of error in the code if this happens
+							c->Message(13, "ERROR: reagent item disappeared while processing?");
+						}
 					}
-				} // end bard/not bard ifs
-			} // end reagent loop
+				}
+			} // end missingreags/consumption
 		} // end `focus did not help us`
 	} // end IsClient() for reagents

[joligario]

Don't worry. I just went through and did a manual patch. This is in r1945.

[Leere]

Minor correction needed, sorry. Seems in classic EQ fashion I forgot about bards and slightly broke them... *embarrassed*

Index: spells.cpp
===================================================================
--- spells.cpp	(revision 1953)
+++ spells.cpp	(working copy)
@@ -1032,7 +1032,7 @@
 					return;
 				}
 			}
-			else
+			else if (!bard_song_mode)
 			{
 				for(int t_count = 0; t_count < 4; t_count++) {
 					component = spells[spell_id].components[t_count];

[joligario]

This is in r1954

[sorvani]

Originally Posted by Leere

Minor correction needed, sorry. Seems in classic EQ fashion I forgot about bards and slightly broke them..

It's a Sony spy trying to keep the bards down!!!!!!

Errr wait, he fixed it n/m..