Upgrade libraries (in particular winit)

[vn971]

Fixes #176
Fixes #145

[vn971]

The CI errors are about some code that I didn't touch. I could try fix that though. Should I? (CC @ArturKovacs)

[ArturKovacs]

If am pretty sure, you are getting these errors because you updated the ureq dependency to a new major version (1 -> 2), which unsuprisingly has a different API. So yes, you should fix these.

To see these errors on your own computer, you should enable the networking feature when compiling locally. (Preferably you should enable all features but avif is tricky to set up). You can do this by something like

cargo check --features=networking

[benediktwerner]

There's a bit of a mismatch with the keys going on now. Plus and Minus get mapped to Add and Subtract but Asterisk doesn't become Multiply.

I'm wondering, why are the first two even combined in the first place? It seems sensible to separate them. If you want both to trigger something you can just bind both to a key but if you want to differentiate between them you can't with the current system.

[vn971]

Hey, I've fixed the code around ureq. I don't know what to do with the AtomicU32 deprecation warning though -- could anybody help? It's deprecated in Rust itself since "1.50.0"

[benediktwerner]

The solution is to replace it with compare_exchange. The docs explain what the new orderings should be. But I think that can be done in a separate PR. I already offered to fix this and a few of the other clippy warnings in another PR but I haven't heard from Artúr yet.

[vn971]

@benediktwerner indeed I tried to use compare_exchange, but then the result is a Result, so I had to either retry or return the Result out. I had it timeboxed and decided to abort in case somebody knows what to do better.

Let's wait for Artúr's then

[benediktwerner]

The operation could already "fail" before. It's just that compare_and_swap signals failure by returning something that is different from the passed in current value and you had to check it yourself. But in both cases, a failure doesn't really mean that something went wrong. It just means that the current value was different and nothing was changed. But this could be completely expected. Since the current use of compare_and_sawp ignores the returned value the equivalent code could just discard the Result.

[ArturKovacs]

Yeah replace those with compare_exchange and (I guess) use Ordering::SeqCst for both success and failure. We should ignore the result of the function which can be done by

let _ = PRIORITY_REQUEST_ID.compare_exchange(...);

I already offered to fix this and a few of the other clippy warnings in another PR but I haven't heard from Artúr yet

We should probably merge this before #182

[vn971]

Thanks @benediktwerner @ArturKovacs for the explanations! Fixed that warning now, let's see if the CI will succeed.

BTW, I'm not sure if it's checked in CI, but I also see a lot of (unrelated) clippy warning across the codebase. I wonder if it's something that emulsion as a project would want to do (to fix the warnings and enforce lack thereof in CI)

[vn971]

Maybe it's the newer version of clippy that finds more inconsistencies tho

[vn971]

Maybe it's my local clippy that has a newer version and complains. CI just succeeded

[vn971]

I've created a PR for clippy/CI: #184

[ArturKovacs]

There's a bit of a mismatch with the keys going on now. Plus and Minus get mapped to Add and Subtract but Asterisk doesn't become Multiply.

I'm wondering, why are the first two even combined in the first place? It seems sensible to separate them. If you want both to trigger something you can just bind both to a key but if you want to differentiate between them you can't with the current system.

This follows the way it's been in emulsion, so this is correct for now. This is due to a limitation in winit's current keyboard input API. When you press a key which generates a character, you get both a KeyInput and a ReceivedCharacter. The generated unicode character is not known in the KeyInput event, and there's no reliable way to tell which ReceivedCharacter belongs to which KeyInput. But I wanted to allow users to specify unicode characters for key-bindings, instead of having to figure out which virtual keycode they should specify in the config. So the way emulsion handles this, is that whenever it receives a KeyInput, it checks whether the pressed key would generate a character, and if it does, then we don't handle the input because expect that the key-binding will be triggered through ReceivedCharacter. But in ReceivedCharacter you can't differentiate between the numpad add, subtract, etc. and the alphanumeric variant of those.

[ArturKovacs]

Thanks a lot!