InputSystem icon indicating copy to clipboard operation
InputSystem copied to clipboard

ScreenKeyboard implementation.

Open todi1856 opened this issue 4 years ago • 6 comments

Introducing Screen Keyboard implementation for iOS and Android.

The important key of this PR - screen keyboard implementation is fully inside a package:

  • for iOS, there are corresponding iOSScreenKeyboard.mm, iOSScreenKeyboardBridge.mm, iOSScreenKeyboardBridge.h files
  • for Android, AndroidScreenKeyboard.java.

This should help with iteration and bug fixes, because we won't depend on core Unity.

The editor and platforms which don't implement screen keyboard, FakeScreenKeyboard will be used instead, while it doesn't do any visual appearance, it ensures that API behaves in the same manner as platform specific screen keyboards.

This is also backed up by tests Assets\Tests\InputSystem\ScreenKeyboardTests.cs

The screen keyboard code is marked with internal keyword, since the API for it is not finalized. UI toolkit and similar packages shouldn't access it directly anyways, but instead abstraction layer should be used, where input system would implement it. This will give flexibility to do any major refactoring, until we're fully happy with it, only then the API will be made public.

Differences between this implementation and https://docs.unity3d.com/ScriptReference/TouchScreenKeyboard.html

  • Screen keyboard can/will be accessible via InputRuntime.s_Instance.screenKeyboard;
  • Screen keyboard can be shown via Show method by providing ScreenKeyboardShowParams or using overload without any params, in that case keyboard with default params will be shown.
  • There's no alternative for https://docs.unity3d.com/ScriptReference/TouchScreenKeyboard-hideInput.html, you decide if input field should be shown or not during call of Show method.
  • No https://docs.unity3d.com/ScriptReference/TouchScreenKeyboard-isInPlaceEditingAllowed.html
  • For https://docs.unity3d.com/ScriptReference/TouchScreenKeyboard-visible.html & https://docs.unity3d.com/ScriptReference/TouchScreenKeyboard-active.html see state property.
  • No https://docs.unity3d.com/ScriptReference/TouchScreenKeyboard-canGetSelection.html, https://docs.unity3d.com/ScriptReference/TouchScreenKeyboard-canSetSelection.html
  • No https://docs.unity3d.com/ScriptReference/TouchScreenKeyboard-characterLimit.html, since this property is very limiting, in this implementation the same can be achieved by subscribing to inputFieldTextChanged and changing the text from within, this provide much more powerful behavior, since beside character limitation, you can do all kinds of text manipulations
  • No https://docs.unity3d.com/ScriptReference/TouchScreenKeyboard-targetDisplay.html
  • Added events - stateChanged, inputFieldTextChanged, selectionChanged which gives more control over the keyboard
  • All events are called on Unity main thread.

Update 2020-10-12 added keyboard emulation in Editor - https://www.youtube.com/watch?v=ISAv3A9NY2s&ab_channel=UnityTactic

QA since keyboard implementation is internal, you need to have an assembly with name Unity.InputSystem.ScreenKeyboardTests to access keyboard directly. Alternatively checkout Input System project and scene ScreenKeyboardTests.

todi1856 avatar Sep 02 '20 13:09 todi1856

For additional safety, wrap entire if statement in do { } while (0)

That's no good, since destructor would be called too soon on scope. I didn't really understood, how function macro would help, it might cover some scenarios, but not all

todi1856 avatar Sep 08 '20 13:09 todi1856

For additional safety, wrap entire if statement in do { } while (0)

That's no good, since destructor would be called too soon on scope. I didn't really understood, how function macro would help, it might cover some scenarios, but not all

No, leave the scope variable before the "do", just place "if" in it. What I'm suggesting is the the following code should not compile:

KEYBOARD_LOG("xxxx"); else do_something_funny();

currently it would. It may not seem like an issue, until you write:

if (something) KEYBOARD_LOG("something"); else something_else();

aurimasc avatar Sep 08 '20 13:09 aurimasc

if (something) KEYBOARD_LOG("something");

Hmph, I see. Though code like

if (something) KEYBOARD_LOG("something");

will still compile, but will be executed not in the way you would expect.

todi1856 avatar Sep 08 '20 14:09 todi1856

if (something) KEYBOARD_LOG("something");

Hmph, I see. Though code like

if (something) KEYBOARD_LOG("something");

will still compile, but will be executed not in the way you would expect.

Indeed. It seems that the indentation thingy is more risky than it is valueable. It could be solved by making macro expand into object creation and the logging happening inside the constructor of the object.

aurimasc avatar Sep 08 '20 15:09 aurimasc

if (something) KEYBOARD_LOG("something");

Hmph, I see. Though code like if (something) KEYBOARD_LOG("something"); will still compile, but will be executed not in the way you would expect.

Indeed. It seems that the indentation thingy is more risky than it is valueable. It could be solved by making macro expand into object creation and the logging happening inside the constructor of the object.

Good idea, did that. It looks fine now. The indentation doesn't behave completely in a way I wanted, since

KEYBOARD_LOG("sdsd"); KEYBOARD_LOG("nextline"); <-- this will be indented.

But I think it's fine for now

todi1856 avatar Sep 09 '20 08:09 todi1856

Looks like the design doc wasn't shared in this PR: https://docs.google.com/document/d/1iDsV8WRumTDap23TAyBcVRT7RS3oAslEKtVnBgayivA/edit#

TautvydasZilys avatar Sep 22 '20 23:09 TautvydasZilys