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

possible fix to the linux e,s,r bug in text editor. #2334

Merged
merged 1 commit into from
Jan 14, 2019

Conversation

Dunbaratu
Copy link
Member

@Dunbaratu Dunbaratu commented Oct 4, 2018

Fixes #1071

So it was a bug in kOS, but a bug that got hidden by Unity's order of handling input events on Windows.

The bug - While I was only reacting to the E, S, or T keys when "control" was pressed, which is correct, I was still "consuming" those keyboard events unconditionally whether the control key was down or not.

Why it didn't affect Windows: Apparently in Windows the generic Textfield GUI widget from Unity's IMGUI (the thing the text editor really is), got first dibs on the keyboard events before my code did, so my code never got to see key events that the Textfield used, so S, E, and R never got seen by my code unless they were accompanied by special ctrl- or alt- keys. Vanilla S, E, and R keys never reached my code - the Textfield got them first and consumed them.

On Linux they seem to be queued in the opposite order so that my mistaken consuming of the key happened before the Textfield got to see it.

@Dunbaratu
Copy link
Member Author

I have had 2 different Linux users try a test compiled ZIP of this PR, and both reported that it fixed the problem. I also tried it on Windows to verify that it still works on Windows after the change. I say it's good enough to merge based on that, but someone else should merge it, unless I decide it's okay to abandon that rule because other devs have no time for the mod anymore.

@Dunbaratu
Copy link
Member Author

Dunbaratu commented Nov 29, 2018

Can one of the devs using Linux just give this a quick once-over and put an official (as in reviewer) stamp of approval on it so I can merge it without breaking our "don't merge without second person review" rules?

^Tagging @lucaelin and @evandisoft on this. Either one of you would do.

@evandisoft
Copy link
Contributor

I'll check this out after I set things up to work on multiple branches at a time.
https://stackoverflow.com/questions/6270193/multiple-working-directories-with-git/30185564#30185564

@evandisoft
Copy link
Contributor

evandisoft commented Dec 1, 2018

It Works! Thank you! And the code changes don't raise any alarms to me. (Since you've tested it on windows.)

@erendrake erendrake merged commit 4f04f31 into KSP-KOS:develop Jan 14, 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.

3 participants