Shortcut config overhaul

[LordMZTE]

As previously discussed in #9, the current system for click actions isn't all that flexible, and there is still room for improvement by allowing one mouse button to run multiple actions. I think the best way to implement this would be to replace the shortcut config with this kind of data structure:

shortcuts: ShortcutsConfig (
    // support old format
    notification_closeall: 3,

    // new format; actions will be executed sequentially
    1: [
        notification_action1,
        notification_close,
    ],
)

as I mentioned in #9. Should we wait on this for the next release since this might break existing configs, or should we allow for backwards compatibility like I suggested in #9?

[Toqozz]

I'm open to improving the shortcuts config, but like I said in #9, I don't see a reason to overhaul this just yet. I think it should be a breaking change to make it correctly, but I feel like there may be other uses we want for shortcuts that we don't know about yet.

I don't think this is a critical issue but I'll have more of a think on how it should look and get back to you.

Are you interested in contributing for this?

[LordMZTE]

Yes I am! I agree that this isn't important, I just think that it would be nice to consider for a future version.

[Toqozz]

So I think we should definitely turn actions into an enum, which is probably how it should've been from the start:

pub enum Shorcut {
    Interact,
    Close,
    CloseAll,
    Action(u8),
}

Then do as you say (name changes explained later):

pub struct ShortcutConfig(Vec<(u16, Vec<Shortcut>)>); 

Which is kind of ugly, but should be workable. Hate the amount of brackets we need here:

mouse_shortcuts: ShortcutConfig(
    [
        (1, [NotificationInteract, NotificationClose]),
    ]
)

I realize also that this is basically what you suggested in #9 . :)

---

As far as handling the breaking change, I think I want to support the existing method for a while to be a bit more pleasant for users:

struct Config {
    ...
    shortcuts: Option<ShortcutsConfig>,    // old shortcuts config
    mouse_shortcuts: Option<ShortcutConfig>  // new shortcut config
}

Then we can check if they're still using the old version and send a deprecation notification or something. We can still remove the old shortcuts code, it's just for the warning basically.

[LordMZTE]

I agree, but why use a Vec<u16, Vec<Shortcut>)> and not a HashMap<u16, Vec<Shortcut>> does ron not support maps? If it does, I think that would be nicer.

[Toqozz]

Sure, it can be a map. I just default to using Vec unless there's a ton of elements.

You actually prodded me to investigate this a bit more. With my very unprofessional benchmarking, the test bellow shows HashMap and Vec meeting in performance around ~50 elements on average:

use std::collections::HashMap;

const SIZE: usize = 50;
const ITERATIONS: usize = 100_000;

fn main() {
    let mut map = HashMap::new();
    for i in 0..SIZE {
        map.insert(i, "something".to_owned());
    }

    let mut arr = Vec::new();
    for i in 0..SIZE {
        arr.push((i, "something".to_owned()));
    }

    use std::time::Instant;
    {
        let now = Instant::now();
        for _ in 0..ITERATIONS {
            test_vec(&arr);
        }
        let elapsed = now.elapsed();
        println!("Vec: {:.2?}", elapsed);
    }

    {
        let now = Instant::now();
        for _ in 0..ITERATIONS {
            test_map(&map);
        }
        let elapsed = now.elapsed();
        println!("Map: {:.2?}", elapsed);
    }
}

fn test_vec(arr: &Vec<(usize, String)>) -> Option<String> {
    for val in arr {
        if &val.0 == &(SIZE / 2) {
            return Some(val.1.clone());
        }
    }

    return None;
}

fn test_map(map: &HashMap<usize, String>) -> Option<String> {
    map.get(&(SIZE - 1)).cloned()
}

0 | ~/code/benchmark --> cargo run --release
    Finished release [optimized] target(s) in 0.01s
     Running `target\release\benchmark.exe`
Vec: 4.78ms
Map: 4.82ms

There's a lot of other factors to consider as well though. Like if the key or value is larger or smaller then it affects cache locality etc.

Up to you which you want to use here.