Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Item Pickup Improvements #666

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

braccali1
Copy link
Contributor

@braccali1 braccali1 commented Feb 6, 2025

Problem: Character was moving/casting during item pickup routine which moves the screen. The coordinates of an item move when the screen moves. This was causing attempts of PickupItem() to fail.
Solution: Added a check if player is casting or moving in PickupItem() function in pickup_item.go action step with a small sleep to minimize cpu usage. Also added this error to ItemPickup() function in item_pickup.go action to not add to the 3 attempts, similar to error ErrMonsterAroundItem and ErrItemTooFar but without debug logging because this check occurs frequently.

Problem: Refreshing game data for hover status was not returning fast enough, so extra iterations of PickupItem() were occurring when the item was actually hovered.
Solution: Add small delay after mouse move and refreshgamedata() that is right before checking hover status in PickupItem() function in pickup_item.go action step.

Problem: Spiraling too much and timeouts too long. This causes unnecessary delay between retries. Testing showed there is a very low percentage of items successfully being picked up after 6-7 spiral attempts with the above changes, but there was still some success at 8-9 attempts.
Solution: Set max spiraling attempts to 10 (9 in pickup_item.go since it starts at 0). Set the pickupTimeout to not multiple by 2 in the timeout condition check.

Testing showed no blacklisting of items with over 1000 items to pickup, > 90% success on 1st pickup attempt, and an average of 2 spirals per successful pickup attempt. Retries occur much quicker since we are spiraling much less.

EDIT:
Added 2 more retries to the main ItemPickup loop. One following the 2nd and 3rd retry character movement and one that follows the no LOS routine(move beyondposition).

EDIT 2:
Removed item too far from skipping adding to the attempts. There was a loop that could occur if an item was > 7 away but there may have been a door or stash or object in the way. The additional attempts will move more than the 1st or 2nd attempt, and the final attempt executes a movebeyondposition that should resolve the issue.

Set hard values for sleep.
update comment
reduce monstercheckinterval
Remove tab
Had a couple of blacklists. Since we are spiraling less, we can retry more with extra character movement.
Bumped up retries to 5
Added missing ctx.PauseIfNotPriority() to item_pickup.go
@davidfvsilva
Copy link
Contributor

davidfvsilva commented Feb 8, 2025

Tested this PR, and it feels much slower and adds alot of delay to runs. Also, there seems to be a large loop back to go get items that weren't picked up when clearing areas. It seems to have more trouble than usual picking things up (I didn't notice any issues with items being skipped before. The only "problem" I had was not picking up fast enough, but it still felt quick. I wouldn't want to slow it down). I'll give it another go some other time because I could be wrong, and it could have been a bnet issue when I tested. I tried this PR in p8 terror zones.

Removed ErrItemTooFar from not counting toward retries. There could be a loop where the item is too far away and the logic does not move enough or try other moving techniques to get closer. This allows other pickup attempts using moving logic and a movebeyond when the item is more then 7 away.
@braccali1
Copy link
Contributor Author

@davidfvsilva This PR has been a lot faster than the existing pickup routines in all of my testing. Are you sure there are not any other modifications in your local repo?

Added logic to attempt random movement 5 times if item is too far. Pathing to an item that is too far away could be hindered by an object. We don't want to loop forever. randommovement allows for better chances of an attempt to succeed.
@davidfvsilva
Copy link
Contributor

@davidfvsilva This PR has been a lot faster than the existing pickup routines in all of my testing. Are you sure there are not any other modifications in your local repo?

nothing for item pickup on my end, I'll test it again tomorrow on different runs. There just seemed to be some delay between items after the first item pickup when multiples were dropping.

@davidfvsilva
Copy link
Contributor

Alright, I'm testing this morning and it's looking ok for one item at a time, but there seems to be a blockage when many things drop at the same time. I'll keep testing to see if I can get more precise info.

@davidfvsilva
Copy link
Contributor

davidfvsilva commented Feb 9, 2025

For context, this is a drop from a telestomping sequence where the bot is sitting on the target, after Diablo kill, here are some logs:
time=13:52:11 level=DEBUG msg="Item Detected: VampirefangBelt [4] at X:7795 Y:5298"
time=13:52:12 level=DEBUG msg="Pickup attempt 1 failed: item is too far away (7): Vampirefang Belt"
time=13:52:13 level=DEBUG msg="Picking up: Vampirefang Belt [Magic]"
time=13:52:14 level=DEBUG msg="Pickup attempt 2 failed: failed to pick up Vampirefang Belt after 10 attempts"
time=13:52:15 level=DEBUG msg="Picking up: Vampirefang Belt [Magic]"
time=13:52:15 level=INFO msg="Picked up: Vampirefang Belt [Magic] | Item Pickup Attempt:3 | Spiral Attempt:5"
time=13:52:15 level=DEBUG msg="Item Detected: GreaterHealingPotion [2] at X:7844 Y:5285"
time=13:52:17 level=DEBUG msg="Item Detected: GreaterHealingPotion [2] at X:7844 Y:5285"
time=13:52:17 level=DEBUG msg="Picking up: Greater Healing Potion [Normal]"
time=13:52:18 level=INFO msg="Picked up: Greater Healing Potion [Normal] | Item Pickup Attempt:1 | Spiral Attempt:1"
time=13:52:18 level=DEBUG msg="Picking up: Greater Healing Potion [Normal]"
time=13:52:18 level=INFO msg="Picked up: Greater Healing Potion [Normal] | Item Pickup Attempt:1 | Spiral Attempt:0"

I'll do some more testing later, because fast pickup is totally worth it.

@davidfvsilva
Copy link
Contributor

Ok, so its actually very quick most of the time and quite reliable. The issue when theres a bunch of items falling on the floor at the same time might not be related to these changes. If anyone else can test, that would be awesome.

@davidfvsilva
Copy link
Contributor

The performance upgrade PR seems to greatly improve this issue. Looking good!

@braccali1
Copy link
Contributor Author

@davidfvsilva I found something but it does not fully resolve all pickup attempts yet, but it does make hover checking much much faster and lets spiral attempts also occur faster. refreshgamedata takes a very long time to return ishovered on items, and other objects as well. Calling context.GameReader.GameReader.GetData().HoverData.UnitID against the unitid of the item we want to pickup checks hover without waiting for a refresh of all gamedata to return. You will notice that no item pickups occur on spiral attempt 0 with the current version of this PR. I will work on testing/data gathering of different retry attempt logic and add the new hover logic soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants