Refactor Input system - Config 1` of N

[fmatthew5876]

This PR refactors the Input subsystem.

• Preparations for being able to reconfigure input mappings
• Optimize speed and memory usage

[fmatthew5876]

I used these commits quite a lot while testing #2035.

Retested some games using this PR. I also retested #1346 and 8 way movement still works fine.
This is ready

[Ghabry]

The "naive" implementation would use std::(unordered_)map -> std::vector but your data structure seems to be a flat vector containing the keys. That's okay, but I'm still annoyed that this needs this unreadable std::lower_bound everywhere because C++ has no useful std::binary_search -_-

What is from an input point of view: A domain type a value type? Maybe use better words here that fit to a button mapping?

Can't you implemant ReplaceAll via RemoveAll followed by Add? Imo the ReplaceAll function is too complex. Has multiple code paths depending on src/dst size and micro-optimisations are not useful here.

[fmatthew5876]

The "naive" implementation would use std::(unordered_)map -> std::vector but your data structure seems to be a flat vector containing the keys. That's okay, but I'm still annoyed that this needs this unreadable std::lower_bound everywhere because C++ has no useful std::binary_search -_-

lower_bound is the C++ binary search, it just has a different name. std::map has very poor memory locality and also memory usage overhead, especially when your object type is just a pair of integers. This implementation is highly optimized for the common case (check input keys each frame) at the expense of the rare case (initialize and remap the input keys).

What is from an input point of view: A domain type a value type? Maybe use better words here that fit to a button mapping?

Apologies. This was pretty manageable when I did the first pass and it was just built on the ButtonMapping class and wasn't a template.

As soon as I added DirectionMapping, I realized it was the same case with different semantics. You want a different type because you don't want to be able to assign or copy a ButtonMapping to a DirectionMapping etc.. But doing that necessitates templating the InputMappingArray class. As soon as I did that I was forced to use iterators, generic algorithms, and add a few typedefs and then code readability went to hell fast.

In C++ value_type is the value stored in a container. So I invented domain_type and range_type using the "domain" and "range" terminology from mathematics.

Can't you implemant ReplaceAll via RemoveAll followed by Add? Imo the ReplaceAll function is too complex. Has multiple code paths depending on src/dst size and micro-optimisations are not useful here.

Code is nastier, but it's more efficient. I'd rather keep the faster code, despite it being harder to follow. I wrote a unit test suite around this because that is the only way you can ensure it works. This is one self contained bit where we need to write it once, test it to death, and then never touch it again.

I can take another look at this and see if there is a way to make it more readable.

[Ghabry]

lower_bound is the C++ binary search, it just has a different name. std::map has very poor memory locality and also memory usage overhead, especially when your object type is just a pair of integers.

Sorry this was not a criticism. I knew why you used std::vector and binary search here and it makes totally sense.

I would just prefer if there would be a binary_search function that returns "end" when the element was not found. Instead of the element next to it.

---

Would this data structure also be useful for the FileFinder in my refactor?

I would need a sorted std::string -> std::string mapping and a "Find" function. So imo InputArray would fulfill this required? (just need a "Find").

---

And clang fails to resolve the enum type correctly. When I look in the debugger it shows negative numbers for some keys even though you specified uint8_t :(

[fmatthew5876]

I factored out the map implementation into a FlatUniqueMultiMap container and switched the 2 mapping types to pair.

Despite the code being more generic now, I think it's actually a lot more understandable.

Would this data structure also be useful for the FileFinder in my refactor?

I would need a sorted std::string -> std::string mapping and a "Find" function. So imo InputArray > would fulfill this required? (just need a "Find").

Not exactly, this is one is mapping of keys to multiple values, where each (key, value) pair has to be unique. It's not a FlatMap or even a FlatMultiMap.

What you want is just a simple FlatMap that doesn't allow multiple keys or values. I suggest doing a quick benchmark and just pulling in something like boost::flat_map and seeing if it's faster than std::unordered_map for your use case. If it is, we can add an implementation of it for filefinder.

And clang fails to resolve the enum type correctly. When I look in the debugger it shows negative > numbers for some keys even though you specified uint8_t :(

I don't quite follow this? If the type is uint8_t it can't contain negative values. Is sizeof() the type 1 in your debugger session?

Is the bit pattern / hex the same as an unsigned uint8_t? Could it be something funky in how your debugger is printing values?

[fmatthew5876]

Some numbers on map vs flat_map vs unordered_map

https://stackoverflow.com/questions/36392583/is-an-unordered-map-really-faster-than-a-map-in-practice