15
\$\begingroup\$

I made a Guitar Hero III bot for PC. I was able to beat the song "Through the Fire and Flames" on Expert with the bot which can you see here.

If you watched the video, you can see that the bot misses a few notes, which isn't normal for a BOT to do. When you normally see bots playing Guitar Hero, they never miss a single note.

I feel like my code is very flawed and sluggish which hinders my bot's ability to play notes that require fast execution.

I'd like some help optimizing my code so that my bot can match the performance of other bots.

I commented almost all of my code to avoid any confusion but if I left out any information, or you'd like a better explanation of something, please let me know and I'll do my best to answer your question!

using System;
using System.Collections.Generic;
using System.Runtime.InteropServices;
using System.Threading;
using System.Windows.Forms;

namespace Testing_Grounds
{
    public partial class Form1 : Form
    {
        // The text file "Timestamps and Notes.txt" is 3722 lines long.
        //
        //
        // Ignore the line numbers! They are not part of the text file. (e.g., "25:")
        //
        //
        // The numbers to the LEFT of the separator "|" is the timestamp.
        // This is when the note should be played.
        // The time is in ticks.
        //
        //
        // The numbers to the RIGHT of the separator "|" is the note.
        // Each note is 6 digits long containing 1s and 0s.
        // The first 5 numbers in a note show which keys need to be held down to play the note.
        // the LAST number in a note is irrelevant and It's always a 1.
        // 1 means, hold down that key.
        // 0 means, ignore that key.
        // Since this is Guitar Hero, we have 5 buttons that we use to play notes. (Green, Red, Yellow, Blue, and Orange) [G] [R] [Y] [B] [O]
        // So, if we were given the note "101011", then we would hold down the Green, Yellow, and Orange keys. (Remember, we ignore the last number in each note)
        //
        //
        // Here is what the text file "Timestamps and Notes.txt" contains:
        //
        // 1:    0|010001
        // 2:    750000|100001
        // 3:    1510000|001001
        // 4:    2260000|100001
        // 5:    3010000|000101
        // 6:    3770000|100001
        // 7:    4520000|010001
        // 8:    5280000|100001
        // 9:    6030000|001001
        // 10:   6790000|100001
        // 11:   7540000|000101
        // 12:   8290000|100001
        // 13:   9050000|000011
        // 14:   9800000|100001
        // 15:   10560000|000101
        // 16:   11310000|100001
        // 17:   12070000|000011
        // 18:   12820000|100001
        // 19:   13580000|001001
        // 20:   14330000|100001
        // 21:   15080000|000101
        // 22:   15840000|100001
        // 23:   16590000|010001
        // 24:   17350000|100001
        // 25:   18100000|000011
        //
        // ... The full text file would be too long to display!
        //
        // 3698: 4334290000|000101
        // 3699: 4335790000|001001
        // 3700: 4337290000|010001
        // 3701: 4338790000|100001
        // 3702: 4340300000|000101
        // 3703: 4340670000|000011
        // 3704: 4346310000|000011
        // 3705: 4347060000|000101
        // 3706: 4347810000|001001
        // 3707: 4348560000|010001
        // 3708: 4349310000|100001
        // 3709: 4350060000|010001
        // 3710: 4350810000|001001
        // 3711: 4351570000|000101
        // 3712: 4352320000|000101
        // 3713: 4353320000|001001
        // 3714: 4354320000|010001
        // 3715: 4355320000|100001
        // 3716: 4356320000|010001
        // 3717: 4357320000|001001
        // 3718: 4358330000|100001
        // 3719: 4360330000|001001
        // 3720: 4362330000|000101
        // 3721: 4364340000|000101
        // 3722: 4364710000|000011

        private const ScanCodeShort GreenFretKey = ScanCodeShort.KEY_V; // V key
        private const ScanCodeShort RedFretKey = ScanCodeShort.KEY_C; // C key
        private const ScanCodeShort YellowFretKey = ScanCodeShort.KEY_X; // X key
        private const ScanCodeShort BlueFretKey = ScanCodeShort.KEY_Z; // Z key
        private const ScanCodeShort OrangeFretKey = ScanCodeShort.LSHIFT; // Left SHIFT key
        private const ScanCodeShort StarPowerKey = ScanCodeShort.SPACE; // SPACEBAR
        private const ScanCodeShort WhammyKey = ScanCodeShort.KEY_W; // W key
        private const ScanCodeShort StrumDownKey = ScanCodeShort.OEM_2; // '/?' key

        // When button1 is clicked, execute this code.
        private void button1_Click(object sender, EventArgs e)
        {
            // Store all of the lines in the text file "Timestamps and Notes.txt" into the variable named "lines".
            var lines = System.IO.File.ReadAllLines("Timestamps and Notes.txt");

            // Declare a timestamp and a note list which will be used to store all of the timestamps and notes.
            var timestampList = new List<double>();
            var noteList = new List<string>();

            // For each line in the text file "Timestamps and Notes.txt", "extract" the timestamp and note and then add them into their appropriate list.
            foreach (var line in lines)
            {
                var timestamp = line.Substring(0, line.IndexOf("|"));
                // Store everything to LEFT of the separator "|" into the variable named "timestamp".
                var note = line.Substring(line.LastIndexOf("|") + 1);
                // Store everything to RIGHT of the separator "|" into the variable named "note".
                timestampList.Add(Convert.ToDouble(timestamp));
                // We must convert to double because the timestamps get very large because they are in ticks.
                noteList.Add(note);
            }

            // I need to loop through each timestamp and note simultaneously in order to play each note at the proper timestamp.
            // Since I am using a foreach loop below for the timestamps, I also needed a way to cycle through the notes at the same time.
            // This variable will be used to help with that.
            var noteListIndex = 0;

            // button1 needs to be clicked as soon as the first note of the song is ready to be played.
            // This variable stores the time (in ticks) that the Guitar Hero song commenced.
            var startingTime = DateTime.Now.Ticks;

            // The timestampList and noteList will always be the same length so I figured it would be easier just to use a foreach loop.
            foreach (var timestamp in timestampList)
            {
                // This while loop basically means, "While we are waiting for the next note to be played, use the whammy bar."
                while (DateTime.Now.Ticks - startingTime < timestamp)
                {
                    UseWhammy();
                }

                // Store all of the digits in the note that needs to be played into an array so we can then check what keys need to be held down.
                var noteDigits = noteList[noteListIndex].ToCharArray();

                // This list will be used to store all of the keys that need to be held down in order to play the note.
                var nInputs = new List<INPUT>();

                // If the green button needs to be held down to play the note, then add it to the nInputs list.
                if (noteDigits[0].ToString() == "1")
                {
                    nInputs.Add(new INPUT
                    {
                        type = InputType.KEYBOARD,
                        U =
                        {
                            ki =
                            {
                                wScan = GreenFretKey
                            }
                        }
                    });
                }

                // If the red button needs to be held down to play the note, then add it to the nInputs list.
                if (noteDigits[1].ToString() == "1")
                {
                    nInputs.Add(new INPUT
                    {
                        type = InputType.KEYBOARD,
                        U =
                        {
                            ki =
                            {
                                wScan = RedFretKey
                            }
                        }
                    });
                }

                // If the yellow button needs to be held down to play the note, then add it to the nInputs list.
                if (noteDigits[2].ToString() == "1")
                {
                    nInputs.Add(new INPUT
                    {
                        type = InputType.KEYBOARD,
                        U =
                        {
                            ki =
                            {
                                wScan = YellowFretKey
                            }
                        }
                    });
                }

                // If the blue button needs to be held down to play the note, then add it to the nInputs list.
                if (noteDigits[3].ToString() == "1")
                {
                    nInputs.Add(new INPUT
                    {
                        type = InputType.KEYBOARD,
                        U =
                        {
                            ki =
                            {
                                wScan = BlueFretKey
                            }
                        }
                    });
                }

                // If the orange button needs to be held down to play the note, then add it to the nInputs list.
                if (noteDigits[4].ToString() == "1")
                {
                    nInputs.Add(new INPUT
                    {
                        type = InputType.KEYBOARD,
                        U =
                        {
                            ki =
                            {
                                wScan = OrangeFretKey
                            }
                        }
                    });
                }

                // Release all of the buttons BEFORE holding down new buttons.
                RefreshKeys();

                // Send all of the keyboard input! (Which will hold down all of the keys required to play the note)
                SendInput((uint)nInputs.ToArray().Length, nInputs.ToArray(), INPUT.Size);

                // Try using star power! (The bot can't determine if star power is ABLE to be used, so it will simply try to use it before playing each note)
                UseStarPower();

                // Now that all of keys are being held down (because of the SendInput above) it's time to actually "play" the note.
                UseStrum();

                // After each note is played, increment noteListIndex so we don't keep playing the same note over and over again.
                noteListIndex = noteListIndex + 1;
            }
        }

        private void RefreshKeys()
        {
            var nInputs = new[]
            {
                new INPUT
                {
                    type = InputType.KEYBOARD,
                    U =
                    {
                        ki =
                        {
                            wScan = GreenFretKey,
                            dwFlags = KEYEVENTF.KEYUP
                        }
                    }
                },
                new INPUT
                {
                    type = InputType.KEYBOARD,
                    U =
                    {
                        ki =
                        {
                            wScan = RedFretKey,
                            dwFlags = KEYEVENTF.KEYUP
                        }
                    }
                },
                new INPUT
                {
                    type = InputType.KEYBOARD,
                    U =
                    {
                        ki =
                        {
                            wScan = YellowFretKey,
                            dwFlags = KEYEVENTF.KEYUP
                        }
                    }
                },
                new INPUT
                {
                    type = InputType.KEYBOARD,
                    U =
                    {
                        ki =
                        {
                            wScan = BlueFretKey,
                            dwFlags = KEYEVENTF.KEYUP
                        }
                    }
                },
                new INPUT
                {
                    type = InputType.KEYBOARD,
                    U =
                    {
                        ki =
                        {
                            wScan = OrangeFretKey,
                            dwFlags = KEYEVENTF.KEYUP
                        }
                    }
                }
            };

            // Release ALL of the 5 fret keys. (Green, Red, Yellow, Blue, and Orange) [G] [R] [Y] [B] [O]
            SendInput((uint)nInputs.Length, nInputs, INPUT.Size);
        }

        private void UseStarPower()
        {
            var StarPowerKeyUp = new[]
            {
                new INPUT
                {
                    type = InputType.KEYBOARD,
                    U =
                    {
                        ki =
                        {
                            wScan = StarPowerKey,
                            dwFlags = KEYEVENTF.KEYUP
                        }
                    }
                }
            };

            // Release the star power key. (SPACEBAR)
            SendInput((uint)StarPowerKeyUp.Length, StarPowerKeyUp, INPUT.Size);

            // I don't know why, but if I do not include the delay seen below, then it will not release the star power key.
            // If anyone knows the reason why, I'd appreciate your insight.
            Thread.Sleep(1);

            var StarPowerKeyDown = new[]
            {
                new INPUT
                {
                    type = InputType.KEYBOARD,
                    U =
                    {
                        ki =
                        {
                            wScan = StarPowerKey
                        }
                    }
                }
            };

            // Hold down the star power key. (SPACEBAR)
            SendInput((uint)StarPowerKeyDown.Length, StarPowerKeyDown, INPUT.Size);

            // I don't know why, but if I do not include the delay seen below, then it will not hold down the star power key.
            // If anyone knows the reason why, I'd appreciate your insight.
            Thread.Sleep(1);
        }

        private void UseWhammy()
        {
            var WhammyKeyUp = new[]
            {
                new INPUT
                {
                    type = InputType.KEYBOARD,
                    U =
                    {
                        ki =
                        {
                            wScan = WhammyKey,
                            dwFlags = KEYEVENTF.KEYUP
                        }
                    }
                }
            };

            // Release the whammy key. (W key)
            SendInput((uint)WhammyKeyUp.Length, WhammyKeyUp, INPUT.Size);

            // I don't know why, but if I do not include the delay seen below, then it will not release the whammy key.
            // If anyone knows the reason why, I'd appreciate your insight.
            Thread.Sleep(1);

            var WhammyKeyDown = new[]
            {
                new INPUT
                {
                    type = InputType.KEYBOARD,
                    U =
                    {
                        ki =
                        {
                            wScan = WhammyKey
                        }
                    }
                }
            };

            // Hold down the whammy key. (W key)
            SendInput((uint)WhammyKeyDown.Length, WhammyKeyDown, INPUT.Size);

            // I don't know why, but if I do not include the delay seen below, then it will not hold down the whammy key.
            // If anyone knows the reason why, I'd appreciate your insight.
            Thread.Sleep(1);
        }

        private void UseStrum()
        {
            var StrumDownKeyUp = new[]
            {
                new INPUT
                {
                    type = InputType.KEYBOARD,
                    U =
                    {
                        ki =
                        {
                            wScan = StrumDownKey,
                            dwFlags = KEYEVENTF.KEYUP
                        }
                    }
                }
            };

            // Release the strum key. ('/?' key)
            SendInput((uint)StrumDownKeyUp.Length, StrumDownKeyUp, INPUT.Size);

            // I don't know why, but if I do not include the delay seen below, then it will not release the strum key.
            // If anyone knows the reason why, I'd appreciate your insight.
            Thread.Sleep(6);

            var StrumDownKeyDown = new[]
            {
                new INPUT
                {
                    type = InputType.KEYBOARD,
                    U =
                    {
                        ki =
                        {
                            wScan = StrumDownKey
                        }
                    }
                }
            };

            // Hold down the strum key. ('/?' key)
            SendInput((uint)StrumDownKeyDown.Length, StrumDownKeyDown, INPUT.Size);

            // I don't know why, but if I do not include the delay seen below, then it will not hold down the strum key.
            // If anyone knows the reason why, I'd appreciate your insight.
            Thread.Sleep(6);
        }

        public Form1()
        {
            InitializeComponent();
        }

        /// <summary>
        /// Synthesizes keystrokes, mouse motions, and button clicks.
        /// </summary>
        [DllImport("user32.dll")]
        internal static extern uint SendInput(uint nInputs,
            [MarshalAs(UnmanagedType.LPArray), In] INPUT[] pInputs,
            int cbSize);

        [StructLayout(LayoutKind.Sequential)]
        public struct INPUT
        {
            internal InputType type;
            internal InputUnion U;

            internal static int Size
            {
                get { return Marshal.SizeOf(typeof(INPUT)); }
            }
        }

        internal enum InputType : uint
        {
            MOUSE = 0,
            KEYBOARD = 1,
            HARDWARE = 2
        }

        [StructLayout(LayoutKind.Explicit)]
        internal struct InputUnion
        {
            [FieldOffset(0)]
            internal MOUSEINPUT mi;

            [FieldOffset(0)]
            internal KEYBDINPUT ki;

            [FieldOffset(0)]
            internal HARDWAREINPUT hi;
        }

        [StructLayout(LayoutKind.Sequential)]
        internal struct MOUSEINPUT
        {
            internal int dx;
            internal int dy;
            internal int mouseData;
            internal MOUSEEVENTF dwFlags;
            internal uint time;
            internal UIntPtr dwExtraInfo;
        }

        [StructLayout(LayoutKind.Sequential)]
        internal struct KEYBDINPUT
        {
            internal ScanCodeShort wScan;
            internal KEYEVENTF dwFlags;
            internal int time;
            internal UIntPtr dwExtraInfo;
        }

        [StructLayout(LayoutKind.Sequential)]
        internal struct HARDWAREINPUT
        {
            internal int uMsg;
            internal short wParamL;
            internal short wParamH;
        }

        [Flags]
        internal enum MOUSEEVENTF : uint
        {
            ABSOLUTE = 0x8000,
            HWHEEL = 0x01000,
            MOVE = 0x0001,
            MOVE_NOCOALESCE = 0x2000,
            LEFTDOWN = 0x0002,
            LEFTUP = 0x0004,
            RIGHTDOWN = 0x0008,
            RIGHTUP = 0x0010,
            MIDDLEDOWN = 0x0020,
            MIDDLEUP = 0x0040,
            VIRTUALDESK = 0x4000,
            WHEEL = 0x0800,
            XDOWN = 0x0080,
            XUP = 0x0100
        }

        [Flags]
        internal enum KEYEVENTF : uint
        {
            EXTENDEDKEY = 0x0001,
            KEYUP = 0x0002,
            SCANCODE = 0x0008,
            UNICODE = 0x0004
        }

        internal enum ScanCodeShort : short
        {
            LBUTTON = 0,
            RBUTTON = 0,
            CANCEL = 70,
            MBUTTON = 0,
            XBUTTON1 = 0,
            XBUTTON2 = 0,
            BACK = 14,
            TAB = 15,
            CLEAR = 76,
            RETURN = 28,
            SHIFT = 42,
            CONTROL = 29,
            MENU = 56,
            PAUSE = 0,
            CAPITAL = 58,
            KANA = 0,
            HANGUL = 0,
            JUNJA = 0,
            FINAL = 0,
            HANJA = 0,
            KANJI = 0,
            ESCAPE = 1,
            CONVERT = 0,
            NONCONVERT = 0,
            ACCEPT = 0,
            MODECHANGE = 0,
            SPACE = 57,
            PRIOR = 73,
            NEXT = 81,
            END = 79,
            HOME = 71,
            LEFT = 75,
            UP = 72,
            RIGHT = 77,
            DOWN = 80,
            SELECT = 0,
            PRINT = 0,
            EXECUTE = 0,
            SNAPSHOT = 84,
            INSERT = 82,
            DELETE = 83,
            HELP = 99,
            KEY_0 = 11,
            KEY_1 = 2,
            KEY_2 = 3,
            KEY_3 = 4,
            KEY_4 = 5,
            KEY_5 = 6,
            KEY_6 = 7,
            KEY_7 = 8,
            KEY_8 = 9,
            KEY_9 = 10,
            KEY_A = 30,
            KEY_B = 48,
            KEY_C = 46,
            KEY_D = 32,
            KEY_E = 18,
            KEY_F = 33,
            KEY_G = 34,
            KEY_H = 35,
            KEY_I = 23,
            KEY_J = 36,
            KEY_K = 37,
            KEY_L = 38,
            KEY_M = 50,
            KEY_N = 49,
            KEY_O = 24,
            KEY_P = 25,
            KEY_Q = 16,
            KEY_R = 19,
            KEY_S = 31,
            KEY_T = 20,
            KEY_U = 22,
            KEY_V = 47,
            KEY_W = 17,
            KEY_X = 45,
            KEY_Y = 21,
            KEY_Z = 44,
            LWIN = 91,
            RWIN = 92,
            APPS = 93,
            SLEEP = 95,
            NUMPAD0 = 82,
            NUMPAD1 = 79,
            NUMPAD2 = 80,
            NUMPAD3 = 81,
            NUMPAD4 = 75,
            NUMPAD5 = 76,
            NUMPAD6 = 77,
            NUMPAD7 = 71,
            NUMPAD8 = 72,
            NUMPAD9 = 73,
            MULTIPLY = 55,
            ADD = 78,
            SEPARATOR = 0,
            SUBTRACT = 74,
            DECIMAL = 83,
            DIVIDE = 53,
            F1 = 59,
            F2 = 60,
            F3 = 61,
            F4 = 62,
            F5 = 63,
            F6 = 64,
            F7 = 65,
            F8 = 66,
            F9 = 67,
            F10 = 68,
            F11 = 87,
            F12 = 88,
            F13 = 100,
            F14 = 101,
            F15 = 102,
            F16 = 103,
            F17 = 104,
            F18 = 105,
            F19 = 106,
            F20 = 107,
            F21 = 108,
            F22 = 109,
            F23 = 110,
            F24 = 118,
            NUMLOCK = 69,
            SCROLL = 70,
            LSHIFT = 42,
            RSHIFT = 54,
            LCONTROL = 29,
            RCONTROL = 29,
            LMENU = 56,
            RMENU = 56,
            BROWSER_BACK = 106,
            BROWSER_FORWARD = 105,
            BROWSER_REFRESH = 103,
            BROWSER_STOP = 104,
            BROWSER_SEARCH = 101,
            BROWSER_FAVORITES = 102,
            BROWSER_HOME = 50,
            VOLUME_MUTE = 32,
            VOLUME_DOWN = 46,
            VOLUME_UP = 48,
            MEDIA_NEXT_TRACK = 25,
            MEDIA_PREV_TRACK = 16,
            MEDIA_STOP = 36,
            MEDIA_PLAY_PAUSE = 34,
            LAUNCH_MAIL = 108,
            LAUNCH_MEDIA_SELECT = 109,
            LAUNCH_APP1 = 107,
            LAUNCH_APP2 = 33,
            OEM_1 = 39,
            OEM_PLUS = 13,
            OEM_COMMA = 51,
            OEM_MINUS = 12,
            OEM_PERIOD = 52,
            OEM_2 = 53,
            OEM_3 = 41,
            OEM_4 = 26,
            OEM_5 = 43,
            OEM_6 = 27,
            OEM_7 = 40,
            OEM_8 = 0,
            OEM_102 = 86,
            PROCESSKEY = 0,
            PACKET = 0,
            ATTN = 0,
            CRSEL = 0,
            EXSEL = 0,
            EREOF = 93,
            PLAY = 0,
            ZOOM = 98,
            NONAME = 0,
            PA1 = 0,
            OEM_CLEAR = 0,
        }
    }
}
\$\endgroup\$
  • 8
    \$\begingroup\$ I still have nightmares about that song... \$\endgroup\$ – Phrancis Jun 26 '16 at 19:58
  • 1
    \$\begingroup\$ That VirtualKeyShort enum takes up a lot of room, have you considered perhaps using more descriptive names to cut down on some of the documentation? For example MBUTTON could just be self-explaining MIDDLE_MOUSE_BUTTON. A lot of your <summary> comments seem to just tell what button it really is, so a lot of those could just be eliminated by better naming. \$\endgroup\$ – Phrancis Jun 26 '16 at 20:20
  • \$\begingroup\$ @Phrancis I wasn't actually using any of it in my code, so I removed it completely! Thanks for bringing that to my attention. \$\endgroup\$ – Owen Jun 26 '16 at 20:52
7
\$\begingroup\$

That huge comment block at the top should be an actual comment block:

/*
 * The text file "Timestamps and Notes.txt" is 3722 lines long.
 *
 * Ignore the line numbers! They are not part of the text file. (e.g., "25:")
 *
 * ...
 *
*/

This would make the whole block collapsible, so you or anyone looking at the code wouldn't need to scroll down two entire screens worth of comments just to get to the actual code.

These key constants should be an enum:

private const ScanCodeShort GreenFretKey = ScanCodeShort.KEY_V; // V key
private const ScanCodeShort RedFretKey = ScanCodeShort.KEY_C; // C key
private const ScanCodeShort YellowFretKey = ScanCodeShort.KEY_X; // X key
private const ScanCodeShort BlueFretKey = ScanCodeShort.KEY_Z; // Z key
private const ScanCodeShort OrangeFretKey = ScanCodeShort.LSHIFT; // Left SHIFT key
private const ScanCodeShort StarPowerKey = ScanCodeShort.SPACE; // SPACEBAR
private const ScanCodeShort WhammyKey = ScanCodeShort.KEY_W; // W key
private const ScanCodeShort StrumDownKey = ScanCodeShort.OEM_2; // '/?' key
[Flags]
private enum GuitarKeys
{
    GreenFretButton = ScanCodeShort.KEY_V,
    RedFredButton = ScanCodeShort.KEY_C,
    YellowFretButton = ScanCodeShort.KEY_X,
    BlueFretButton = ScanCodeShort.KEY_Z,
    OrangeFretButton = ScanCodeShort.LSHIFT,
    StarPowerButton = ScanCodeShort.SPACE,
    WhammyBarButton = ScanCodeShort.KEY_W,
    StrumButton = ScanCodeShort.OEM_2 // '?' key
}

Notice the only comment that's actually useful is the last one. The [Flags] attribute indicates that the values can be combined and that you can use bitwise logic and the .HasFlag method to determine the combination of values a GuitarKeys represent; by using a set of constants like you did, you lose this ability and need to resort to conditionals and logical operators instead.

// When button1 is clicked, execute this code.
private void button1_Click(object sender, EventArgs e)
{
    // Store all of the lines in the text file "Timestamps and Notes.txt" into the variable named "lines".
    var lines = System.IO.File.ReadAllLines("Timestamps and Notes.txt");

The comments are overkill. Bad comments state the obvious and tell nothing the code doesn't already say. Good comments say why, and let the code say what. If mere implementation details such as the name of the text file needs to change, you made yourself two places to maintain it in: in the actual code, and in the comment above it. Everywhere. There's so many comments everywhere, it's distracting, and makes the code actually harder to follow because of the constant context-switching.


Your noteDigits have pre-determined and specific significance, that you're referring to with magic numbers - one has to resort to reading the comments to figure out what's what. Make another enum:

private enum NotePosition
{
    Green,
    Red,
    Yellow,
    Blue,
    Orange
}

And then map them to actual keys:

private static readonly IDictionary<NotePosition,GuitarKeys> NoteButtonMap =
    new Dictionary<NotePosition,GuitarKeys>
{
    { NotePosition.Green, GuitarKeys.GreenFretButton },
    { NotePosition.Red, GuitarKeys.RedFretButton },
    { NotePosition.Yellow, GuitarKeys.YellowFretButton },
    { NotePosition.Blue, GuitarKeys.BlueFretButton },
    { NotePosition.Orange, GuitarKeys.OrangeFretButton }
};

And now you can take this repeated chunk:

if (noteDigits[0].ToString() == "1")
{
    nInputs.Add(new INPUT
    {
        type = InputType.KEYBOARD,
        U =
        {
            ki =
            {
                wScan = GreenFretKey
            }
        }
    });
}

And extract it into its own function, as Matt suggested:

private void AddInput(char[] notes, NotePosition notePosition, IList<INPUT> inputs)
{
    if (notes[notePosition] != '1') { return; }

    inputs.Add(new INPUT
    {
        type = InputType.KEYBOARD,
        U =
        {
            ki =
            {
                wScan = NoteButtonMap[notePosition]
            }
        }
    });
}

Being a char[], each index in the notes array corresponds to a char. Why waste cycles in the conversion to a String when you can compare it to a char?

About this sleep:

Thread.Sleep(6);

That 6ms is probably below the clock resolution (MSDN):

The system clock ticks at a specific rate called the clock resolution. The actual timeout might not be exactly the specified timeout, because the specified timeout will be adjusted to coincide with clock ticks.

Per this SO answer, the thread time slice on Windows is 15ms, so it's not clear what that arbitrary 6 really does.

Same here:

Thread.Sleep(1);

You definitely need a function that returns an INPUT[], and since that function will have pre-determined outputs you could reduce the object creation/destruction overhead by keeping an INPUT[] instance around somewhere in static context.


The DllImport declarations should be in a NativeMethods static class, not in the form's code-behind.

Actually... none of this code belongs in the form's code-behind - even less so in a button's Click handler. The only way you have of testing any of your code, is by F5-debugging it and clicking that button.

You can't even give it arbitrary inputs and test any of it without loading the file that's specified there.

You need to separate the concerns of reading the input vs. processing it and producing the output. By properly separating the concerns, you will be able to run your logic in a tight loop, profile it, and fine-tune its performance.

But to achieve this, you need to stop making the UI run the show. The entire form's code-behind could then look something like this:

public partial class Form1 : Form
{
    private readonly IGuitarHero _hero;

    // parameterless constructor for designer only
    public Form1()
    {
        InitializeComponent();
    }

    public Form1(IGuitarHero hero)
        : this()
    {
        _hero = hero;
    }

    private void button1_Click(object sender, EventArgs e)
    {
        _hero.Play(fileNameBox.Text);
    }
}
\$\endgroup\$
  • \$\begingroup\$ Thank you very much for the detailed response! I'm not sure exactly how I'm supposed to use AddInput, could you please explain? These are the 2 errors inside of the AddInput function: "Cannot implicitly convert type 'Testing_Grounds.Form1.GuitarKeys' to 'Testing_Grounds.Form1.ScanCodeShort'. An explicit conversion exists (are you missing a cast?)" "Cannot implicitly convert type 'Testing_Grounds.Form1.NotePosition' to 'int'. An explicit conversion exists (are you missing a cast?)" \$\endgroup\$ – Owen Jun 28 '16 at 4:35
  • \$\begingroup\$ The enum's underlying type is int by default (i.e. Int32), try making it Int16 instead (Int16 is a "short" integer), enum GuitarKeys : Int16 or enum GuitarKeys : short - and yeah, the other one will need a cast. sorry about that. \$\endgroup\$ – Mathieu Guindon Jun 28 '16 at 4:38
  • \$\begingroup\$ I am using a cast on the other one and that fixed 1 error. I tried your suggestion and I'm still getting this error: "Cannot implicitly convert type 'Testing_Grounds.Form1.NotePosition' to 'int'. An explicit conversion exists (are you missing a cast?)" Should I just use a cast to solve this error as well? \$\endgroup\$ – Owen Jun 28 '16 at 4:58
  • \$\begingroup\$ Yep. Any enum can be cast to an int, but they didn't make the conversion implicit for some reason. \$\endgroup\$ – Mathieu Guindon Jun 28 '16 at 5:11
  • \$\begingroup\$ I'm still not exactly sure how to go about using the AddInput function to see what keys need to be pressed for each note in the song. :l \$\endgroup\$ – Owen Jun 28 '16 at 5:15
6
\$\begingroup\$
  1. The code blocks if (noteDigits[4].ToString() == "1") look like the could be refactored into a function. Also why are you comparing strings? Wouldn't comparing numbers or anding flags be quicker?
  2. In RefreshKeys() you are always going to send the same input, so why not make it a static so you don't need to recreate it every time. You could exploit the new function from point 1, but I'm not sure if that will improve performance.
  3. In the UseStarPower() does Thread.Sleep(0) work? If so it might be some issue with your thread needing to yield to another thread. Sleep(0) will be a little bit faster too.
  4. The functions UseWhammy(), UseStrum() and UseStarPower() are virtually identical. I don't know how the optimiser for C# makes its decisions so I don't know if combining them will make it faster, but it will make it easier to read.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.