milkytracker / MilkyTracker

An FT2 compatible music tracker
http://milkytracker.github.io/
Other
1.67k stars 160 forks source link

windows "keyrepeat" retriggers note inputs (MilkyTracker X build) #304

Open meowgli opened 1 year ago

meowgli commented 1 year ago

on windows, when holding down a key assigned to note input, the sample will re-trigger constantly.

"key repeat delay" can be extended, but slows down arrow key movement, (and still does not solve samples retriggering, just gives you a little more time). The nuclear option, "filter keys - repeat keys off" solves this and allows keyboard playing of notes for the full duration of a sample, however completely nerfs arrow key movement/selection, holding backspace to delete text)

microsofts documentation related to repeat keys: "KeyDown event occurs when the user presses any key. The KeyUp event occurs when the user releases the key. Duplicate KeyDown events occur each time the key repeats, if the key is held down, but only one KeyUp event is generated when the user releases the key." https://learn.microsoft.com/en-us/dotnet/api/system.windows.forms.keyeventargs?redirectedfrom=MSDN&view=windowsdesktop-7.0#remarks How reasonable would it be to filter keydowns not paired with a keyup specifically for note inputs?

SyntaxErrol commented 1 year ago

IMO only as a toggleable setting. Otherwise it would be too drastic a change to expected behavior. I wonder, if how many .mod trackers worked (prior to instruments and keyoff behavior introduced in FT2) would be preferable to you where the sample playback didn't stop on keyup. Admittedly this option isn't available on Milky either but can sort of be simulated with the right instrument settings.

meowgli commented 1 year ago

IMO only as a toggleable setting. Otherwise it would be too drastic a change to expected behavior. I wonder, if how many .mod trackers worked (prior to instruments and keyoff behavior introduced in FT2) would be preferable to you where the sample playback didn't stop on keyup. Admittedly this option isn't available on Milky either but can sort of be simulated with the right instrument settings.

That's fair. Absolutely no pressure to force it upon the whole base. Personally I feel It's expected behavior to listen to a sample through until I no longer hold a note key. (Keyup functionality is still really important!)

https://user-images.githubusercontent.com/12127517/215731067-99e05a50-3bc1-4bfe-b471-1f463870f916.mp4

it's an interesting effect, but not particularly musical when trying to jam with samples.

coderofsalvation commented 1 year ago

Hi @meowgli , nice to meet you and thanks for reporting this bug (and using milkytracker). This 'bug' is fixed here https://github.com/coderofsalvation/MilkyTrackerX/tree/bugfix/sustain-keyjazz-note-instead-of-retriggering. Without any toggle/setting though, because at that time:

  1. this partticular fix was able to filter out keyrepeats after they arrived at the pattern-editor (so the expected workflow remains, but preventing rapid-firing-audio-events).
  2. I could not come up for a usecase to want rapid-firing-audio-events
  3. it looked like the code was accidentally commented out

You can try it out in the MilkyTrackerX build. There's some other experimental live-jammable features in there too.

ps. you might want to remove/backup your ~/.config/milkytracker path to enjoy the MilkyTrackerX defaults

meowgli commented 1 year ago

Hi @meowgli , nice to meet you and thanks for reporting this bug (and using milkytracker). This 'bug' is fixed here https://github.com/coderofsalvation/MilkyTrackerX/tree/bugfix/sustain-keyjazz-note-instead-of-retriggering. Without any toggle/setting though, because at that time:

1. this partticular fix was able to filter out keyrepeats **after** they arrived at the pattern-editor (so the expected workflow remains, but preventing rapid-firing-audio-events).

2. I could not come up for a usecase to want rapid-firing-audio-events

3. it looked like the code was accidentally commented out

You can try it out in the MilkyTrackerX build. There's some other experimental live-jammable features in there too.

ps. you might want to remove/backup your ~/.config/milkytracker path to enjoy the MilkyTrackerX defaults I've spent my evening getting a brute forced method setting windows key repeat filtering, then simulating key repeat for arrows/del/ins/bkspc :P it's good news to hear.

Hey! I believe I am on the latest MilkyTrackerX build in that video. Am I missing something? Other thing I'm probably missing is ASIO output, but I'll look around for a bit for that one before yelping.

Ok, mostly cleared up: after building with that commit, I'm still getting retriggers when recording is enabled and transport is not going. Seems fine when playing without record, or record with playback. Small enough issue that I can learn to avoid it.

Thanks for your hard work on milkytrackerX

meowgli commented 1 year ago

A little Follow-up - I got this working how I would roughly expect the key repeat filtering to work. (Expanding the scope of releasePlay's effect, and tailoring a couple of the state filters previously sensitive to isLiveRecording) not sure if it's helpful at all, and I'm sure I broke something else in the process, but it seems to work without machinegun key events lol

RecorderLogic.cpp line 87

    `bool record = (tracker.editMode == EditModeMilkyTracker ? tracker.screen->hasFocus(patternEditorControl) : recordMode);
    bool releasePlay = false;
    bool isLiveRecording = playerController->isPlaying() && 
                           !playerController->isPlayingRowOnly() &&
                           record && 
                           tracker.shouldFollowSong();
    // separated out releaseplay filtering from the currently in use isLiveRecording to break as few things as possible while filtering note key repeats from the OS
    // 
    // Key repeat suppression is enabled when we are looking at a note column (wider suppression than previously done with isLiveRecording)
    releasePlay = patternEditorControl->getCursorPosInner() == 0;

    if (releasePlay)
    {
        // Somewhere editing of text in an edit field takes place,
        // in that case we don't want to be playing anything
        if (tracker.isActiveEditing())
            return;

        // take a look if this key is already pressed
        bool isPressed = false;
        for (i = 0; i < TrackerConfig::MAXNOTES; i++)
        {
            if (keys[i].note == note)
            {
                isPressed = true;
                break;
            }
        }
        // this key is already pressed, we won't play that note again
        if (isPressed)
        {
            // If we're recording, this event should not be routed anywhere else
            // it terminates HERE! (event shall not be routed to the pattern editor)
            if (!playerController->isPlayingRowOnly() && record && event)
                event->cancel();
            return;
        }

        // if we're not recording, cycle through the channels
        // use jam-channels for playing if requested, 
        // use jam channels when recording/demoing notes under playback when not following song. 
        if (!(playerController->isPlaying() && record) || !tracker.shouldFollowSong())
        {
            chn = playerController->getNextPlayingChannel(chn);
        }
        else
        {
            // Get next recording channel: The base for selection of the 
            // next channel is the current channel within the pattern editor
            chn = playerController->getNextRecordingChannel(chn);
        }
    }
    else
    {
        // The cursor in the pattern editor must be................... `
coderofsalvation commented 1 year ago

@meowgli what OS are you testing on? windows? (your play/stop button seems a bit funny) I've tried to reproduce your notransport+record machinegun on Linux, but without succes. Nevertheless, your solution compiles/works fine here, so I think I'll just include it in the milkytracker X build for now .

coderofsalvation commented 1 year ago

thank you very much for adressing this, it has been fixed in v1.04 🎉

coderofsalvation commented 11 months ago
mr_lou> Isn't it possible to hold down a key anymore, in order to fill up a whole track with the same note?
coderofsalvation> how about in the next version, we allow key-repeat when edit-mode is enabled?

ps. meowgli's fiox is now the default. Apoligies for overlooking the settings-thing (the whole release was quite intense in other areas).

WDYT of the suggestion of key-repeat in case of edit-mode? Best of both worlds?

meowgli commented 11 months ago

@coderofsalvation

WDYT of the suggestion of key-repeat in case of edit-mode? Best of both worlds?

I use hold in edit mode to hear my inputs, especially for pads and notes with an attack etc. A bit of heartbreak if this is reverted.

consider: a) A toggle setting with a hotkey to enable key repeat filtering or not, allows old behavior permanently or just temporarily

b) A modifier key - slightly less attractive, who wants to be holding another key while doing note input; though if the user intends to fill a whole column it's likely one note and they have another hand available...

b2) when a note is held, holding an arrow key down/up will fill the column as if it was repeating. currently it moves the cursor but does not fill more notes.

c) there's an option that I really can't use personally, but would be reasonable for most users: only filter in edit mode when Add lines is 0. I rely on hearing the notes play out even with +add lines on though :/

Whatever the solution is, an attempt to keep current edit mode filtering as an option would be wonderful.

coderofsalvation commented 11 months ago

OK, so this turned a bit into a catch22 :) What if we just introduce a keyrepeat toggle in settings (disabled by default?) I'm leaning towards that, eventhough it's not an easy trade-off. However, I guess keyrepeat is a nice-to-have (like a rapid fire-toggle on a joystick) not a must-have (like consistent audio playback of a sample).