Handin plugin confusion...

[Kagehi]

Still don't have a server up and working, but been playing around on PEQ. I noticed things going wrong in one area, so decided to see what it would take to patch in the check_handin function to them, and in the process, try to figure out what the heck it does and how. I think I have a good grasp of it now, but I have two questions:

1. itemcount - is this being passed as a reference or a table? Regardless of which, does it automatically "unstack" items, or does it leave them stacked in a keyed table?

2. I noticed that the code in the plugin, as written, can't handle stuff like "1234 => 3". It will only look at the index key, not the *value*, when checking if it found an item, and determining if it was a success. This is a) imho stupid, and b) something they fixed in live, like two years ago, within probably less than 1-2 months before I stopped playing. While I suppose its sort of amusing seeing the problem appear in EQEmu too, its also annoying.

Needless to say, fixing this relies on what the answer to #1 is, and its not going to be as simple as just checking item numbers and hoping that the coder knew to explicitly ask for one item 2 times, instead of one items, of which you need 2. I.e. '1234 => 1, 1234 => 1' vs. '1234 => 2'. I see absolutely no way how the later, which is valid when doing a straight check, would work at all, when calling plugin:check_handin. If itemcount just returns a table, in the way its being passed, which shows the item, and how many where stacked, not separate items, then this is broken on both ends. If its passing a reference, then this is a bit easier, since one could just ask it, presumably, "How many of these are there, really, and does it match the number actually requested?" In fact, that method is hundreds of times preferable imho.

Sorry, but I just get nit picky about stuff like this. lol

[Kagehi]

Ok. Did some tests in a nice text based client, which supports Perlscript. After some nightmares trying to figure out why it wasn't working, I got these results:

Looking for                    Found                         Result
----------                    ------                         ------
1234 => 1                    1234 => 4                    Fail
1234 => 4                    1234 => 1                    Fail
1234 => 1                    1234 => 1                    Success

This is obviously not as bad as I was expecting, but it still means that you can't, for example, ask for 1234 => 2, and have someone give you '1234 => 1, 1234 => 1'. Its not testing how many you have, just if the hashes are exact matches, with both the keys *and* the values. This is flawed imho, even if it does prevent huge errors.

[AndMetal]

I think this all has to do with how the items are read.

If I'm not mistaken (one would have to look through the source to confirm), the server finds out what the Item IDs are for each slot & stores them as $item1, $item2, $item3, & $item4. Then, an array (table) is created from that info: %itemcount. I assume the logic is something like this:

%itemcount{$item1}++;
%itemcount{$item2}++;
%itemcount{$item3}++;
%itemcount{$item4}++;

That way, if you hand in 2 of the items, it increments the key, which is the Item ID, by 1.

I agree, the "better" way to do this is to do something like this:

%itemcount{%item1{id}} += %itemcount{%item1{stacksize}};
%itemcount{%item2{id}} += %itemcount{%item2{stacksize}};
%itemcount{%item3{id}} += %itemcount{%item3{stacksize}};
%itemcount{%item4{id}} += %itemcount{%item4{stacksize}};

That way, you can stack items AND have them ALL count for the quest.

However, the issue then becomes backwards compatibility. I would think the best way to handle that would be to keep $item1-4 as is, and change just %itemcount to handle the new logic.

Anyone up for digging through the source code?

[Kagehi]

Well. Truth is, the code for handin is just broken. The return code does, sort of, what it *should be* doing. Handin does: "For every key+value in what I was given, is there an *identical* key+value in what I need?" No attempt is made to check if its the right item, but you just have too many. Item_return does: "For every key in what I am given, start with 0 items, and increment until I exceed the *value* of that key."

The code needed for handin is kind of more complicated though. It needs to combine its "did it exist at all", with its, "is it a match or not", parts, so that you do something like:

for every KEY in what I want do
  Is there a key in what I was given that matches?
    yes - is the value > than, = to, or < than what I need?
       if equal, delete both keys and continue.
       if >, then delete the hash key and decrease the given one by the hashes value.
       if <, then delete the given key, subtracting its value from the hash, and return to step 2.
  no - exit with fail.

When you run out of keys and values in the hash, you pass what ever is *left* of the given items to return_items.

The existing code works perfectly, as long as you don't turn in stacks, or your stacks are the *exact* size passed to the test, but otherwise, it just doesn't work as written.

[Theeper]

Maybe you have an old version of the check_handin.pl, this version from PEQ works fine for me. It does check for more than one item at a time.

AFAIK, you can't pass stacked items to NPCs on the emu or on Live though.

# plugin::check_handin($item1 => #required_amount,...);
# autoreturns extra unused items on success
sub check_handin {
    my $hashref = shift;
    my %required = @_;
    foreach my $req (keys %required) {
	if ((!defined $hashref->{$req}) || ($hashref->{$req} != $required{$req})) {
            return(0);
	}
    }
    foreach my $req (keys %required) {
	delete $hashref->{$req};
    }
    return 1;
}

sub return_items {    
    my $hashref = shift;
    foreach my $k (keys(%{$hashref})) {
	next if($k == 0);
	my $r;
	for($r = 0; $r < $hashref->{$k}; $r++) {
		quest::summonitem($k);
	}
	delete $hashref->{$k};
    }
}

1;

[Kagehi]

I was pretty sure that Live did, at some point, fix it so that stacked items where treated as though they where handed in separately... I even remember people talking about the change, while I played. I suppose I could be wrong, but I am pretty sure it was a major deal at the time it happened.

As for the plugin. That is the code I have. The problem is, you **can** tell it to look for more than one item, and you *can* try to pass a stack to it, and it will work.

For example, if you use that code, and you do:

%itemcount = (59954 => 1, 86010 => 2, 13068 => 1);
if (check_handin(\%itemcount, 59954 => 1, 86010 => 2, 13068 => 1)) {
  $world->note("Great!");}
else{
  $world->note("Fail!");
}

It will return true and display "Great!".

NOTE: $world->note is the function in the text client I am using as a test bed to figure it out which prints to the output stream. Normally you would, of course, use print.

So, obviously, the code *is* checking stacks, but its comparing the literal stack you ask for "two of item 86010" to the literal stack of "I gave you two of item 86010". If you substitute any other value, in either the request list, or the items given hash, it fails.

I.e., if the intent is to not allow stacks, which I still say they changed in live in one of the more recent updates (I was on a progressive, so things changed all the time from patch to patch), then its **still** wrong, since it accepts them, just *only* if its an exact match to the request, as above. If you don't want stacks, then you need an additional check, which says:

If hashref->{$req} > 1 {return 0;}

*Before* you check for it. Otherwise, if someone takes existing code, like from Vahlara in the tutorialb area, and rewrites it to say, well, the above example, which is taken from that code, then there "lookfor" is going to be wanting a stack, and will fail *every time*, since people will be giving it:

$item1 = 59954
$item2 = 86010
$item3 = 86010
$item4 = 13068

But its expecting:

$item1 = 59954
$item2 = 86010 x 2
$item3 = 13068


See what I am saying?

[Kagehi]

Ok. Been kind of carrying on a two sided conversation on this between here an the PEQ forum. As it stands, the code is *currently* working, since the server itself doesn't allow stacks on hand ins. So far so good. However, you are dead wrong about Live and handing in stacks. Just google the terms "everquest hand in stacks" and on the first page is a link to the Epic Ranger quest, in which the poster says, "Head back to Felwithe South, run to the mage guild, and hand in wings, 4 per stack in all 4 give slots, to Niola Impholder, a trainer in the Felwithe Mage guild."

Oops! I was sure I was right about that changing, but glad I can find confirmation. And its definitely EQ1, since handins are either handled as sales style menus, as quest counters, where you don't need them "in inventory" to hand them in (they are autoconsumed as collected, then updated in the quest data), or, in a few rare cases, taken direct from your inventory, as soon as the quest knows you have them, and you talk to the NPC who wanted them. A lot cleaner and less bug prone than EQ1s method imho.

[So_1337]

Found a bit of information. I searched as hard as I could for patch notes that mentioned any change, but I sure couldn't. Have to settle for the historical 'accuracy' of Allakhazam comments =P

Originally Posted by Source (Jul 31st 2003)

You mentioned turning in 4x4 stacks... I am not familiar with the quest you mentioned, but no quest I am familiar with allows you to get credit for stacked items (only the first one on the stack). For example, turning in 4 stacks of bone chips just gives you credit for 4 bone chips. If this wasn't taken into consideration for the 5k muffin quest, that would mean you gave 20 times as many muffins as necessary (you would only need 250). Anyhow, just a thought. *shrug*

Originally Posted by Source (Aug 7th 2003)

It's not such a duh. If you've done the bone chip turn in in Cab, then you might think that stacking bone chips is allowed, since for that quest, it is.

Frankly, I don't understand why they coded it so you have to do individual items. If it requires 2, then stack 2 of htem.....or at least let me take just one out.

SOE is just one big carpal tunnel lawsuit waiting to happen.

Originally Posted by Source (Oct 28th 2003)

Yah, people were factioning just waay to quick for them back when you could hand in 4 stacks of 4 things at a time. Was too much of a pain for them.

Seriously, I think they removed the stacking issue to enable the code that makes NPC's hand back items they don't want. Granted they won't hand back things stacked now, but at least they'll have back un-needed or accidentally given items. Wish they could find a way to have them hand back everything not needed so we can hand in stacks again.

I absolutely agree that it was changed at some point, but I recall it specifically being only certain quests that allowed it to happen. The bone chip turn-in for Cabilis faction (as mentioned above) is one of them, and I remember cheering when Chardok factioning (turning in salts and skins to the Herald) was changed to allow stacks as well.

I personally have no problem allowing stacks to be turned in. You've got the big green light as far as being true-to-Live goes. Can it be coded as such without breaking anything else?

[Theeper]

%itemcount = (59954 => 1, 86010 => 2, 13068 => 1);

This will work fine on the emu. It won't fail if you handin all 4 items seperately.

There may be quests on Live that allow stacked handins, I don't remember ever doing one, but I probably did. I couldn't find one that did accept stacks on Zek last night. I will check the ones you mentioned though.

[Theeper]

The proper syntax to use the plugin is like this though.

if (plugin::check_handin(\%itemcount,59954 => 1, 86010 => 2, 13068 => 1))

[Kagehi]

Hmm. The comment made on PEQ forum was that one might want to match the function of the game "at the stage of the patch supported". I.e., if Ykesha didn't allow it, then it shouldn't be allowed for that quest set. This presents either a major, or a minor problem. Kind of depends. I haven't looked into how globals work, so I don't know if, due to them being stored in the DB, if you have to define them, before using them? Reason I wonder is, the easiest solution is to have a setting, which can be checked against, which if not found/set to false, will cause the code to treat things as "don't allow stacks".

This still means rewriting the hand in to make it work properly, both with requests for multiple items, like normal quests do automatically, and for stacks, when allowed (can check the same setting to determine if the handing in of a stack causes an immediate return 0).

Perl on the other hand is giving me hives. Feels like I am trying to code in regexp... lol Now, it seems that the variable that %itemcount gets shifted into becomes a "reference" to that variable. This is why 'delete blah->{blah};' works with it, instead of 'delete blah{blah};'. If you use the wrong one, Perl spits out a syntax error, in my tests. Now, the simplest solution is to do what I suggested before. I.e., decrement which ever one is higher, delete the one that hits zero, then pass the remains of the %itemcount back, once you are sure everything handed in right, so return_items can give everything back, minus the items taken. This means making a copy of the hash, so if you hit a fail condition, while doing this, we can simply substitute the copy for the current hash, then return, with it intact. Supposedly, its as simple as doing '%temp_copy = %hashref;', but since I already ran into a problem using the wrong delete, I figure it might be a good idea to ask if doing a hash copy with something that contains a "reference" to a hash, will produce a copy, or if its going to freak, because I am trying to copy the hash, not make a new reference.

I am so confused by the syntax conventions going on here... :(

[AndMetal]

Well, I had a chance to dig through some of the source, and this is what I've come up with:

zone/embparser.cpp

  435 		case EVENT_ITEM: {
  436 			npcmob->FaceTarget(mob);
  437 			//this is such a hack... why arnt these just set directly..
  438 			ExportVar(packagename.c_str(), "item1", GetVar("item1", npcid).c_str());
  439 			ExportVar(packagename.c_str(), "item2", GetVar("item2", npcid).c_str());
  440 			ExportVar(packagename.c_str(), "item3", GetVar("item3", npcid).c_str());
  441 			ExportVar(packagename.c_str(), "item4", GetVar("item4", npcid).c_str());
  442 			ExportVar(packagename.c_str(), "copper", GetVar("copper", npcid).c_str());
  443 			ExportVar(packagename.c_str(), "silver", GetVar("silver", npcid).c_str());
  444 			ExportVar(packagename.c_str(), "gold", GetVar("gold", npcid).c_str());
  445 			ExportVar(packagename.c_str(), "platinum", GetVar("platinum", npcid).c_str());
  446 			string hashname = packagename + std::string("::itemcount");
  447 			perl->eval(std::string("%").append(hashname).append(" = ();").c_str());
  448 			perl->eval(std::string("++$").append(hashname).append("{$").append(packagename).append("::item1};").c_str());
  449 			perl->eval(std::string("++$").append(hashname).append("{$").append(packagename).append("::item2};").c_str());
  450 			perl->eval(std::string("++$").append(hashname).append("{$").append(packagename).append("::item3};").c_str());
  451 			perl->eval(std::string("++$").append(hashname).append("{$").append(packagename).append("::item4};").c_str());
  452 			break;
  453 		}

Which basically translates to this:

%itemcount = ();
++$itemcount{$item1};
++$itemcount{$item2};
++$itemcount{$item3};
++$itemcount{$item4};

I tried to find the packet structure of the handin, so I could then see what is referenced, but I didn't have much luck.

Anyways, hope this helps.