Skip to content

Fixed panic on windows when tabbing into WindowsSandbox (Fixes #363)#421

Merged
CryZe merged 3 commits intoLiveSplit:masterfrom
Splamy:master
Apr 1, 2021
Merged

Fixed panic on windows when tabbing into WindowsSandbox (Fixes #363)#421
CryZe merged 3 commits intoLiveSplit:masterfrom
Splamy:master

Conversation

@Splamy
Copy link
Copy Markdown
Contributor

@Splamy Splamy commented Apr 1, 2021

So I took matters into my own hands and fixed the crash I reported in #363 :P

According to this SO post windows may send 0xFF as virtual key code is some edgecases or when a key isn't mapped to a VK.
And apparently switching focus into the windows sandbox is one of them.

Anyway, adding Unmapped = 0xFF, is sufficient to fix the crash because rust apparently didn't like the transmute to a nonexistant enum value.

But for good measure I also added num-traits/num-derive to safely map numbers back to enum values. I don't think this shouldn't have any significant negative runtime impact, though I haven't tested it. If you don't like the change I can also remove it entirely.

An alternative fix without additional libraries or the extra enum field would be to just check if key_code <= 0xFE, but it kinda feels wrong since windows being windows might still send undeclared vk values...

@CryZe
Copy link
Copy Markdown
Collaborator

CryZe commented Apr 1, 2021

Wow, thank you for doing this.

Are you sure it makes sense to map this as a "valid key" rather than "ignoring" it? The Windows documentation states that only values from 1 to 254 are valid. It's also not part of their enum: https://docs.microsoft.com/en-us/windows/win32/inputdev/virtual-key-codes

Also I'm not sure if I like the additional dependency. I recently wanted to remove dependencies that do very little for us, and these dependencies certainly would fall under that same category.

@Splamy
Copy link
Copy Markdown
Contributor Author

Splamy commented Apr 1, 2021

Sure, I can understand that. I just edited my top post to add my note but that probably didn't reach you in time :P
And ignoring the key instead of declaring it seems reasonable.
As said, if you want I can remove it and just make it a 0x0 < key_code <= 0xFE check. It's not as nice as with the number parsing but it should do the job

@CryZe
Copy link
Copy Markdown
Collaborator

CryZe commented Apr 1, 2021

Yeah I'd prefer that tbh.

@CryZe CryZe added bug There is a bug. enhancement An improvement for livesplit-core. hotkey This is about the hotkey implementation. labels Apr 1, 2021
@CryZe CryZe linked an issue Apr 1, 2021 that may be closed by this pull request
@Splamy
Copy link
Copy Markdown
Contributor Author

Splamy commented Apr 1, 2021

Alright I canged it.

@CryZe CryZe merged commit 624783a into LiveSplit:master Apr 1, 2021
@CryZe CryZe added this to the v0.12 milestone Jun 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug There is a bug. enhancement An improvement for livesplit-core. hotkey This is about the hotkey implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

livesplit-hotkey crate: panics when interacting with Windows Sandbox

2 participants