Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix several issues with left-mouse orders #17291

Merged
merged 5 commits into from Oct 30, 2019

Conversation

@pchote
Copy link
Member

pchote commented Oct 28, 2019

This fixes #13723 the most important part of #13723 - the on-click behaviour changing based on which unit in the selection was closest to the target. This does not attempt to fix the mouse cursor issues which, while annoying, are not as game breaking as orders being ignored if a non-combat unit is closest to the target.

It should be fairly obvious (unless i'm missing something) that doing any kind of filtering here is wrong. If any unit in the selection has a valid order then that should take priority over selecting the target, regardless of any selection priorities or distances.

Also fixes #12922.

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Oct 28, 2019

This is a self-contained and high-impact fix for people who use left-click orders (and doesn't affect right-click orders at all). As we now need another playtest due to the other regressions, I would like to scope-creep this one in too.

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Oct 28, 2019

Updated with another fairly obvious (unless i'm again missing something) fix to sync the cursor to what will happen (order vs selection) when clicking. This should now fix #13723 properly.

@matjaeck

This comment has been minimized.

Copy link
Contributor

matjaeck commented Oct 29, 2019

This has the side effect that you can no longer give a move order at a location with a selected actor without using force move.

In original RA, the move-blocked cursor is shown when pointing at cells with already selected units or when the cell is occupied / blocked. Select seems to be used for unselected selectables unless there are orders against the target / cell (attack, move-blocked, enter).

These mod- specific cursor details are probably out of the scope of fixing the bug so I think it's enough to restore the old behavior for move orders on selected units.

Edit: This is not a change so unrelated to this PR.

Copy link
Contributor

matjaeck left a comment

Fixes the issue in RA without noticeable regressions.

@pchote pchote force-pushed the pchote:left-click-orders branch from 298b7d5 to 42ef0c5 Oct 29, 2019
@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Oct 29, 2019

Fixed.

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Oct 29, 2019

Added a fix for the issue @matjaeck raised above - move orders now override selection if the moused-over target is already selected. This fixes another inconsistency between LMB and RMB orders and makes LMB less frustrating to use.

@pchote pchote changed the title Fix InputOverridesSelection only considering the closest actor Fix several issues with left-mouse orders Oct 29, 2019
@pchote pchote force-pushed the pchote:left-click-orders branch from e126842 to dd3a24b Oct 29, 2019
@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Oct 29, 2019

Updated again to fix @matjaeck's comments in IRC about not selecting other player's units.

@pchote pchote added this to the Next Release milestone Oct 29, 2019
Copy link
Contributor

matjaeck left a comment

Changes work well and improve left-click orders a lot. The cursor behavior is now also more similar to the original.

@pchote pchote added the PR: Needs +2 label Oct 29, 2019
@matjaeck

This comment has been minimized.

Copy link
Contributor

matjaeck commented Oct 29, 2019

There is the side effect that the select cursor is displayed for actors under fog of war. On bleed the default mouse pointer is shown.

@pchote pchote force-pushed the pchote:left-click-orders branch from dd3a24b to 2976f1f Oct 29, 2019
@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Oct 29, 2019

Fixed by updating UOG.GetCursor to show the default cursor if the target isn't an actor with the selectable trait. I also reorganized the method to avoid querying the SelectableInfo in the RMB case except when absolutely necessary.

Copy link
Contributor

matjaeck left a comment

Fix works.

The changes here also seems to fix #12922 (bug is present on 230a0b3).

@abcdefg30 abcdefg30 merged commit 5315f86 into OpenRA:bleed Oct 30, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Oct 30, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.