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

Implement focus groups #7

Open
tomrijnbeek opened this issue Jun 30, 2019 · 6 comments
Open

Implement focus groups #7

tomrijnbeek opened this issue Jun 30, 2019 · 6 comments
Labels
enhancement New feature or request

Comments

@tomrijnbeek
Copy link
Member

For example, a menu you can navigate through with up/down arrows.

My first idea on how to implement this is to make a FocusGroup control. When instantiating the focus group control, you define what keys/buttons move focus from one control to the next. Within the focus group, the order in which controls are added will define the order in which the elements are focused.

Nested focus groups should also be trivially supported. Since focus is something that is propagated along an entire path in the control tree, a focus group will only update the focus of its children when it itself is focused. If there is a focus group inside another one with different keys to move focus, this will work without much problem. As long as the inner focus group is focused inside the outer focus group, the inner focus group can respond to those focus buttons. Roughly (pseudo-code):

public void override KeyHit(Key k) {
  if (!IsFocused) return;
  if (nextFocusKeys.Contains(k)) moveFocusForward();
  if (prevFocusKeys.Contains(k)) moveFocusBackward();
}

If there is a focus group within a focus group that uses the same keys as an ancestor focus group, this can be supported easily as well. A focus group will mark an event as handled if it has moved focus to the next element. When trying to move the focus forward without there being a control left, the focus group could do one of two things:

  1. It doesn't handle the event, making the event propagate upwards in the control tree, and the ancestor focus group will receive the event instead. We let the focus escape the focus group.
  2. It handles the focus by cycling the focus back to the first element. The ancestor never receives the key event. The control captures the focus.

I think with these rules, focus within a group should be relatively easy thing to implement, and also appears to cover the major use cases. I also think using a builder pattern, we can make this very descriptive and easy to use. For example:

  FocusGroup.Builder()
    .AddForwardKey(Key.Down)
    .AddForwardKey(Key.S)
    .AddBackwardKey(Key.Up)
    .AddBackwardKey(Key.W)
    .AddForwardButton(Button.Down)
    .AddBackwardButton(Button.Up)
    .Build();

This would create a focus group where up and down on they arrow keys, WASD keys, and gamepad would navigate forward and backwards. To make sure the focus cycles back to the top, another .MakeCyclic() call could be added.

I looked at the Roche Fusion main menu screen to check if it is possible to implement that using the focus group rules I outlined above. The screen looks like this:

image

By using the left and right actions, you can switch between the three main controls on this screen:

image

Within both the menu on the right and the vertical highscore navigation, the up and down actions navigate through the options. However, to switch between "easy" and "normal", you use the same directions as you would switch between the high level groups. Once you move "right" from the "normal" option though, it switches back to the vertical highscore controls.

All in all, it seems this implementation will make it easy to implement menus, and make it possible to build in keyboard and gamepad support in menus with ease.

@paulcscharf
Copy link
Member

Focus groups are just... composite components, right? With some properties that determine how the focus moves.

I don’t like the idea that the UI system itself knows what keys move focus back/forth and what don’t, that seems very odd and a pain if you wanna rewind things, or whatever. Why not just have that at the top level and propagate a next/previous focus event? And if you need several ‘groups’ like in RF, implement that specifically for that situation, or add directional controls of some sort. I think that’s what you want for game pads and maybe arrow keys anyways, be able to navigate directionally.

I’m just not sure I’m buying that the approach above will really make things easy, since you have to write a lot of code to make sure focus in your situation works as expected, and if you make changes you have to change that too, or it’ll start behaving strange.

@tomrijnbeek
Copy link
Member Author

Thanks. Good to see a very different approach. Personally, I am not convinced that managing focus at the top-level works. Here are a few concerns I have with that (and sorry for the wall of text):

  • There are different top-level "screens" in which the focusing behaviour may be completely different. To take Roche Fusion as an example again: the main menu (disregarding the left boxes for simplicity) uses up-down arrows to move focus forwards and backwards. The options menu on the other hand uses left-right arrows to move focus forwards and backwards, because it uses a horizontal menu, and then within a category uses up-down for the individual setting. How do you fit that in a single top-level next/previous focus event? Does the screen tell the top-level focus manager when the controls for changing focus are? In that case, why doesn't the screen manage focus itself?
  • By making the focus system simple, every control that is not a simple list needs to implement custom code. I feel that is somewhat missing the point of this library, and will lead to lots of "dumb" duplicated code just to listen to all the right actions and cater the behaviour to a specific layout.
  • How does focus propagate from one control to the next, especially if the controls don't live on the same layer in the control tree? For example, what if you have two separate vertical menus below each other with a gap? You would have a root control, with two children, each having several children as well. Do we do a depth-first traversal and focus each control that "IsFocusable" in turn? That immediately falls apart if one of the buttons is instead three buttons horizontally, because then somewhere you would expect the controls to switch to left-right, and it all becomes very complicated to handle from a single place.
  • A single top-level focus system can't really deal with focus within focus. Let's imagine an item in a list of settings where you can switch between 3 settings. When you have this setting focused, you can use the orthogonal focus controls to switch the settings. There is a focus system within a focus system going on. Without making it possible to have its own little focus system, I don't see that working with a centralised event, and we would have to do custom programming for the inner control. Maybe that is fine if we explicitly choose for a lightweight system. However... this means controls can only assume they can reliably use the focus system if they are the the highest level control in the control tree that is claiming the use of this focus event. Problem is, controls are - and should be - agnostic of where they are in the component tree, meaning they could never be sure the focus works reliably.

Please let me know if I interpreted your idea wrong and/or whether these items are not actually issues.

Also to respond to some parts of your comment specifically:

Focus groups are just... composite components, right? With some properties that determine how the focus moves.

Yes, that is correct.

I don’t like the idea that the UI system itself knows what keys move focus back/forth and what don’t, that seems very odd and a pain if you wanna rewind things, or whatever.

I am not sure why this is a problem at all. Actually, I think this is actually desired behaviour, since a control (both how it looks and how it behaves) should be completely self-contained. How focus works within a control is part of that, I believe. I don't see how that should be any different from a text input knowing what button keys to respond to for adding symbols to its value, or even a button knowing it needs to respond to a left mouse button hit only.

I think that’s what you want for game pads and maybe arrow keys anyways, be able to navigate directionally.

I agree that this is not necessarily a solved problem in my proposed solution. There may still be some custom logic needed, but providing a framework that does most of the work for you should still make that easier (if it doesn't, the framework is badly designed).

I’m just not sure I’m buying that the approach above will really make things easy, since you have to write a lot of code to make sure focus in your situation works as expected, and if you make changes you have to change that too, or it’ll start behaving strange.

I am not sure understand this section entirely, but as to the "write a lot of code" part: I don't think it's too badly. We already keep track of focus, so the only thing a focus group has to do is listen to events and move focus to the next or previous child. All the complexities I described above make full use of the already existing framework of events and focus, so I believe by just implementing the FocusGroup control, we have the vast majority of the described functionality down.

@paulcscharf
Copy link
Member

It’s hard to imagine how exactly it is going to look once it’s done, but I think we might be less at odds than I thought.

One difference seems to be whether each control has code to check for navigation key presses, or whether that is done at the top level and instead of keys presses you pass down ‘MoveFocusLeft/Right/Up/Down’ events. I think the latter would be easier for ensuring that navigation works with the same keys everywhere, and not having to possibly perform as many checks while propagating the key press event (which would still happen I figure).

These move focus events (or a single one with a direction parameter) would be handled by ‘focus groups’ just like the keys presses would be handled in your approach. What bothers me about your approach is that the ‘left/right/up/down/whatever’ thing is only a thing because of the keys/buttons you pass in, otherwise there is only previous/next which makes a 2d layout require column/row nesting.

The other big difference is that I prefer the directional approach, where instead of moving ‘just’ within the focus group, it searches along the hierarchy, if there is something in the chosen direction that can be focussed. I don’t see any issues with this really, though it might be tricky to implement.

In the case where you have a list of controls, one of which has sub-focussable things in a different direction, moving down that list would focus the bigger control, which would then have a few lines of code that would focus the right child (based on it status), and then the focus is in that sub system, where you can move it, say, left and right, but if you want to go up/down there won’t be local children to move to, so it’ll search in the hierarchy for the next thing in that direction.

I think that’s really not so different from your approach, expect that you sort of define 100% what navigation is possible in code, while this here would be implicit and based on location. In both cases, on navigation, it performs a check if there is a sibling that can be focussed in the given direction, if yes, that’s focussed, maybe even one of its children, if not, you go up in the tree and see what the next thing is where we can move focus (you call that thing a focus group), and try the same thing there.

One argument against my approach my be that it could more easily conflict with other key-handling, though maybe it would only do the focus moving if the key event came back unhandled in the first place (which also leaves room for custom handling, where needed). In fact... given this, our two approaches could even be combined. Or a ‘directional focus mover’ could just be another composite control, and you get different behaviours where you want them?

I’m not exactly sure what the best approach is, but these are my thoughts right now.

@tomrijnbeek
Copy link
Member Author

I understand where you are coming from about having the individual groups deal with what keys control them. I agree that having the keys consistent across the app is in many cases desirable, though I believe it is the app's responsibility to make that so. I also agree that having support for more than just forward/backward sounds nice, but it feels like we're just moving the problem. What if there is a third dimension we have to move along? This may sound silly, but I am thinking of a screen where you can navigate both horizontally and vertically, but there are also multiple tabs. Think of an inventory screen such as Breath of the Wild for example, which actually has a hierarchy of tabs:

It is a bit of a shame that navigating along the tabs can't use the focus group system if that system is limited to up/down/left/right and always linked to the same keys. I am not opposed to supporting four-directional focus changes, but I would then just make that a separate FocusGrid component that works in a very similar manner to FocusGroup (which should maybe become FocusList then).

One thing I like of having the global MoveFocusX events is that there is only a single place where you have to deal with the problem of what happens once keys/buttons are rebound or a controller (re/dis)connected. That being said, I don't like the limitations that puts on where you can use the focus system.

Then regarding the directional search approach. It sounds cool, but also complicated to implement. Apart from having some sort of axis-aligned ray-box intersection that needs to implemented, I think there are a lot of cases where the behaviour is not strongly defined. It sounds like this has the potential of being a rabbit hole of different situations cropping up that don't quite behave as expected, needing continuous changes to how the logic works.

I don't think directional approaches are what is always wanted. In the case of a single list-like menu it is probably a bit overkill to search the control tree. The solution also doesn't really capture all use cases, for example if a part of the control wants to "capture" the focus (e.g. a dialog), and even if you do build in those changes, it feels like we're once again diving into that rabbit hole of making this solution quite complex.

Personally, I think the explicit approach is the better option. While this puts some burden on the client implementing the focus, it has the advantage that it is not only a much easier system to implement, it is also easier to understand. Since everything is explicitly defined, there is a much lower chance of the system behaving unexpectedly and/or the implementation having to deal with an edge case.

As you rightfully mentioned, there is no reason why both systems couldn't exist next to each other. I am not opposed to having both in the library, though we should avoid having many different ways to achieve the same thing. I like your suggestion of supporting at least 4-directional movement, and would propose to create both a 2- and 4-directional focus movement control.

Appreciate your thoughts. You have convinced me that there may not be a one-size-fits-all solution here.

@tomrijnbeek
Copy link
Member Author

I tried making a quick proof of concept here (see beardgame/td@654bf44) to show how I envision the control working. There is definitely a lot that could be improved. In particular, the control doesn't skip over unfocusable controls yet, and we could probably improve performance with some additional bookkeeping.

There are two problems I have run into:

  1. I am not sure how to make this work with two dimensions, as there is no easy way to figure out what control is actually there without asking the control creator to supply a method that can do that lookup.
  2. This is probably the biggest concern: focus doesn't actually work the way I expected. A single control is focused, and that focus does not propagate upward in the control tree. So as soon as the FocusList loses focus by giving it to its child, it can't be controlled any more. I believe this is something we should fix, since if any control is focused, all its ancestors in the controls are in focus as well.

@paulcscharf
Copy link
Member

Your example of more-than-2d movement above might have convinced me. There is a time and place for what I had in mind, but it’s not a solution that fits everywhere. We were trying to solve different problems, and I don’t want to block you from moving forward with your ideas here.

I don’t have time to look into the issues you found right now, but get the gist of it. Worth discussing perhaps.

Couple of thoughts that I had:

  • what you’re essentially implementing is tab-order, except injectable keys, maybe it’s worth checking our other up frameworks (winforms/wpf, which we’re sort of mirroring to some degree?) implement this
  • limiting navigation to back/forth seems.. maybe as limited as limiting to two dimensions... but a more general approach seems even worse.. maybe?
  • you probably shouldn’t inject keys, you should inject Actions taken from an input manager, which can then include rebindable actions which makes reminding trivial

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants