rust3ds / ctru-rs

Rust wrapper for libctru
https://rust3ds.github.io/ctru-rs/
Other
116 stars 17 forks source link

Get software keyboard input without max byte size parameter #157

Closed FenrirWolf closed 5 months ago

FenrirWolf commented 5 months ago

Ever since I first started messing with the software keyboard from Rust, I've always hated that libctru's swkbd API forces you to use a fixed-size buffer in order to receive string data from the keyboard. We've worked around that restriction by just forcing the user to specify a maximum size for the intermediate text buffer, but I've always hated that solution too. And so I randomly decided it's finally time to do something about it.

However, it turns out that "doing something about it" involves reimplementing both swkbdInputText and swkbdMessageCallback in Rust and handling a whole bunch of gnarly unsafe code ourselves instead of just leaving all of that nonsense to libctru.

Is it worth introducing this much unsafe code just to solve a small API nitpick? Frankly I'm not entirely sure, but I thought I'd throw this out there and see what you guys think.

Meziu commented 5 months ago

Wow! This re-implementation looks quite nice. I must say I'm not against the "amounts" of unsafe code present here, since it manages to strengthen the API quite a lot and give us more control over the internal implementation (as I said in the past, especially regarding ndsp, there is a lot of things we could improve by getting our hands a bit dirty).

I'll try a deep dive review once I manage to get some time.

FenrirWolf commented 5 months ago

Okay, I think this PR is in a pretty good state now and I probably won't need to push any more changes. I'm not sure what the nightly test failures are about though. They don't seem related to anything actually going on in the code here.

FenrirWolf commented 5 months ago

Also while we're messing around in here, what do you guys think about renaming SoftwareKeyboard::get_string to SoftwareKeyboard::launch? I only chose the former name back when there were 2 functions for getting text data from the keyboard and I wanted to make sure there was a strong distinction between the two, but since this PR does away with the write_exact method, I feel like that would be a sensible change

ian-h-chamberlain commented 5 months ago

Also while we're messing around in here, what do you guys think about renaming SoftwareKeyboard::get_string to SoftwareKeyboard::launch?

Works for me

FenrirWolf commented 5 months ago

Apparently next_multiple_of is only stable as of rust 1.73 and nightly-2023-06-01 is older than that. Everything works on my end though.