Skins sets 'cue_set' ControlPushButton as a toggle button

The Outline & Collusion skin family uses <NumberStates>2</NumberStates> for "play/pause" buttons among others.

It works also with <NumberStates>1</NumberStates> like in the Deere,Latenight,Shade, Phoney skin families.
Would be interesting to know if you have the same midi problem with one of these.

Mhh not sure where you see that it uses <NumberStates>1</NumberStates> in those skin for the play/cue_set button.

I'm currently using the trunk branch, modifying any of the skins to use 1 instead of 2 for that button leads to a mixxx crash in when I start it.

If I set Shade to use 1 as NumberStates then the play button no longer changes state when you left click it. I can confirm that cue_set is broken because of this. Right-clicking the GUI button only works while the deck is playing. The Shift+D keyboard shortcut seems to work no matter what, although it causes the GUI Play button to become confused about its state and un-highlight even though the deck is still playing.

I think the only time we ever have 2 states and a RightClick option is the Play button. For 1.8.0 purposes, I think adding a simple workaround to WPushButton is in order. The workaround would treat any RightClick bound connection as a pushbutton regardless of NumberStates.

What do you guys think about that? Jus do you use 2 states and a RightClick for anything other than the play buttons in your skins?

This is an ugly workaround. Please don't go that way.

Why not drop <NumberStates> completely and set the toggling properties of the control itself in the code ?
I don't think the skins should do anything to the controls, the controls should have a pre-defined behavior.

Unfortunately, the NumberStates property is essential for picking which pixmap to render on the button. There's no way the widget can determine the number of states a control has just by looking at it. For example, the End of Track mode has 3 states. The widget has no way of knowing that except from the skin.xml definition.

There is actually a separate bug here which I hinted at earlier. The right-click connection on the play button causes the state of cue_set to affect the state of the button, while the play/pause button is only meant to reflect the state of the play/pause control it is connected to. The right-click is more of a handy extra feature of the button and isn't meant to contribute to its state.

To fix this issue, I've tested a flag for the <Connection> node in skin.xml, <TwoWay> which allows you to specify a connection as write only, i.e. the state of the connected control will not be fed back into the widget, it will only be set by the widget. The skin.xml connection for cue_set would go like this:

                <Connection>
   <ConfigKey>[Channel1],cue_set</ConfigKey>
   <EmitOnDownPress>true</EmitOnDownPress>
   <ButtonState>RightButton</ButtonState>
                        <TwoWay>false</TwoWay>
  </Connection>

If not provided, it defaults to true.

OK I think I have a general workaround which is not (too) ugly.

<Connection>
  <ConfigKey>[Channel1],cue_set</ConfigKey>
  <EmitOnDownPress>true</EmitOnDownPress>
  <ButtonState>RightButton</ButtonState>
  <ConnectValueToWidget>false</ConnectValueToWidget> <!-- fixes play/pause button state bug -->
  <ConnectValueFromWidget>true</ConnectValueFromWidget> <!-- not required since it defaults true -->
  <PushButton>true</PushButton> <-- used to force WPushButton to treat this connection like a pushbutton -->
</Connection>

It will be an error to connect two controls to RightButton (or LeftButton) with different PushButton values.

I've been talking about GUI issues, but the cause of the MIDI problems stems from WPushButton setting the ControlPushButton::setToggleButton(true) which changes the way the control handles values coming from MIDI devices (as Guy mentions in the bug description).

My workaround in #6 will allow WPushButton to check for the <PushButton> child of <Connection> and not set that ControlPushButton as a toggle button.

<quote #3>Jus do you use 2 states and a RightClick for anything other than the play buttons in your skins?</quote>

Using it for Play/EQ/Headphone/Flanger/Kill buttons .

Dunno if this is needed since these skins grown from a simple copypasta over the time and it may even wrong in the original source.
The <numberstate> & <EmitOnDownPress> stuff is really nasty (have not understand it to full extend yet :-) , there are most likely more errors in the skins (e.g. hotcues have <EmitOnDownPress> true and false at the same time).

Jus:

I meant using 2 NumberStates AND also having a RightButton connection. In Shade 1024x768 for example, I only see that on the Play/Pause button.

Let me try to explain the EmitOnDownPress a little bit. The reason you have to have two connections, one with EmitOnDownPress as true and one with false for the hotcues is that when there is a connection with it true, it will set the control to 1 when you hit the button, when it is false, it will set the control to 0 when you release the button.

Some parts of Mixxx require that the control first be set to 1 when the button is pressed, and then it be set to 0 when the button is depressed. That is the reason why some connections need to be doubled with EmitOnDownPress true and false.

It's very silly that the skin.xml mappings work this way, but it's been that way since before I joined the project :).

cue_set is actually supposed to have a double connection with EmitOnDownPress true and false as well (all connections are supposed to have this) but cueSet does not do anything when you depress the button so it is fine. The hotcue buttons need to do something when you depress the button because of hotcue previewing. If you are holding down the button then the cue is previewing, and when you depress the button, the preview stops and it jumps back to the hotcue. I hope that made it more clear :).

OK, I've added ConnectValueToWidget and ConnectValueFromWidget to <Connection> group in the XML. This is in the 1.8 branch now.

I added another option to the <PushButton> group, LeftClickIsPushButton and RightClickIsPushButton. To fix cue_set you now have to do the following: http://pastebin.com/y8Mx5wJx

The jist of it is, adding <RightClickIsPushButton> forces all connections made to the right-click handler to not be set to be a toggle button, they are handled as push buttons. They are set to 1 on a press and a 0 on a depress, regardless of the state of the widget.

Adding <ConnectValueToWidget> as false to the cue_set connection ensures that the Play toggle button's state is not changed when someone hits cue_set, as I could do in the 1.8 branch by playing a deck and hitting Shift+D to set the cue point (which resulted in the play button un-highlighting even though the deck was still playing).

The code fix is there and Jus is in the process of applying the fix to the skins, so I'm marking this fixed.

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/5513