feat: select idle builders by distance to camera - #5498
Conversation
|
Might want to add support for sorting via distance to cursor (which is the better alternative when using a hotkey instead of clicking a button with a mouse). Apprently this action doesn't explicitly support a hotkey yet (?) so it's probably fine to just leave a FIXME or a ticket or something. Also it would be good to apply this behaviour to other actions that select something (also not necessarily in this PR). |
Stale review, Saurtron still out of commission, refer to #5498 (comment) for hihoman's logic. TLDR; Saurtron's comment is sound and should ideally follow his suggestion, but it isn't an outright blocker.
|
@SethDGamre @sprunk @efrec |
efrec
left a comment
efrec
left a comment
There was a problem hiding this comment.
I see the point about splitting the difference between searches from hotkey and non-hotkey input. But LGTM otherwise, code is sane, only hoping for a future enhancement.
sprunk
left a comment
sprunk
left a comment
There was a problem hiding this comment.
Code looks alright. I haven't tested though.
Test Results16 tests 7 ✅ 3s ⏱️ Results for commit 4f5ba55. ♻️ This comment has been updated with latest results. |
|
|
Sounds worth addressing but it's not from this PR. |
|
Left click works great, cycles though all idles in distance from camera. Right click, which also moves the camera, seems weird though. 
In the above screenshot/configuration, the next right click is actually going to the rez bot on the far RIGHT of the screen (off the current camera fov) and not the one on the left, in fact repeatedly pressing it never went to the one on the left. There are many such configurations depending on when the selection was changed and where camera started. In Master/Stable it correctly cycles through every idle unit with repeated clicks. |
|
We pulled and tested in PR review and think similarly. This is close but not completed. |
|
@hihoman23 Have you had a chance to address TheChronographer's feedback? |
|
No, not yet but I can reproduce the issue. |
|
It should be fixed now. |
Integration Test Results16 tests 8 ✅ 3s ⏱️ Results for commit f2ac041. |
|
Pulled the pr, seems to still select cons at random to me 2026-04-02.21-14-18.mp4 |
|
@hihoman23 Any feedback on this? |
|
I can't fully see how the planes are arranged, but it seems to work fine on my branch, so I can't really repro. |
|
@PtaQQ could you maybe send another clip of this happening? |
|
Sorry, I missed your message, here is another video: https://youtu.be/E-8Pk3_WU4A I'm not sure if I'm doing something wrong or I misunderstood the intent of the PR, but basically it seems it selects builders in an order of closest to furthest from world origin (0,0 coords). Also, when using the ctrl+b keybind (legacy keys for select AllMap+_Builder_Idle+_ClearSelection_SelectOne+) it has a slightly different behaviour too. |
|
@hihoman23 Any updates on this? |
efrec
added
the
Needs Implementer
|
Needs a retest and to be updated. May have new behaviors with mouse bindings soon: beyond-all-reason/RecoilEngine#3116 Since the mouse actions could become unbound. |
|
No additional changes are striclty required as far as the mouse bindings are concerned, just that this would continue to behave using the hard-coded mouse buttons without adjustment (and you shouldn't do that adjustment until that PR is in anyways). |
10 participants
Work done
The idle builder list is sorted by distance to the camera to select the closest idle builder to the camera.
fixes #5496