Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add Iterators and non-destructive updates for binding hotkeys. #3

Merged
merged 53 commits into from
Jul 12, 2019

Conversation

twestura
Copy link
Collaborator

Additional Hotkey Features

This adds new features to the hotkey file parsing library.

Iterators

Add iterators for both HotkeyGroup and HotkeyInfo. These structs can now be iterated over either by using the num_hotkeys or num_groups methods and a range, directly using a for loop thanks to IntoIter, or by calling the iter method to obtain an iterator over the internal vector.

Previously a hotkey for standard AoC could be iterated by using the enum to grab each of the predefined hotkey groups, then using that group's enum to obtain its hotkeys. But an arbitrary hotkey file can contain an arbitrary number of groups, each with an arbitrary number of hotkeys, this adds support for processing these files as well.

Display/ToString

Structs now have a display method that writes the string id and key codes.

A future feature would be to take in a language file and write the output using the strings from that file. The string_ids describing the commands are included in the hotkey file and can be read easily from the language file. But the keycodes still need to be translated into string_ids in order to obtain their string representations from the language file.

Bind/Unbind

Add bind/unbind methods to the HotkeyGroup and HotkeyInfo structs. These methods allow making a clone of the structs while updating a hotkey, so the original struct is unchanged. With the old API a client would need to clone the struct and then mutate the clone.

The interface design for mutating these structs can still use some work. In particular whenever the hotkey id is passed using one of the enum variants, as usize must be specified. The group ids work out be having two methods with different types, but there isn't currently a type to unify the hotkey ids across their various hotkey groups.

Various other bits

Hotkeys and HotkeyGroups now derive PartialEq and Eq. These traits make the structs easier to use in unit tests.
Hotkeys themselves now also derive Copy.
io::Result is not used explicitly instead of being imported to allow for using the standard Result type.

Copy link
Member

@goto-bus-stop goto-bus-stop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sick, lgtm thus far. are you planning on doing the key name translation in this PR? I'm ok doing it now or later, hki was specifically <1.0.0 because it was unfinished to the point of being barely usable. we have a lot of 0.n.m versioning room to go before needing a stable interface 🐱

crates/genie-hki/src/lib.rs Outdated Show resolved Hide resolved
@twestura
Copy link
Collaborator Author

sick, lgtm thus far. are you planning on doing the key name translation in this PR? I'm ok doing it now or later, hki was specifically <1.0.0 because it was unfinished to the point of being barely usable. we have a lot of 0.n.m versioning room to go before needing a stable interface 🐱

That's the next thing I was going to work on, might as well do it in this PR as well. Most of the ids should be pretty straightforward to figure out from the key-value file.

@goto-bus-stop goto-bus-stop mentioned this pull request May 17, 2019
3 tasks
@goto-bus-stop
Copy link
Member

@twestura I'd like to get this in as is if you don't mind, then the remaining things you were planning on doing in this PR can be landed separately later.

@goto-bus-stop goto-bus-stop merged commit f9904b2 into SiegeEngineers:master Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants