New approach for allowing games to perform own selection logic

[badosu]

I noticed this was implemented for allowing games to take over box selection from engine.

I went through all current Recoil consumers and was able to find none that makes use of Set/GetSelectionBoxHandledByEngine.

At the same time, I believe this is an important ability for the engine to support (games that don't want to use engine defaults/want to implement own selection logic), and have delved into and dealt with smart select (one such widget for own selection logic, used in BAR) issues and their causes: mostly inability to infer selection events due to the complexity managing mousepress/release owner and having to ignore engine triggered selections.

I have a WIP PR that implements a BoxSelection(anchorx, anchory, pivotx, pivoty) callin that allows games to:

• Listen for selection decoration/effects
• Use event for some special logic - remember mouserelease only is listened to if we return true on mousepress hence invalidating engine selection
• Perform our own selection logic with passed mouse coordinates and return true to invalidate engine selection

[PS: As well observed by @sprunk it would also make sense to expose BoxSelection{Move,Update} for dynamic selection boxes (logic without releasing box)]

Apart from that it also addresses a few shortcomings I identified with the previous approach:

• Minimap box selection wasn't being considered
• Hardcoded ctrl/shift for remove-from/append-to selection wasn't being ignored when (and only when) in box selection and box selection not handled by engine
• Instead of just returning early, still performing individual unit selection if in box selection and box selection not handled by engine

Since this seems (to me) to be a more appropriate model for exposing this ability to games I suggest allowing this callin to be available. I am unsure if {Get,Set}BoxSelectionHandledByEngine still have any relevant role in this circumstance.

Nothing changes for consumers that don't intend to perform own selection logic and the incurred overhead is minimal (checking that boxSelectionList.size == 0 on mouserelease without mouseowner and dragselectionthreshold met).

Full selection box externalization would perhaps include things like DrawBoxSelection and/or other control callouts, but this does not seem to be a demanded feature (at least that I know).

PS: This is already somewhat possible, at least Zero-K and BAR have special logic for selections that in some way or another override the default engine one. However they rely on 1) checking mouse state with Update which: is not exactly reliable nor desirable to be used, and at least for BAR is a source of issues and unnecessarily complex code; and 2) nooping or double selecting to ignore engine default.

[badosu]

cc @MaDDoXbr @GoogleFrog @Ruwetuin @springraaar @ForbodingAngel @skymyj

[badosu]

Just for the record, I use those words to denote engine deficiencies that reflect on game code and I might be more precious than might be normally accepted for everyday usage. This is mostly because I have an opinion that the API exposed by the engine should help game developers instead of being a hindrance to them, and most importantly enabling games to consume state lifecycles that avoid requiring an inconsistent or difficult to maintain (feature parity wise) reflection of internal engine state.

[sprunk]

Non-deterministic as in the internal lua state mapped to reflect what would be internal engine state depends on non deterministic factors, such as user input.

Okay, but how is that bad? Doesn't that apply to any user input? Wouldn't the proposed dragged box callout ultimately also just reflect user input? I can't think of any use that would require input to be deterministic to do something useful with it, you just take the unpredictable input "unquestioningly" and do something about it. Say a user is dragging a box from (0, 0) to (x, y). It's not possible to predict x and y because they are user input, but you generally just pass them to a Spring.GetUnitsInScreenRectangle(x, y) function regardless.

[badosu]

Doesn't that apply to any user input?

No, if you perform a selection fast enough you might get inconsistent internal state. For simpler models such as mapping mousepress/release this is not the biggest issue, but with higher cpu constraints it might get exacerbated. If you ensure a proper select box callin is called then you get a correct response, as long as SDL captured the events necessary.

[badosu]

I can't think of any use that would require input to be deterministic to do something useful with it

Sure but the question is about having to work around the engine on a faulty model that might not suffice for all games and/or requirements. It's not a huge effort to provide a proper API.

[badosu]

Just an example, regardless of initial proposal BoxSelection or proper SelectionChanged with filterable input you can get a correct state for the rectangle you want to select. This is relevant for smartselect that works dynamically and continuously maps the selection box rectangle, at the end of the selection you want to 'lock' the proper geometry of the selection. Otherwise fast selections don't do their job.

Keep in mind in this case we don't want or trust the engine defined selection, but our own logic.

[sprunk]

Loose thoughts:

• having a way to distinguish the dragging and the release would be good, but having just the on-release version is already an improvement if you're low on mana to implement both.
• deprecating {S,G}etBoxSelectionHandledByEngine sounds good if you can return a bool from the on-release version of the new box callin.
• handling selection on minimap sounds good but ideally there would also be a way to distinguish it from the main view world selection (since it's easy to click the minimap by accident when dragging diagonally and get a completely different selection than intended).
• DrawBoxSelection sounds good but very low priority, the regular plain box is fine.
• it would be good if this was fit for general cases where you might want to drag a box, not just selection. An example use story would be, if you hold some button and drag it displays some information about the inside of the box (for example total value of mexes) without selecting. As far as I can tell your proposed interface already makes this possible, and there may be better ways to do this (area commands?), but it might be worth keeping that use case in mind for design.

[badosu]

Heck, even SelectionChanged is a hack on top of a Callin that is not proper for that.

[badosu]

In fact, I should make a priority for providing a proper SelectionChanged even before this, since all consumers use the current hack in a very inefficient pipeline. LayoutButtons performs logic that is unnecessary for most games and still requires games to invoke callouts to get the data that the proper callin should provide in the first place.

[sprunk]

Native SelectionChanged sounds good, and a good opportunity to add a "source of change" parameter to it? "box-drag", "control-group", "point-click", "death", "lua", ...

In fact, SelectionChanged(..., source = "box-drag") sounds like it would obsolete the proposed event to finalize the dragging of a box selection (and be somewhat simpler since you get to skip the Spring.GetUnitsInScreenRectangle call).

[badosu]

For dynamic selection boxes plugins it would suffice for the definition of the selection and not needing to override the engine passed selection. yes

[GoogleFrog]

Native SelectionChanged sounds best. With sources (engine tends to send integers as sources and put the meaning in a table of constants). I am looking forward to BoxSelection too, but because it will make some hax a bit cleaner, not for legitimate use. BoxSelectionUpdate should be generally good for the preselection style highlighting or selection widgets though.

[GoogleFrog]

#567 Might be related.

[sprunk]

#567 is for the actual selection (regardless if you use the built-in engine box or some sort of Lua selection) and #668 is just for dragging the box (regardless if you use it to select units or anything else).

[GoogleFrog]

BoxSelection(anchorx, anchory, pivotx, pivoty) reminded me of a related issue with selection where having an event for "a selection box just happened" would be useful, but it would only make a hacky widget very slightly less hacky. I made a ticket about it #671

[badosu]

A per discussion with @sprunk here I'm prioritizing a proper SelectionChanged callin (in place of LayoutButtons) instead of the initial proposal.

We might be able to rework this discussion to reflect future requirements of games for full externalization of selection, but since this is not a demand any consumer apparently has for now I won't be proceeding with it.

Thanks for the feedback and helping find a better solution.