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

Add utils.showFloatingGamepadTextInput() #113

Merged
merged 3 commits into from
Jul 14, 2024

Conversation

dfabulich
Copy link
Contributor

This one was pretty weird, due to the callback.

Unlike all other callbacks in steamworks-rs, the show_gamepad_text_input and show_floating_gamepad_text_input functions expect a dismissed_cb callback as an argument to the function.

https://github.com/Noxime/steamworks-rs/blob/e9cdf88f1c436871e2f72d9726da6eceac0679d4/src/utils.rs#L281

    /// Opens a floating keyboard over the game content and sends OS keyboard keys directly to the game.
    ///
    /// The text field position is specified in pixels relative the origin of the game window and is used to
    /// position the floating keyboard in a way that doesn't cover the text field.
    ///
    /// Callback is triggered when user dismisses the text input
    pub fn show_floating_gamepad_text_input<F>(
        &self,
        keyboard_mode: FloatingGamepadTextInputMode,
        x: i32,
        y: i32,
        width: i32,
        height: i32,
        mut dismissed_cb: F,
    ) -> bool
    where
        F: FnMut() + 'static + Send, // TODO: Support FnOnce callbacks
    {
        unsafe {
            register_callback(&self._inner, move |_: FloatingGamepadTextInputDismissed| {
                dismissed_cb();
            });
            sys::SteamAPI_ISteamUtils_ShowFloatingGamepadTextInput(
                self.utils,
                keyboard_mode.into(),
                x,
                y,
                width,
                height,
            )
        }
    }

The FloatingGamepadTextInputDismissed struct simply doesn't have #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] as other callback structs do, so when I tried adding FloatingGamepadTextInputDismissed to callback.rs here in steamworks.js, the compile failed when I tried to register_callback::<steamworks::FloatingGamepadTextInputDismissed>(threadsafe_handler).

I guess I could investigate fixing that as a bug in steamworks-rs, but I don't actually happen to need the FloatingGamepadTextInputDismissed callback, myself, so, in this PR, I'm just passing show_floating_gamepad_text_input an empty callback, just to satisfy the compiler. Presumably, if someone needs the FloatingGamepadTextInputDismissed callback, they can implement it themselves (perhaps adding serde support to FloatingGamepadTextInputDismissed in steamworks-rs first, then getting that merged, and then adding it to callback.rs here.

@dfabulich dfabulich force-pushed the show-floating-gamepad-text-input branch from 2b4a770 to bf749a0 Compare August 2, 2023 16:48
@dfabulich
Copy link
Contributor Author

Would it be possible to implement a JS-based callback parameter in this case? It'd be awesome if the end user could write code like this:

showFloatingGamepadTextInput(FloatingGamepadTextInputMode.SingleLine, x, y, width, number, () => {
    console.log('callback, input dismissed');
});

My NAPI skills are still pretty weak. (I mostly write these PRs by copying and pasting! 🫠)

Implementing that would also be necessary in order to implement the non-floating showGamepadTextInput method, which returns the typed text in a callback as a string.

@ceifa
Copy link
Owner

ceifa commented Aug 29, 2023

Would it be possible to implement a JS-based callback parameter in this case? It'd be awesome if the end user could write code like this:

showFloatingGamepadTextInput(FloatingGamepadTextInputMode.SingleLine, x, y, width, number, () => {
    console.log('callback, input dismissed');
});

Agreed. We can do it with a threadsafe function on napi. I can work on it later

@dfabulich dfabulich force-pushed the show-floating-gamepad-text-input branch from bf749a0 to c4195e4 Compare August 31, 2023 02:33
@vsobotka
Copy link

If I understand correctly, this will enable us to open virtual keyboard on Steam Deck, when an input is clicked, is that correct? What is the current state? Is there some workaround until this is released?

@dfabulich
Copy link
Contributor Author

There's no workaround; we just have to wait. (Unless you know enough NAPI to fix callbacks in my PR…?)

@cpojer
Copy link

cpojer commented Jun 12, 2024

@ceifa what would it take to get this merged and released, even without the ability to pass a callback for now?

@cpojer
Copy link

cpojer commented Jun 13, 2024

I published this PR as @nkzw/steamworks.js 0.4.0 on top of the current version for anyone who needs this in the short-term.

@ceifa
Copy link
Owner

ceifa commented Jun 14, 2024

Hey @dfabulich! Thanks again for the PR and sorry for the delay. Because I don't have a steam deck to test it myself, can you help me clarify the following questions?

  • The current implementation of "showFloatingGamepadTextInput" works out of the box, as it works as a keyboard and electron/html can handle them just fine?
  • I guess the "showGamepadTextInput" does not work, as it is an external input, which would give us the result in a callback (not implemented)?

@cpojer
Copy link

cpojer commented Jun 16, 2024

I just open sourced my integration for Athena Crisis, see this commit: nkzw-tech/athena-crisis@27e5ab8

To answer your questions:

  • showFloatingGamepadTextInput will open the keyboard whenever you call the function, not automatically when tapping input elements (of course). With the code I shared above, it will work reasonably well and opens the keyboard when focusing an input element and it forwards keypresses correctly. However, I've found that it does not always respect the dimensions that I pass in, putting the keyboard into the wrong spot many times, and it's a bit cumbersome not knowing whether the keyboard is currently open or not.
  • showGamepadTextInput does work in that it opens the keyboard with a custom floating input element, but you are correct, since the callback is not implemented it is basically useless. In order to make it work, the callback needs to be implemented as well as GetEnteredGamepadTextLength. Since that function always needs to be called to get the result, I think it makes sense to automatically call that and pass it to the showGamepadTextInput callback in JS.

A good path forward could be to merge support for showFloatingGamepadTextInput (without callback, since it's still useful), and then focus on the callback use case and showGamepadTextInput support in a separate PR.

@ceifa
Copy link
Owner

ceifa commented Jun 17, 2024

Thanks @cpojer! I will take a look at this and fix the missing points.

@ceifa
Copy link
Owner

ceifa commented Jun 18, 2024

@dfabulich @cpojer just finished the API. Decided to use promises instead of callback because I think it makes sense since it would be used to control code flow and can only be called once.

Because I don't have a steam deck I couldn't test this change... you can help me on that?

Comment on lines +121 to +132
move || {
if let Some(tx) = tx.take() {
tx.send(true).unwrap();
}
},
);

if opened {
rx.await.unwrap()
} else {
false
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm understanding the docs correctly, this function will immediately return true/false if the keyboard was shown or not, and invoke the dismissed callback once the keyboard is dismissed by user. The API might need some adjustments in that case as this promise based API doesn't behave the same way. An alternative solution would be to return false | Promise<void> to denote that either it wasn't opened (and there is no need to listen for the dismissed event), or a tuple/object like [opened: boolean, onDismissed: Promise<void>] for a cleaner state separation.

Let me know if I'm missing something.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see a real use case where you will need both, I think the promise way maes more sense on JS and can benefit the consumer for a more fluid code flow. What do you think?

@cpojer
Copy link

cpojer commented Jun 19, 2024

I do have a Steam Deck but I don't have an easy way to build the artifacts without publishing. If you can provide me with a linux build of an electron app with an example I'm happy to test it right away.

@dfabulich
Copy link
Contributor Author

I can do some testing later this week, but testing this API doesn't require a Steam Deck. You can test it using Steam Big Picture mode with any gamepad controller plugged into your PC, e.g. Xbox or PS5 controller.

Comment on lines +68 to +73
if let Some(tx) = tx.take() {
let text = client
.utils()
.get_entered_gamepad_text_input(&dismissed_data);
tx.send(text).unwrap();
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if there is no result here? The Promise will never resolve, right?

Looking further at the docs, this function returns true if "big picture overlay is running" but I'm wondering if that's a docs mistake and it returns true/false if the keyboard was opened or not similar to the floating text input function (or if it kinda means the same in this case, anyway).

Since the two new functions can be called without showing the gamepad, there is one more wrinkle in the Promise based API: Technically, promises are expected to be resolved at some point in the future, but if the gamepad doesn't open or doesn't exist, the Promise will never be handled. This is kinda ok-ish when using the Promise API, but it's very confusing when using async/await since the code block will just get stuck:

try {
  const result = await showGamepadTextInput();
  doSomething(result); // This will never be called.
} catch (error) {
  // This also never happens.
}

Most likely this is fine if it's documented. One thing to keep in mind is that this behavior is likely attached to input elements and is called repeatedly. Without knowing whether the gamepad can/is ever shown on the platform that it is being called on it will create a lot of dangling promises in memory that will stay unresolved for the duration of the app runtime.

Now, this might be an acceptable trade-off for you, but I wanted to bring it up anyway for consideration.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if there is no result here? The Promise will never resolve, right? (and the questions below that)

I don't think so, the memory management will drop the promise and unsign the callback as soon as the function returns something.

The idea is to always resolve the promise, either with the text input or with an undefined state, meaning the text input could not be shown or was cancelled. Example impl:

const input = await showGamepadTextInput();
if (input) {
    doSomething(input);
} else {
    // do nothing, as use cancelled the input request
}

Looking further at the docs, this function returns true if "big picture overlay is running" but I'm wondering if that's a docs mistake and it returns true/false if the keyboard was opened or not similar to the floating text input function (or if it kinda means the same in this case, anyway).

I think it means the same thing 🤔

@ceifa ceifa merged commit 5a1f44f into ceifa:main Jul 14, 2024
4 checks passed
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.

4 participants