WiimoteEmu: Fix Drum Extension Velocity

[sanjay900]

I did some tests by sending MIDI commands directly to a GHWT kit, and figured out that a lot of the "random" bits were actually just part of the velocity, and that there is actually a full 7 bits of velocity here, not just 3.

It turns out these kits actually just outright send MIDI data raw over the extension port, the MIDI velocity is just 0x7f - midi_velocity and the MIDI note is just 0x7f - velocity_id. The drums will even happily forward any MIDI note and velocity, not just the ones the pads are hardcoded with.

[JMC47]

It looks like there's a bad commit in there? Unless there's meant to be two commits of the same name?

[sanjay900]

Yeah the linter failed on the first one so I pushed another commit to fix that, I assumed it was just gonna get flattened anyways

[JosJuice]

You'll have to flatten it yourself and force push it.

[sanjay900]

Yep I can do that later, was there anything else you saw that needed changes?

[JosJuice]

The way you're using a union is technically undefined behavior. Is there a particular reason why you want to access buttons separately from the bit-packed struct?

If relevant, Dolphin has a helper class called BitField: https://github.com/dolphin-emu/dolphin/blob/master/Source/Core/Common/BitField.h

The PR looks reasonable to me other than that.

[sanjay900]

Yeah I can just make the buttons part of the bit field and get rid of that, I was just copying what was done in https://github.com/dolphin-emu/dolphin/blob/master/Source/Core/Core/HW/WiimoteEmu/Extension/Turntable.h

Though we still need access to buttons so we can flip all the bits that aren't being used by the table.

[JosJuice]

@jordan-woyak Do you have an opinion?

[sanjay900]

I have flattened it now

[jordan-woyak]

@jordan-woyak Do you have an opinion?

Yeah, probably don't copy the union in Turntable.h. It's promoting undefined behavior and should probably be changed. I'll try to review the other code soon though.

[sanjay900]

What part of this is undefined behaviour exactly? If its the union surrounding a u8 and a struct, im noticing that behaviour on Classic and Nunchuk too

[jordan-woyak]

What part of this is undefined behaviour exactly? If its the union surrounding a u8 and a struct, im noticing that behaviour on Classic and Nunchuk too

Writing to one member of a union and reading from another isn't defined in C++, unless the types are the same, but the rules are maybe a bit more complicated than that (common initial sequence). The use of a union for type punning is something that can be legal in C, but not C++.

[sanjay900]

I have swapped it for BitField now.