Composed binds: the fix broke +toggle binds; some improvements #1827

Dune-jr · 2018-12-10T13:01:12Z

bind shift+s +toggle cl_dynamic_camera

no longer works after c4fbceb

Also, I believe we could allow for bind shift+s +fire if nothing is bound to s, only make sure that s binds that start with + are always triggered:

teeworlds/src/game/client/components/binds.cpp

Lines 138 to 140 in 899f9f0

    
           for(int m = 0; m < MODIFIER_COUNT; m++) 
        
           { 
        
           	if(((Mask&(1 << m)) || (m == 0 && m_aaaKeyBindings[Event.m_Key][0][0] == '+')) && m_aaaKeyBindings[Event.m_Key][m][0] != 0)	// always trigger +xxx binds despite any modifier

The text was updated successfully, but these errors were encountered:

Dune-jr · 2018-12-10T13:11:46Z

I think we should simply return true if we find ourselves executing a + bind in there:

 for(int m = 0; m < MODIFIER_COUNT; m++) 
 { 
 	if(((Mask&(1 << m)) && m_aaaKeyBindings[Event.m_Key][m][0] != 0)	// always trigger +xxx binds despite any modifier
        {
               if(m == 0 && m_aaaKeyBindings[Event.m_Key][0][0] == '+')
                     return true;
               // ...

instead of doing

teeworlds/src/game/client/components/binds.cpp

Lines 39 to 41 in c4fbceb

    
           // skip modifiers for +xxx binds 
        
           if(pStr[0] == '+') 
        
           	Modifier = 0;

Am I oversighting anything?

oy · 2018-12-10T20:02:22Z

You're not supposed to use composed binds for +xxx binds.
What you proposed doesn't work. Wouldn't trigger the if statement cause it will always check for the correct modifier. Behaves like before the patch i added.

Dune-jr · 2018-12-11T10:34:57Z

You're not supposed to use composed binds for +xxx binds.

Well then, I guess that closes the issue. Otherwise:

What you proposed doesn't work. Wouldn't trigger the if statement cause it will always check for the correct modifier. Behaves like before the patch i added.

My bad, it should stop after triggering a no-modifer +xxx bind, like this:

 for(int m = 0; m < MODIFIER_COUNT; m++) 
 { 
        if(m == 0 && m_aaaKeyBindings[Event.m_Key][0][0] == '+')
        {
                if(Event.m_Flags&IInput::FLAG_PRESS)
			Console()->ExecuteLineStroked(1, m_aaaKeyBindings[Event.m_Key][m]);
		if(Event.m_Flags&IInput::FLAG_RELEASE)
			Console()->ExecuteLineStroked(0, m_aaaKeyBindings[Event.m_Key][m]);
               return true; // always stop after triggering a +xxx bind
        }
  	if(((Mask&(1 << m)) && m_aaaKeyBindings[Event.m_Key][m][0] != 0)	
        {
               // ...

oy · 2018-12-12T19:24:43Z

Reopened the issue - will improve it for the next version.

oy · 2019-01-10T20:50:01Z

How about removing the restriction that pressing a modifier and a bind key don't trigger a bind that isn't bound by a modifier?
F.e.: pressing shift+s would trigger "bind shift+s" and "bind s"

Cause for modifiers there currently is no such restriction:
pressing alt+shift+s triggers "bind alt+s" and "bind shift+s".

Dune-jr · 2019-01-11T11:27:05Z

How about removing the restriction that pressing a modifier and a bind key don't trigger a bind that isn't bound by a modifier?
F.e.: pressing shift+s would trigger "bind shift+s" and "bind s"

That doesn't sound very good to me. In that case, all keys that are mapped to basic actions (chat, movements, firing, scoreboard, ...) cannot be used in composed binds.

In any game I know, binding Ctrl+X does not trigger X, that is not intuitive. What is the benefit?

pressing alt+shift+s triggers "bind alt+s" and "bind shift+s".

Yeah, that is not great. That comes from the fact that triple binds like alt+shift+s are not supported - if they were, alt+shift+s shouldn't trigger alt+s imo.

oy · 2019-01-11T15:34:44Z

How about removing the restriction that pressing a modifier and a bind key don't trigger a bind that isn't bound by a modifier?
F.e.: pressing shift+s would trigger "bind shift+s" and "bind s"

That doesn't sound very good to me. In that case, all keys that are mapped to basic actions (chat, movements, firing, scoreboard, ...) cannot be used in composed binds.

You could use basic actions with composed binds. There's no restriction regarding this if it would trigger non and composed binds.
Right now you can't use composed binds for these actions - they get converted to non-composed ones automatically.

Dune-jr · 2019-01-12T11:21:32Z

The code snippet I proposed earlier would fix this by allowing to use composed binds for these actions.

You could have some odd case such as:

bind shift+x +left
bind x say hi

and it should work properly, without breaking the "you can use composed binds on already single-bound keys" concept.

Am I missing something?

oy · 2019-01-12T12:17:03Z

But composed binds wouldn't trigger, even "+" ones, when there's a non-composed "+" one for that key.
People might think that's a bug.

Dune-jr · 2019-02-19T18:40:13Z

Note: In the controls menus, binding e.g. shift+a to "Left" briefly prints shift+a before reverting to a, it looks a bit buggy.

I have also encountered two people who believed bind shift+a +left not working to be a bug.

oy · 2019-02-21T07:55:46Z

Should probably gather all possible bugs/down sides and pick the smallest problem.

Dune-jr · 2019-03-25T18:05:28Z

If we don't restrict anything (0.7.1-rc before `c4fbceb`)

Bug: shift+k triggers k binds. This is bad but might be solvable in another way
Bug: if you do bind shift+k +left, and you release shift, you don't stop going left iirc. This is bad but might be solvable

The current approach (0.7.2, strictly no + for composed binds)

	// skip modifiers for +xxx binds
	if(pStr[0] == '+')
		Modifier = 0;

teeworlds/src/game/client/components/binds.cpp

Lines 138 to 148 in 899f9f0

    
           for(int m = 0; m < MODIFIER_COUNT; m++) 
        
           { 
        
           	if(((Mask&(1 << m)) || (m == 0 && m_aaaKeyBindings[Event.m_Key][0][0] == '+')) && m_aaaKeyBindings[Event.m_Key][m][0] != 0)	// always trigger +xxx binds despite any modifier 
        
           	{ 
        
           		if(Event.m_Flags&IInput::FLAG_PRESS) 
        
           			Console()->ExecuteLineStroked(1, m_aaaKeyBindings[Event.m_Key][m]); 
        
           		if(Event.m_Flags&IInput::FLAG_RELEASE) 
        
           			Console()->ExecuteLineStroked(0, m_aaaKeyBindings[Event.m_Key][m]); 
        
           		rtn = true; 
        
           	} 
        
           }

No bugs
Problem: bind shift+d +toggle cl_dynamic_camera will never work

My proposal (#1827 (comment))

 for(int m = 0; m < MODIFIER_COUNT; m++) 
 { 
        if(m == 0 && m_aaaKeyBindings[Event.m_Key][0][0] == '+')
        {
                if(Event.m_Flags&IInput::FLAG_PRESS)
			Console()->ExecuteLineStroked(1, m_aaaKeyBindings[Event.m_Key][m]);
		if(Event.m_Flags&IInput::FLAG_RELEASE)
			Console()->ExecuteLineStroked(0, m_aaaKeyBindings[Event.m_Key][m]);
                return true; // always stop after triggering a +xxx bind
        }
  	if(((Mask&(1 << m)) && m_aaaKeyBindings[Event.m_Key][m][0] != 0)	
        {
 		if(Event.m_Flags&IInput::FLAG_PRESS) 
 			Console()->ExecuteLineStroked(1, m_aaaKeyBindings[Event.m_Key][m]); 
 		if(Event.m_Flags&IInput::FLAG_RELEASE) 
 			Console()->ExecuteLineStroked(0, m_aaaKeyBindings[Event.m_Key][m]); 
 		rtn = true; 
        }
}

If there is a non-composed + bind, stop and execute only that.
- bind x +left; bind shift+x say hi, then pressing shift+x will only trigger +left
- bind x +left; bind shift+x +right, then pressing shift+x will only trigger +left
- bind shift+d +toggle cl_dynamic_camera works
No bugs afaik?
- Need to check that bind shift+x +left does not leave the tee constantly moving left after releasing shift (or x) though

Oy's proposal

How about removing the restriction that pressing a modifier and a bind key don't trigger a bind that isn't bound by a modifier?
F.e.: pressing shift+s would trigger "bind shift+s" and "bind s"

Cause for modifiers there currently is no such restriction:
pressing alt+shift+s triggers "bind alt+s" and "bind shift+s".

The issue I have with this is that composed binds currently expand the range of keys that can be bound, and this kills it. Example: I have 1 2 3 4 5 bound on weapons. I would like to use shift+1 for team 1, shift+1 for team 2, shift+3 for team -1, shift+4 for disconnect.

so

I may be missing something, need to complete.
And I'm also biased towards my proposal as I don't really see downsides to it?

oy · 2019-03-25T18:25:35Z

The downside is that the composed binds won't trigger like in your examples. Feels like a bug.
To improve that (bind x +left; bind shift+x say hi, then pressing shift+x will only trigger +left), how about:

when pressing x check if shift-key is already down
and there's no non-composed bind for shift
then execute the composed bind

Dune-jr · 2019-03-25T18:35:41Z

Ah, I forgot. There is a reason why non-composed + binds must always be executed regardless of modifiers:

if you hold down a to move left and briefly press shift/ctrl the tee should not stop.

This was an issue in 0.7.1-rc.

Another problem:

if shift+q is not a modifier, it would be nice to still trigger lshift (emotes by default)

The downside is that the composed binds won't trigger like in your examples. Feels like a bug.
To improve that (bind x +left; bind shift+x say hi, then pressing shift+x will only trigger +left), how about:

when pressing x check if shift-key is already down
and there's no non-composed bind for shift
then execute the composed bind

Doesn't that mean that bind lshift +emote disables ALL shift+x binds?
Edit: well I guess it's not so bad

I'm trying to make a summary/checklist:

"bind x +left" and "bind shift+x say hi": 
    x MUST DO "+left" # standard stuff

"bind x +left" and "bind shift+x say hi": 
    shift+x BETTER DO "say hi" #but must not interrupt x if x is held down!

"bind shift+q +left":
    release shift MUST NOT DO "+left"

"bind shift+k say yes" and "bind k say no": 
    shift+k MUST DO "say yes"
    shift+k MUST NOT DO "say no"

"bind shift+d +toggle cl_dynamic_camera": 
    shift+d BETTER DO "+toggle cl_dynamic_camera"

"bind q +left" and "unbind shift+q" and "bind lshift emote": 
    shift+q BETTER DO "+left; emote"

"bind lshift +emote" and "bind shift+x say hi": ?

…er key pressed #1827

Dune-jr closed this as completed Dec 11, 2018

oy reopened this Dec 12, 2018

oy added this to the 0.7.2 milestone Dec 12, 2018

oy modified the milestones: 0.7.2, 0.7.3 Dec 24, 2018

Dune-jr mentioned this issue Jan 7, 2019

Issues management Dune-jr/teeworlds#42

Closed

oy added code-related input labels Jan 8, 2019

oy added a commit that referenced this issue Mar 31, 2019

improved composed binds #1827

0e81be4

oy added a commit that referenced this issue Mar 31, 2019

release composed binds when releasing the main key despite the modifi…

afadbfd

…er key pressed #1827

oy closed this as completed Mar 31, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Composed binds: the fix broke +toggle binds; some improvements #1827

Composed binds: the fix broke +toggle binds; some improvements #1827

Dune-jr commented Dec 10, 2018 •

edited

Loading

Dune-jr commented Dec 10, 2018 •

edited

Loading

oy commented Dec 10, 2018

Dune-jr commented Dec 11, 2018 •

edited

Loading

oy commented Dec 12, 2018

oy commented Jan 10, 2019

Dune-jr commented Jan 11, 2019

oy commented Jan 11, 2019

Dune-jr commented Jan 12, 2019 •

edited

Loading

oy commented Jan 12, 2019

Dune-jr commented Feb 19, 2019 •

edited

Loading

oy commented Feb 21, 2019

Dune-jr commented Mar 25, 2019

oy commented Mar 25, 2019

Dune-jr commented Mar 25, 2019 •

edited

Loading

Composed binds: the fix broke +toggle binds; some improvements #1827

Composed binds: the fix broke +toggle binds; some improvements #1827

Comments

Dune-jr commented Dec 10, 2018 • edited Loading

Dune-jr commented Dec 10, 2018 • edited Loading

oy commented Dec 10, 2018

Dune-jr commented Dec 11, 2018 • edited Loading

oy commented Dec 12, 2018

oy commented Jan 10, 2019

Dune-jr commented Jan 11, 2019

oy commented Jan 11, 2019

Dune-jr commented Jan 12, 2019 • edited Loading

oy commented Jan 12, 2019

Dune-jr commented Feb 19, 2019 • edited Loading

oy commented Feb 21, 2019

Dune-jr commented Mar 25, 2019

If we don't restrict anything (0.7.1-rc before c4fbceb)

The current approach (0.7.2, strictly no + for composed binds)

My proposal (#1827 (comment))

Oy's proposal

so

oy commented Mar 25, 2019

Dune-jr commented Mar 25, 2019 • edited Loading

Dune-jr commented Dec 10, 2018 •

edited

Loading

Dune-jr commented Dec 10, 2018 •

edited

Loading

Dune-jr commented Dec 11, 2018 •

edited

Loading

Dune-jr commented Jan 12, 2019 •

edited

Loading

Dune-jr commented Feb 19, 2019 •

edited

Loading

If we don't restrict anything (0.7.1-rc before `c4fbceb`)

Dune-jr commented Mar 25, 2019 •

edited

Loading