First C program: records keyboard/mouse and simulates the recording afterwards

Question

This is my first program ever made in C. Before that I programmed just a little bit in C++ (didn't touch OOP so I did only structural programming). This is my second "project" ever made, the first "project" was even bigger joke. I am new to programming.

The project is nowhere near to be finished. However, it somehow works, and I don't want to add new features until this mess is cleaned up. I feel like for me the bigger the mess is, the harder it is to find small mistakes.

That's why I come here: would you please be able to take a look at my code and tell me what's wrong with it? Especially program structure-wise.

Program features:

• after execution immedietely starts recording cursor's movements and cursor/keyboard keystrokes
• if 'W' is pressed the recording stops
• and 5 seconds later the recording is played-back, simulating what has been recorded

Basically during the recording phase it snapshots the keyboard and mouse state every ~10 miliseconds.

Few things to mention:

• doubly linked list is not freed yet. I will handle the memory leaks.
• the program does not and probably will not support simulating multiple keypresses at the same time
• I will make it more user-friendly and such after I fix this mess

Here is link to the github containing all the code: https://github.com/Wenox/WinAuto

For example, I wonder if such function:

POINT get_cursor(void)
{
    POINT P = {};
    GetCursorPos(&P);
    return P;
}

deserves its own input_cursor.c file?

The main thing I am curious about is the recording.c file containing:

#include ..bunch of files..

#define _GETCURSOR 1
#define _GETKEY 2
#define _SLEEP 3

/** adds cursor's position to the functions queue */
void add_cursor(struct f_queue **head, struct f_queue **tail, POINT P[2])
{
    P[1] = get_cursor();
    if (P[0].x != P[1].x || P[0].y != P[1].y) {               // if current cursor pos != previous
        add_function(head, tail, _GETCURSOR, P[1].x, P[1].y); // add it to the queue
        P[0] = P[1];
    }
}

/** adds latest keystroke's description to the functions queue */
void add_keystroke(struct f_queue **head, struct f_queue **tail, int key_buff[2])
{
    key_buff[1] = get_keystroke();
    if (key_buff[1] != key_buff[0] && key_buff[1] != 0)     // if there was keystroke
        add_function(head, tail, _GETKEY, key_buff[1], -1); // then add it to the queue
    key_buff[0] = key_buff[1];
}

/** returns true if newly added node is function of sleep type, false otherwise */
bool is_prev_sleep_func(struct f_queue **head)
{
    return (*head)->f_type == _SLEEP;
}

/** adds sleep to functions queue */
void add_sleep(struct f_queue **head, struct f_queue **tail, const int sleep_dur)
{
    Sleep(sleep_dur);
    if (!is_prev_sleep_func(head))
        add_function(head, tail, _SLEEP, sleep_dur, -1);
    else
        (*head)->f_args[0] += sleep_dur; // increment the node rather than add new one
}

/** keyboard/mouse recording engine */
void record(struct f_queue **head, struct f_queue **tail, const int sleep_dur)
{
    int key_buff[2] = {-1, -1};               // buffer for curr and prev pressed key
    POINT cursor_buff[2] = {0};               // buffer for curr and prev cursor position

    while(key_buff[1] != KEY_W) {             // stop recording when '3' is pressed
        add_cursor(head, tail, cursor_buff);
        add_keystroke(head, tail, key_buff);
        add_sleep(head, tail, sleep_dur);
    }
}

and the play_recording() function from replay.c file:

/** function replays the recording */
void play_recording(struct f_queue *tail)
{
    while (tail) {
        if (tail->f_type == _GETCURSOR)
            SetCursorPos(tail->f_args[0], tail->f_args[1]);
        else if (tail->f_type == _GETKEY)
            send_input(tail->f_args[0]);
        else if (tail->f_type == _SLEEP)
            Sleep(tail->f_args[0]);

        tail = tail->prev;
    }
}

if it looks neat/readable/understandable to a person who reads it for the first time, and if something is clearly wrong/could be improved/is a bad practice.

For all suggestions/advices on any kind of improvements I am gladly thankful. Many thanks.

Answer 1

I took a look at your github and I think your code is broken up a bit too much. I think a few files can be consolidated. For instance, consider consolidating the files for; input_cursor.c, pressed_key.c into one file called "user_input.c" and do the same for the .h files.

I would also avoid making files named after reserved words in C. like "structures.h". Putting all of your structures in one place is fine if they all serve the same purpose, but name the file after the thing's purpose, not the thing.

I see you're not including std.io until the testing functions are declared in the file. This won't affect performance or compile time. It only distracts the programmer. so regardless of the purpose of the include or which functions will use it, put them all at the top. Now, if you're planning on deleting your test code after release then you really should put all your test functions in their own files in a different directory labeled "test" and define a makefile that can take in a test flag so that it knows to compile the test objects together with the rest of your code.

I also saw a variable declared in a c-code file called keys_pqueue[25] = {}, this is a configuration, and should go in a header file. leave the c-code files for functional code and the headers for all your different kinds of configurations and namespace management.

That's the only feedback I have for now. Your code looks very well thought-out. And C/C++ are hard languages to begin coding with. So it's all very well done!

Answer 2

Initial position

add_cursor() only adds the cursor's position to the function's queue if there is a change.

Depending on needs, I'd expect the initial mouse position to be added too and not assume it is {0,0}. Presently, the true initial mouse position is never saved.

Suggest:

POINT cursor_buff[2] = { get_cursor(), { 0,0}};
add_function(head, tail, _GETCURSOR, cursor_buff[0].x, cursor_buff[0].y);

while(key_buff[1] != KEY_W) {
  ....

---

Backwards?

play_recording() looks like tail = tail->prev; is playing back in reverse order. I'd expect re-play to start at the beginning and play use ->next.

Need to see add_function().