Hold mouse button to keep attacking (Diablo 2-style)

[FluffyQuack]

Here's my unfinished implementation of holding down a mouse button to keep attacking or casting spells.

Notes:

• All of the //Fluffy comments are only there so I can keep track of what code snippets I've changed (I've got the memory of a forgetful goldfish).
• I tried to implement this the same way as Diablo 2. The game remembers what you have targeted when you start holding down a mouse button, so the selection will stick even if your target moves or if you move the mouse around.
• The above feature has resulted in a bunch of bugs for selection. Ie, it should de-select a thing if that thing no longer exists.
• I haven't tested this with gamepad controls. I haven't even looked at the gamepad controls code.
• I arbitrarily set the auto-clicking to happen at 200ms intervals. Wasn't sure how that's best configured.
• The player will typically do an extra attack as an enemy dies (the same can happen if you mash click, so I think this is more related to how the game queues input actions than this PR).

[AJenbo]

* All of the `//Fluffy` comments are only there so I can keep track of what code snippets I've changed (I've got the memory of a forgetful goldfish).

Can't say my memory regarding lines I have changed is much better, but I use Git to track what I have changed :)

[AJenbo]

* I arbitrarily set the auto-clicking to happen at 200ms intervals. Wasn't sure how that's best configured.

Check destAction each tick, if it's empty then fire the event again.

[AJenbo]

The player will typically do an extra attack as an enemy dies (the same can happen if you mash click, so I think this is more related to how the game queues input actions than this PR).

See #962

[FluffyQuack]

Can't say my memory regarding lines I have changed is much better, but I use Git to track what I have changed :)

I like having the information right in front of me in the code. It's also useful in case I need to do big merges. I'll remove them when this feature is done.

[FluffyQuack]

Check destAction each tick, if it's empty then fire the event again.

Maybe I could use that for when a specific enemy is targeted, but then use the timer for shift attacks (using only destAction check for shift attacks means any shift attack becomes 2 attacks: one instant and one queued). Edit: Actually, it should always be a combination of both, otherwise clicks with and without shift becomes double attacks.

[AJenbo]

Can't say my memory regarding lines I have changed is much better, but I use Git to track what I have changed :)

I like having the information right in front of me in the code. It's also useful in case I need to do big merges. I'll remove them when this feature is done.

MSVC lets you have the names in the left margin if that is any help:

[AJenbo]

Check destAction each tick, if it's empty then fire the event again.

Maybe I could use that for when a specific enemy is targeted, but then use the timer for shift attacks (using only destAction check for shift attacks means any shift attack becomes 2 attacks: one instant and one queued). Edit: Actually, it should always be a combination of both, otherwise clicks with and without shift becomes double attacks.

ok, maybe add pmode to the mix :)

[FluffyQuack]

Aside from tweaking some comments, this is close to being done I think.

I can think of one known issue right now:

• Some spells make no sense to spam and might result in the player accidentally casting the spell multiple times. For instance, town portal, disarm, or repair. Not sure how to fix that besides hardcoding a list of spells that can't be spammed by holding right-click.

[julealgon]

I hadn't checked this change but I'm surprised to hear talks of a "timer". Is this just a slightly different way to "simulate clicks in time intervals"? Why are we using any sort of timer here at all?

[FluffyQuack]

I hadn't checked this change but I'm surprised to hear talks of a "timer". Is this just a slightly different way to "simulate clicks in time intervals"? Why are we using any sort of timer here at all?

It's used to prevent a single click from becoming multiple attacks. Otherwise, this ends up happening: you intend to press the mouse button once, it ends up being registered as pressed down for 2 gameplay ticks, thus you do an attack instantly and queue up another one.

[julealgon]

it ends up being registered as pressed down for 2 gameplay ticks

This is the problem though, it should not. We need to add support at the engine level for detection of "holds vs presses" and treat them differently.

I think any other solution will end up resulting in hacks somewhere.

[FluffyQuack]

This is the problem though, it should not. We need to add support at the engine level for detection of "holds vs presses" and treat them differently.

I think any other solution will end up resulting in hacks somewhere.

It's just a couple of lines of code. It's easy to change if we ever have a different way of detecting mouse click holds.

[julealgon]

Not a fan of how invasive this is, the fact that hold detection is intertwinned with other logic, all the hardcodes, and the fact that we are not treating "continuous attack" as a separate action.

Won't block the change but I don't think this is the way to go still.

[AJenbo]

It's not clear to me if this is a general issue with the latest master or specifically with the hold mouse feature, but putting it here for now: #2280 (reply in thread)

[AJenbo]

Looks reasonable to me, @FluffyQuack would you strip the name comments from this :)

[FluffyQuack]

I changed the comments and I renamed the enum to be similar to how I think new enums are created in the project.

In the long run, I think the track_process() and track_repeat_walk() functions should be simplified. I'd personally make it function similar to how I did the "hold click to attack" as I find it more straightforward.

[AJenbo]

In the long run, I think the track_process() and track_repeat_walk() functions should be simplified.

yes, and not time based.