Revision 746 - moveTo("guildbank");

[Jandrana]

I updated to Revision 746 today. One of my scripts is now failing. I created a small script to reproduce the problem:

My script should put the runes dropping in Sascilia mini game into the guild bank. Guild bank has been opened before starting this script.

Code: Select all

<?xml version="1.0" encoding="utf-8"?><waypoints>
<onLoad>
function MGbuildItemTableOfType(nameOfType)
   local result = {};
   local count = 1;
   inventory:update()
   for i, item in pairs(inventory.BagSlot) do
	   if item.Id ~= 0 then
			if item:isType(nameOfType) then 
				result[count]=item;
            count = count + 1;
			end
	   end
   end
   return result;
end

runes = MGbuildItemTableOfType("Runes");
for j, item in pairs(runes) do
   print("moving "..GetIdName(item.Id).." to guildbank");
   item:moveTo("guildbank");
end
</onLoad>
</waypoints>

Error:

Code: Select all

MACRO Test: ok
Ranged skill found: MAGE_LIGHTNING
[DEBUG] CPU Frequency 2550.371
Loaded waypoint path x.xml
No return path with default naming x_return.xml found.
We use the normal waypoint path x.xml now.
moving Passion I to guildbank
Did not find any crashed game clients.
2:45pm - ...rogram Files/micromacro/scripts/rom/classes/item.lua:355: attempt to
 index local 'toItem' (a nil value)

[lisa]

was the guild bank open at the time?

[Jandrana]

Shorty after posting, I was thinking that you were asking this I edited it in my inital posting.

Yes, guild bank is open.

[lisa]

I just tested your wp and I got the same error, I'll see what I can work out.

[lisa]

Rock will no doubt just pop in and say, oops I did a typo and here is the fix.

As for me, yeah not sure issue yet.

Code: Select all

print(guildbank.BagSlot[1])

prints nil, which is incorrect.

Code: Select all

table.print(guildbank:findItem("Fusion Stone"))

prints the info for item on first spot, as it should. So it works fine.

Code: Select all

print(guildbank:itemTotalCount(0))

printed 413, I have max 400 and 313 are empty, so it doesn't work so well.


Ok so this worked

Code: Select all

Command> guildbank.BagSlot[1] = CGuildbankItem(1) item = guildbank.BagSlot[1] table.print(item)

[rock5]

Nah, not a typo. Just didn't take everything into account when I added the multipage support for the guildbank. Try these 3 files. Your file should work as it is although I'll probably have to check if I didn't break something else.

Code: Select all

print(guildbank.BagSlot[1])

prints nil, which is incorrect.

Because there is no guildbank info in memory until you open the guild, the guildbank class does not automatically update. It will be updated in certain functions such as guildbank:findItem() but to use "print(guildbank.BagSlot[1])" after starting the bot, you would have to do a guildbank:update() first.

Code: Select all

print(guildbank:itemTotalCount(0))

printed 413, I have max 400 and 313 are empty, so it doesn't work so well.

That's another symptom of the guildbank not being updated. Still, I've fixed it so it will work right off.

[lisa]

I used code on first post and it moved the runes to guild bank, 1 item didn't get moved but it may have been from lag. Ran it again and it moved that stack across.

All previous tested code worked fine.

[Jandrana]

Thanks rock5 for the quick fix.

Currently can't test the fix, because servers are down. Will give you an update, if servers are back online.

[Jandrana]

I installed the 3 files and tried my guild bank example. This does work now.

But with the new item.lua

item:moveTo("bank")

seems broken. There is no error, it simply does not move the item.

A simple example that shows the problem:

Code: Select all

<?xml version="1.0" encoding="utf-8"?><waypoints>
<onLoad>
 item = inventory:findItem("Phirius Shell")
   if item then
	  player:target_NPC(110752); -- Marlis Sister (Varanas West)
	  sendMacro("ChoiceOption(5);");   
	  print("moving "..GetIdName(item.Id).." to bank");   
      item:moveTo("bank")
   end
</onLoad>
</waypoints>

PS: This was tested with the new 5.0.7 patch. Hopefully the new patch did not introduce new problems.

Edit: Just did a few more tests with the new version of item.lua and found out that this problem has some kind of randomness.
Sometimes it moves the item, sometimes it doesn't. Maybe lisa encountered a similar problem with guildbank.

It seems that it does not work if you did a fresh login. Maybe a "bank update problem"?

[rock5]

No, the guildbank issue was due to changes to the guildbank class. No changes were made to the bank class. I'll check as soon as the update finishes installing.

[rock5]

The addresses seem to have updated correctly and moving to the bank seems to work.

I think your problem is that you are not giving it enough time to open the bank. Try adding a yrest after the choiceoption, about 1000ms should do it.

[haplo]

Cany you move the Phirius Shells to the guild bank? they are bound

[rock5]

Jandrana is talking about 'bank' not 'guildbank'.

[Jandrana]

I think your problem is that you are not giving it enough time to open the bank. Try adding a yrest after the choiceoption, about 1000ms should do it.

Ok, will try it. But this was not necessary up to now. Maybe some of optimizations done for the new update caused a speedup resulting in this timing issue.

[rock5]

We have always needed a pause to wait for dialogs, stores , banks, etc. to open. If you haven't needed to till now then you were just lucky.

[Jandrana]

I still have issues with bank access that were not present before the major upgrade. I have some other problems, but I think I will create an extra topic for this.

I've written a test bank access script that shows the problem.

Before the major upgrade an item being moved to the bank (phirius shells in my case) was always placed on the existing stack. After the update the first item being moved to the bank, is being places on a free slot, if a free slot is available - otherwise this move fails. I tried to increase wait times, added "bank:update()" but nothing removed the problem. Simply run the script two times: once with bank full, once with several free slots.

This brings me to the idea, that we should have more such test scripts. In various other programming languages so called "unit tests" (http://en.wikipedia.org/wiki/Unit_testing) are being used to verify if other parts of the software are working as expected. At my work we have several thousands of tests running during the night and are testing if the currently released software modules are still working as expected.

If we got something similar for the bot, this could make things easier in several aspects:

- people writing scripts would have an example how to do things - this acts as a kind of documentation
- if people have a problem, tests can reveal what is the problem
- you can verify, if the bot still works after internal things have been changed.

Of course you have some additional work - writing and maintaining tests can also be time consuming. But you gain much more safety in the end. So I would like to encourage people to write some tests, if you have problems. Perhaps we can have a "special place" for them (may be in the SVN).

Tell me what you think.

[rock5]

Ok, I've done some testing.

Issues:

• 1. I had to change all the table.getn to # because I'm using a test version of MM that uses lua 5.2 and that function is no longer available in 5.2. But that shouldn't affect you.
  2. You used the slotnumber in the split command so it didn't work so I changed it to BagId.
  3. I've done a lot of work in the past making item moves super fast and reliable. It all involves looking at the states of the from and too slot as well as the cursor. The fact is there is no way to make splitting or merging fast. There isn't even any indicator that it's ready to split again so there is no way to make it as fast as possible even if it is slow. So you have to add big enough pauses to make sure it doesn't fail. That said, I was having a lot of trouble trying to just get the splitting to work. It only work reliably if I had pauses of about 750 to 1000ms.
  4. There is definitely something fishy going on with the merging. After some more extensive work I figured out that there is a bug when it's looking for a stack to merge with. It gets the item but doesn't update it. A bank:update() after each merge in your script fixes it but I'll be fixing it in the bot too.

In the end having about 500ms for both the splitting and merging seemed to work well.

You do realise though that this is highly unusual. You would not normally have lots of smaller stacks in your bags. Normally you would only need to merge one stack in you bags to the stack in the bank or you would move full stacks to empty slots. All of which probably already worked properly and efficiently.

At least after all this work I found a bug to fix.

Nearly forgot. Me and lisa have both dabbled in making scripts to test the memory addesses mainly. I never posted my work but later Lisa posted her work but they both only barely scratched the surface. For starters it would be nearly impossible to test everything and second most of the tests would require you to be at a particular place to run the test so you couldn't just run all the tests in one place. There would be a lot of running around so it would actually be a lot of work to run them. Also I think the tests you run at work have expected results for each test. The results would be different for each character with a lot of the tests so you wouldn't have expected results.

[Jandrana]

That said, I was having a lot of trouble trying to just get the splitting to work. It only work reliably if I had pauses of about 750 to 1000ms.

Hm, ok. Maybe this is related to the ping you have. I usually have 16-35ms. With that ping pauses of 250ms seems to be sufficient.

You do realise though that this is highly unusual. You would not normally have lots of smaller stacks in your bags.

In this test case, of course. But I 'using a script, where a char is collecting stuff that he gets via trading. After a trade items are not stacked automatically, so you can have 5 stacks of the same item in your bags. Dropping these into the bank, would be the same like in my test.

At least after all this work I found a bug to fix.

Then it was worth writing this test. It just was a "first shot", how this could be done. Just think about it. Of course you can't test every thing.

Also I think the tests you run at work have expected results for each test. The results would be different for each character with a lot of the tests so you wouldn't have expected results.

Test do have expected results, of course. That is the main reason writing them. This way you start thinking about how things should be and what is expected and unexpected behavior. In our case we would have several aspects that would be general behavior (movement, accepting quests, dealing with items, bags, bank etc.) and things that may be class specific. A class specific test does only make sense if being used with that class.

I don't think that you should write all tests - mostly it's even better that "users" write tests, because they don't know how things work internally, so they look at it from a different view point which sometimes reveals weaknesses or bugs.
If such a thing as a "test framework" for the bot should happen, it should be community project. But if this should happen, there should be some kind of guideline/template how tests should be written so they could be run easily by many people.

[rock5]

ok, but I don't think you'll muster much interest.

[Jandrana]

At least after all this work I found a bug to fix.

rock5, did you already find a fix for this problem? Is it ready for release?