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

Transition.EndNavigation not always triggered #586

Closed
oomek opened this issue Dec 2, 2019 · 10 comments
Closed

Transition.EndNavigation not always triggered #586

oomek opened this issue Dec 2, 2019 · 10 comments

Comments

@oomek
Copy link
Collaborator

@oomek oomek commented Dec 2, 2019

Let assume we want to remap the navigation buttons for a certain layout without remapping default controls in settings. When we intercept all the signals up down left right and call next_game and prev_game we have the following output in the log:

Command intercepted by script handler: up
Command intercepted by script handler: prev_game
[Transition] type=EndNavigation, var=0
Command intercepted by script handler: down
Command intercepted by script handler: next_game
[Transition] type=EndNavigation, var=0
Command intercepted by script handler: left
Command intercepted by script handler: prev_game
Command intercepted by script handler: right
Command intercepted by script handler: next_game

As you can see the EndNavigation transition is only triggered for buttons that are mapped in Controls as next_game and prev_game

Here is a minimalistic code that is showing the problem

function on_signal( sig )
{
	switch ( sig )
	{
		case "left":
			fe.signal( "prev_game" )
			return true

		case "right":
			fe.signal( "next_game" )
			return true

		case "up":
			fe.signal( "prev_game" )
			return true

		case "down":
			fe.signal( "next_game" )
			return true

		case "prev_game":
		case "next_game":
			return true

		default:
			return false
	}
}
fe.add_signal_handler( "on_signal" )
@oomek

This comment has been minimized.

Copy link
Collaborator Author

@oomek oomek commented Dec 3, 2019

The same goes for changing the fe.list.index

[Transition] type=ToNewSelection, var=-1
[Transition] type=FromOldSelection, var=1
[Transition] type=EndNavigation, var=0
Command intercepted by script handler: up
[Transition] type=EndNavigation, var=0

[Transition] type=ToNewSelection, var=1
[Transition] type=FromOldSelection, var=-1
[Transition] type=EndNavigation, var=0
Command intercepted by script handler: down
[Transition] type=EndNavigation, var=0

[Transition] type=ToNewSelection, var=-1
[Transition] type=FromOldSelection, var=1
[Transition] type=EndNavigation, var=0
Command intercepted by script handler: left

[Transition] type=ToNewSelection, var=1
[Transition] type=FromOldSelection, var=-1
[Transition] type=EndNavigation, var=0
Command intercepted by script handler: right

For up and down the EndNavigation transition is called after each fe.list.index change and then when the button is released

For left and right it's called only after fe.list.index change

function on_signal( sig )
{
	switch ( sig )
	{
		case "left":
			fe.list.index--
			return true

		case "right":
			fe.list.index++
			return true

		case "up":
			fe.list.index--
			return true

		case "down":
			fe.list.index++
			return true

		case "prev_game":
		case "next_game":
			return true

		default:
			return false
	}
}
fe.add_signal_handler( "on_signal" )

I don't know yet what would be the best way around it. Maybe a new Squirrel function that would allow to remap the default signals for the directional controls ( those in brackets )?

@oomek

This comment has been minimized.

Copy link
Collaborator Author

@oomek oomek commented Dec 3, 2019

Conclusions:

Current implementation only allows vertical layouts to benefit from EndNavigation transition. If you have mixed layouts, some vertical and some horizontal, remapping signals in controls is not an option. Additionally some end-users are unaware of that neccessity.

Shouldn't the EndNavigation transition be called only after releasing the key after changing the value of fe.list.index ?

@oomek

This comment has been minimized.

Copy link
Collaborator Author

@oomek oomek commented Dec 3, 2019

fe.remap_control( "left", "prev_game" )
fe.remap_control( "right", "next_game" )
fe.remap_control( "up", "prev_filter" )
fe.remap_control( "down", "next_filter" )

Just a rough idea. It would be temporary if the layout sets it.

@oomek

This comment has been minimized.

Copy link
Collaborator Author

@oomek oomek commented Dec 4, 2019

I've opened a new branch in my fork with a new function I've added fe.set_action()
oomek@70e9d0f

remap_control was a bit misleading name.
I'm not opening a PR yet as we are still testing it under all scenarios.

@mickelson

This comment has been minimized.

Copy link
Owner

@mickelson mickelson commented Dec 5, 2019

I think I've got a fix for this

mickelson added a commit that referenced this issue Dec 5, 2019
- frontend should now consistently trigger Transition.EndNavigation
  when navigating using fe.signal()
@oomek

This comment has been minimized.

Copy link
Collaborator Author

@oomek oomek commented Dec 5, 2019

Not neccessarily, if you just suppress the signal without calling next_game the EndNavigation transition now fires up every time.
Besides this does not help for horizontal layouts. Have you checked my branch yet?

@oomek

This comment has been minimized.

Copy link
Collaborator Author

@oomek oomek commented Dec 5, 2019

With my commit we can take advantage of the navigation acceleration without remapping actions every time we change the layout.

@keilmillerjr

This comment has been minimized.

Copy link
Contributor

@keilmillerjr keilmillerjr commented Dec 5, 2019

🤦‍♂️

mickelson added a commit that referenced this issue Dec 7, 2019
- corrected error with last commit so EndNavigation transition isn't
  triggered when a script signal handler catches and stops the
  navigation
- smarter handling of key repeats when a script has a signal handler
  that is remapping navigation actions
@oomek

This comment has been minimized.

Copy link
Collaborator Author

@oomek oomek commented Dec 7, 2019

Hats off to your commit a45bff8

It actually fixed the underlying problem. I have to admit that my commit was a workaround as I've decided to not to mess with the main loop too much.

By adding just:

case "left":
    fe.signal( "prev_game" )
    return true

case "right":
    fe.signal( "next_game" )
    return true

we have now proper transitions calling and acceleration.
Closing the issue. Thanks Andrew.

@oomek oomek closed this Dec 7, 2019
@mickelson

This comment has been minimized.

Copy link
Owner

@mickelson mickelson commented Dec 7, 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.