split cmd_disable_attack from unit_thrower.

[amnykon]

No description provided.

[GoogleFrog]

I really dislike how ugly this is. More specifically, this seems like the wrong solution for the problem and I expect this to incur a maintenance cost greater than the benefit of such an obscure feature. I am unconvinced that BlockShot will continue to work into the future in the face of other changes, and it is easy to imagine a balance tweak that accidentally only affects one of the duplicate weapons. If this type of thing is to be generalised to more units like this, then I want a better system.

Perhaps block the attack command at the point it is issued.

[GoogleFrog]

It sounds like you want something like selection rank but for right-click issued attack commands. Perhaps mess around with CommandNotify. See how the split attack widget does things.

[amnykon]

I got a 1/2 working path using gadget:AllowCommand. This working path is similar to LuaRules/Gadgets/cmd_block_ally_attack.lua and conditionally ignores attack commands. Looks like widget:CommandNotify could also be used instead. Is this preferred over AllowCommand?

I still need to convert cmd_manual commands to attack. Once this is done the bogus commands can be removed from all 3 units.

This solution is better then the current one as the current one will not dequeue an an attack command.

I am also going to look at LuaRules/Gadgets/cmd_fire_once.lua and probably use it to optionally repeat attacks. Cmd_manual normally fires once; however, firewalker should have a repeat option as one of it's purpose is to decloak units in an area, the other units mentioned could benefit from this as well.

I am also unsure of removing attack commands from the queue when turning off cmd_dissable_attack. I am leaning towards just leaving them on the queue.

[GoogleFrog]

Looks like widget:CommandNotify could also be used instead. Is this preferred over AllowCommand?

Yes. I don't want this PR to touch gadgets.

[amnykon]

Agreed with exception of the unit_thower gadget. The disable attack functionality is going to be removed from it, as it will be in a widget.

[amnykon]

Requires #4660, (preferably with unwritten part 2)

I still need to convert cmd_manual to cmd_attack calls.

[amnykon]

GoogleFrog: As far as I know, widget:CommandNotify() is sufficient.
CommandNotify is not sufficient, ether widgets handle the command returns true, preventing all other widgets from acting, or they return false, and allow the command to propagate to higher layers. There is currently no way to modify the units the command is issued for and propagate the command to other widgets. #4660 adds this functionality.

[GoogleFrog]

I would prefer if this PR didn't touch Lobster, to keep the diff low and reduce potential regressions.

[amnykon]

Currently CMD_DISABLE_ATTACK is Inserted in the Unit Cmd Desc by LuaRules/Gadgets/unit_thrower.lua
CMD_DISABLE_ATTACK is entangled in unit_thrower.lua, where it really has nothing to do with that gadget.
I am moving it to LuaUI/Wiget/cmd_disable_attack.lua where it belongs. unit_thrower.lua and cmd_disable_attack.lua both cannot handle CMD_DISABLE_ATTACK. Removing CMD_DISABLE_ATTACK from lobster is in integral part of the PR.

The changes to scripts/jumparty.lua and scripts/striderarty.lua on the other hand are artifacts from a previous attempt and should be reverted.

[amnykon]

Updated to work with changes in PR 4660. That PR is required for this to work.

[amnykon]

Updated PR. Units with disabled attack still fire with force fire using custom line formations (force fire + alt). This is a harder problem to solve as custom line formations capture mouse input which is before widgets handles commands. Its also a smaller issue as I don't think that functionality is used as much.

[amnykon]

Updated PR to work with custom formations.

[GoogleFrog]

I don't know, any thoughts @sprunk? I feel like the disable attack toggle on Lobster was put there because I couldn't decide whether Lobster should be used with attack commands or manual fire. It was a "temporary" thing and I think I have come down on the side of manual fire. I would be removing the toggle from Lobster and just making it manual fire, rather than adding the toggle to more units.

Having units with an attack command and a manual fire command that do the same thing feels wrong somehow. It dilutes the clarity of the UI to have two buttons do the same thing, and for the manual fire button to mean both "secondary long reload weapon" and "just another way of issuing an attack command". But on the other hand, manual fire with the block attack toggle is undeniably a feature. It is an extra thing that players can use to customise the game.

The power of defaults say that almost nobody will toggle the block attack command on, or notice it exists. So the average user will just see that Firewalker has a manualfire command that does the same thing. Is it a bug that it seems to just fire its normal weapon instead of a special weapon? They won't know, but some might still find it useful regardless. More fundamentally, if the attack command is so unfriendly that players greatly benefit from having a duplicate command that does the same thing, then shouldn't we work on the default UI rather than patch over it? I want to set my Lances to hold fire and right click on things, I don't want to end up in a world where pressing D and clicking is the accepted way to control these units. Also, notably, I haven't ever heard of someone have an issue with the attack command on Merlin.

The implementation of the Lobster toggle looks prettier than my original one. It avoids block shot and duplicating weapondefs. But maybe it's worth removing the Lobster toggle rather than maintaining an improvement? I would be much happier if the feature was such that players could pick between their units only having an attack command or only having manual fire. Then I would enable this by default for Lobster, and disable it for Merlin etc. Players would not be confused by the addition of a manualfire on their Firewalker and Merlins would not fire when they part of a selection with a manual firing Paladin. Players would have to pick one or the other in the units states menu.

[sprunk]

I dislike the use of WG.units, why not use GiveNotifyingOrder?

[amnykon]

I dislike the use of WG.units, why not use GiveNotifyingOrder?

I believe one of my implementations did; however, I believe some widgets ignored the setting (area attack, custom line formation) this was the only real way to propagate it threw multiple widgets.

[sprunk]

I should be able to do something this weekend.