beyond-all-reason / Beyond-All-Reason

Main game repository for Beyond All Reason.
https://www.beyondallreason.info/
Other
1.69k stars 277 forks source link

Update legacy 60% / gridkeys 60% properly to selectcomm #3564

Closed Hazy-uhyR closed 1 month ago

Hazy-uhyR commented 1 month ago

The old janky comm selection bind was recently changed with the addition of the selectcomm keybind action. The 60% keybinds seems to not have been updated properly after an update to selectcomm.

Addresses Issue(s)

Tab / ctrl-c not focusing on comm after selection on 60% keybinds. That's how it used to work before selectcomm update afaik, and also how it works in grid / legacy 100% binds now.

In addition, gridkeys 60% can append comm to current selection with shift-tab, like on gridkeys 100%. That bind is not in Legacy keys 100%, so I didn't add it to the 60% version.

sprunk commented 1 month ago

I haven't tested and I don't know what the semantics of each key combination are supposed to be (so I won't put an Approve stamp) but at a glance it looks good.

badosu commented 1 month ago

LGTM ty

TheSilverHornet commented 1 month ago

I can't recall the 60% functionality beforehand, but it makes sense to standardise that regardless.

The odd thing is the gadget itself should handle com focussing, I specifically recall adding code to move the camera, so anything that invokes the new selectcomm method, should also center on it. Unsure how that's falling through the gaps, and I can't look into it this second.

That said it was also one of those PRs that went through 'review scope bloat', with a lot changed after the main dev phase, so it is very possible that something was errant in end code when all was said and done.

badosu commented 1 month ago

Yeah, we changed a lot of stuff, in particular moving to action argument decoupling. Maybe it was my fault for not providing better default bindings at the time