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.

Sign up for GitHub

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

Jump to bottom

Add gesture communicator #3802

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Add gesture communicator #3802

wants to merge 10 commits into from

Conversation

esainane
Copy link
Contributor

This is a menu that can be bound to a key, such as BACKQUOTE. When opened, the user is presented with a series of radial menus which select predefined text for a marker to be added at the initial point. Inspired by DotA's hot chat wheel, but as Zero-K has much more we want to say, why not take inspiration from the existing and rather impressive gesture build widget?

The exact predefined text is open for discussion and/or total bikeshedding. I'm of two minds about some, especially those in the six-o-clock submenu. Ultimately, I left them in, as I feel having markers more descriptive than "omg" or "lobster", which now also take less effort to send, might help to nudge people into being slightly more helpful in very common situations. Maybe. One can hope.

This necessarily involves exposing the real MarkerAddPoint to widgets, which is also something to consider.

Since there would have been extremely high overlap with the gesture build widget, the gesture build widget has been refactored into an API widget that provides the overwhelming majority of common code and takes callbacks. This also means a better functional separation of concerns - gui_gesture_build and gui_gesture_comms are absolutely tiny compared to the previous gui_chili_gesture_menu, and api_gesture_menu is slightly more readable too.

Included are a few bug fixes and cleanups for the gesture build menu, which can be turned into a separate PR if desired.

Wishlist:

  • Find a nicer way of initializing values of the default instance options
  • Work out whatever was supposed to happen with determining whether the minimap location was left or right, and either fix or remove it as appropriate
  • Turn right-click-drag options into a radio menu, with each gesture menu instance adding their own entry ... somehow, eventually
  • Have some sort of receiving widget that can optionally translate the predefined text into each player's native language ... somehow, eventually

@sprunk
Copy link
Member

sprunk commented Feb 21, 2020 •
edited
Loading

This necessarily involves exposing the real MarkerAddPoint to widgets, which is also something to consider. (...) Have some sort of receiving widget that can optionally translate the predefined text into each player's native language

Replace Spring.MarkerAddPoint(x, y, z, "Attack here!") with Spring.SendLuaUIMsg("ChatWheel attack_msg" .. x .. y .. z) (possibly with some math.floor or even VFS.PackF32 applied to the numbers). Then, add widget:RecvLuaMsg(msg, playerID) which parses the message and calls MarkerAddPoint locally with the relevant player's ID as the 6?th param (which makes it appear as if that player made the marker) and WG.Translate("attack_msg") as the message. See https://github.com/ZeroK-RTS/Zero-K/blob/master/LuaUI/Widgets/camera_lockcamera.lua for an example. Decoupling the interface from MarkerAddPoint also allows the markers to eventually replaced with something fancy (eg. could be hooked into https://github.com/ZeroK-RTS/Zero-K/blob/master/LuaUI/Widgets/gui_custom_markers.lua )

@sprunk
Copy link
Member

sprunk commented Feb 21, 2020

Related #3104

sprunk
sprunk reviewed Feb 21, 2020
View reviewed changes
LuaUI/Widgets/gui_gesture_comms.lua

local spEcho = Spring.Echo

options_path = 'Settings/Interface/Gesture Menus/Comms Menu'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: comms could be interpreted as commanders

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and I don't think Gesture Menus needs to be part of the path.

@GoogleFrog
Copy link
Contributor

I agree with Sprunk. Another benefit of SendLuaUIMsg is that it would slot easily into the localisation system.

I don't know where the menu should go in hotkeys. Do you have any good suggestions that don't conflict? The gesture menu worked because Licho was personally fine with not having access to custom formations when constructors are selected.

I think the texts are generally too long.

@esainane
Copy link
Contributor Author

Replace Spring.MarkerAddPoint(x, y, z, "Attack here!") with Spring.SendLuaUIMsg("ChatWheel attack_msg" .. x .. y .. z) [...]

Sounds like the moonshots are more achievable than I initially thought! That's still probably going to be shelved for the immediate future, though that should make for a really nice extension.

I don't know where the menu should go in hotkeys. Do you have any good suggestions that don't conflict?

Personally, I've been using BACKQUOTE, since I tend to use \ as the drawing key.

It might be possible to use BACKQUOTE for both, with a short keypress opening the menu, and a held key used for drawing. That might fit with the existing "quickly communicate" semantics. I'm not sure if there's an easy way to accomplish this with the current framework.

I think the texts are generally too long.

I agree. Do you have a more palatable set of options in mind?

@GoogleFrog
Copy link
Contributor

  • I do not want to pull the Spring.MarkerAddPoint version.
  • You could try a backquote tap. All the other backquote uses involve holding the key so it might work.
  • I expect writing texts in the configuration while working on the widget directly would be faster than communicating changes.

sprunk added a commit that referenced this pull request Apr 2, 2020
@sprunk
esainane's bugfixes/cleanup for gesture menu
ebab291 
A safe part of #3802.
@GoogleFrog
Copy link
Contributor

Does it PR work entirely except that it sends engine markers instead of as-yet unimplemented lua markers?

@sprunk
Copy link
Member

sprunk commented Apr 2, 2020

I pulled the first 5 commits that just apply some bugfixes.
I left extracting the API out but am planning to also pull it shortly (bigger change).
I don't plan on touching the markers part yet.

@esainane
Copy link
Contributor Author

esainane commented Apr 2, 2020

@GoogleFrog The PR is in a fairly stable state and is something that I've been using for a while. gui_gesture_comms as a whole is under 100 lines total, and the PerformAction callback only 5, so swapping out the action for something that uses a system more featureful than engine markers should be very straightforward, once such a system is available.

@GoogleFrog
Copy link
Contributor

Is the behaviour of gesture build menu unchanged?

@esainane
Copy link
Contributor Author

esainane commented Apr 3, 2020

There should be two user facing changes:

  • People are able to configure a longer icon distance if they so choose. The default is unchanged.
  • Previously, if the item selected at the end of a gesture was not a hub, the flash tracing the gesture would be off-center. I've fixed this so that the flash always traces to the end. I consider the old behaviour a bug.

@Licho1
Copy link
Member

Licho1 commented Apr 3, 2020

Can there be a bad interaction with gesture build menu?

@GoogleFrog
Copy link
Contributor

Part of this PR is that it makes a generic gesture system that is then used by the communicator and the gesture menu. As far as I know there is no control overlap because the communicator is opened with a hotkey.

@sprunk
Copy link
Member

sprunk commented Apr 3, 2020

People are able to configure a longer icon distance if they so choose. The default is unchanged.

That was among the other fixes and is already in master.

@esainane
Gesture system: Complete refactor
d109ab4 
All potentially common code is split out into api_gesture_menu_factory,
allowing code reuse between widgets.

An example use is provided in the build gesture menu, now moved to
gui_gesture_build.

Also changed:
 - Some variable shadowing removed.
 - Random tripling of segments in path flashing removed.
 - Dead code removed.
@esainane
api_gesture_menu: Fix old issue with menu_flash
c57697a 
If the item selected was not a menu hub, the flash would be off-center.
@esainane
Add gesture communicator
b4d6096 
Communication? In a competitive strategy game? With polite defaults?

This necessarily permits use of the real MarkerAddPoint.
@esainane
Gesture system: Default options
9b8d192
@esainane
api_gesture_menu: Documentation
199a9e1
@esainane
Gesture system: Remove superfluous handler
fc30888
@esainane
api_gesture_menu: API must load first
cff0631
@esainane
gui_gesture_comms: Use VFS.Include
76a695b
@esainane
Gesture system: Unbreak local_options
71e822a 
(oops)
@esainane
Epic Menu: Fix Gesture Build Menu shortcut
0af6317
@GoogleFrog
Copy link
Contributor

There is a standalone ping menu by a high mana BAR guy.

@sprunk
Copy link
Member

sprunk commented Jun 23, 2023

The standalone ping menu seems to be fairly crap though, it's still GL3 and only has one layer.

@GoogleFrog
Copy link
Contributor

Yes, just worth noting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GoogleFrog GoogleFrog GoogleFrog left review comments

@sprunk sprunk sprunk left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@esainane @sprunk @GoogleFrog @Licho1