diasurgical
Auto Pickup more frequently
Fixes: https://github.com/diasurgical/DevilutionX/issues/8334
Triggers auto pickup on beginning the stand animation regardless of what action led to the player reaching the stand animation.
While this is a nice improvement, wouldn't it be possible to generalize it by making the logic "proximity-based" regardless of how the player got close to the gold tiles?
Otherwise, each and every case is always going to be a special case.
While this is a nice improvement, wouldn't it be possible to generalize it by making the logic "proximity-based" regardless of how the player got close to the gold tiles?
Otherwise, each and every case is always going to be a special case.
Then it becomes impossible to drop the respective items without immediately picking them back up
Then it becomes impossible to drop the respective items without immediately picking them back up
You could add a field to track who/what dropped the item (the source), then use that to avoid automatically picking up piles explicitly dropped by the player. Just an idea.
Then it becomes impossible to drop the respective items without immediately picking them back up
You could add a field to track who/what dropped the item (the source), then use that to avoid automatically picking up piles explicitly dropped by the player. Just an idea.
Seems like unnecessary complexity. The original behavior is auto pickup on completion of walk movement, this PR expands that to auto pickup on completion of any type of movement.
The original behavior is auto pickup on completion of walk movement, this PR expands that to auto pickup on completion of any type of movement.
That does evoke one thought in my mind. AutoPickup() is called in DoWalk() on the last frame of the animation. This PR calls AutoPickup() on the hit frame of the casting animation. If you were to click on an item after casting Teleport, you would need to wait for the casting animation to finish before picking up that item. So technically auto-pickup is getting special treatment.
On the other hand, it's possible to shortcut the casting animation by casting another spell so you may never even reach the last frame of the casting animation for Teleport. If we attempt to move auto-pickup to the end of the casting animation, then this becomes a somewhat unreliable feature.
Something to think about.
The original behavior is auto pickup on completion of walk movement, this PR expands that to auto pickup on completion of any type of movement.
That does evoke one thought in my mind.
AutoPickup()is called inDoWalk()on the last frame of the animation. This PR callsAutoPickup()on the hit frame of the casting animation. If you were to click on an item after casting Teleport, you would need to wait for the casting animation to finish before picking up that item. So technically auto-pickup is getting special treatment.On the other hand, it's possible to shortcut the casting animation by casting another spell so you may never even reach the last frame of the casting animation for Teleport. If we attempt to move auto-pickup to the end of the casting animation, then this becomes a somewhat unreliable feature.
Something to think about.
I suppose the correct way to do this then would be to go back to my first approach, which is to call it every time the player starts the standing animation. This already occurs indirectly, because StartStand() is called, then AutoPickup() is called shortly after.
I suppose the correct way to do this then would be to go back to my first approach, which is to call it every time the player starts the standing animation.
I'm not so sure about that, because it would trigger auto-pickup when transitioning from other completely unrelated actions, like a melee attack or a Fireball spell.
That said, you should take what I say with a grain of salt. I can only imagine the type of person who would really care about having this feature because I am not one of them. Perhaps the player who turns on this feature doesn't actually care about these little details, and they just want the gold and potions to be picked up automatically along their path. And perhaps even the purists wouldn't particularly care if someone could theoretically have a slight advantage by turning on auto-pickup because it's just gold and potions. Really low-stakes stuff.
, you should take what I say with a grain of salt. I can only imagine the type of person who would really care ab
You're right though about potential issues. Through testing, I noticed that there was no autopickup while chain casting Teleport. or immediately queuing another action after a teleport. Doing autopickup when the player goes to idle animation doesn't fix that, but it's at least consistent. If the player has this option enabled, I would assume they want to pick up items as soon as possible. This is the closest we can get to that without running the function every frame.
Through testing, I noticed that there was no autopickup while chain casting Teleport. or immediately queuing another action after a teleport.
Huh, I'm not seeing that at all. With only the first commit of this PR, I can't walk or teleport next to a gold pile without picking it up, no matter how I try to queue other actions.
Through testing, I noticed that there was no autopickup while chain casting Teleport. or immediately queuing another action after a teleport.
Huh, I'm not seeing that at all. With only the first commit of this PR, I can't walk or teleport next to a gold pile without picking it up, no matter how I try to queue other actions.
I tested the first commit and was able to not pick up a gold pile by clicking and holding right click onto the pile and continuing to hold right click to teleport off of it. The pile was still there.
That's because auto-pickup doesn't work on items you're standing on.
Yikes I didn't realize that