rust3ds / ctru-rs

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

PS Service Integration in STD #38

Closed Meziu closed 2 years ago

Meziu commented 2 years ago

Because rust-linker-fix-3ds uses the PS service to generate random data, the service needs to be initialised before using getrandom, or any type (like HashMap) that calls it under the impl. Now, while we can make docs about it for direct usage, this does create a problem when the function is called by the underlying implementation of an unknown crate. Should we init this service by default (like libctru already does with many services) in ctru::init?

P.S. This is just a discussion thread because I don't know how to proceed about this. Tell me what you think. @AzureMarker @ian-h-chamberlain

ian-h-chamberlain commented 2 years ago

I have a small local patch which creates a Ps struct that would be initialized similar to Console, in that you must bind it to an unused variable that lives for the duration of the program... The difference compared to Console is that std will panic if it's not initialized, instead of e.g. println doing nothing without a Console.

We could also add a feature std to this crate, which enables automatically initing services like Ps that are expected to be used during the course of normal development with std, in which case we'd probably use sync::Once or something instead to initialize it, and I guess we'd just never call psExit()?

I'm open to different options, but I think the API being similar to Console sort of makes sense, plus that handle could be used for other things later on if we add functionality (like PS_GetLocalFriendCodeSeed(), PS_GetDeviceId()), which will never be called by std.

AzureMarker commented 2 years ago

What's the error look like if something calls getrandom before PS is initialized? If it's informative then maybe it's not a big issue, and the user can figure it out.

Otherwise, I do like the std feature idea, but if we do that then we need to revisit how the rest of the crate handles std.

Using Once is another approach, and that might be fine if we can determine that not calling the exit function is OK.

The other PS functions are a good reason to have an explicit handle... This approach would be the best in terms of making things explicit and would allow surfacing the PS functions easily. However we need to make sure the user knows to use it (ex. When something needs randomness, like HashMap).

Meziu commented 2 years ago

What's the error look like if something calls getrandom before PS is initialized? If it's informative then maybe it's not a big issue, and the user can figure it out.

It's a generic "getrandom returned an unknown error".

Meziu commented 2 years ago

I too think the Ps handle approach is best, but the issue is in the communication to the user. Also, as I said, the real issues come when a crate uses HashMaps without the user knowing. How bad is not calling psExit? It looks like a pretty mild service, without much integration with the system.

Another idea is returning some sort of "generic ctru" handle from ctru::init, though that is stretching a bit how much thought that function needs.

Otherwise, if there isn't an issue with calling psInit multiple times, we could just leave the call there (without a Once). This way the user can make and drop the safe handle we provide, but the function will always work (just not exit when it could).

ian-h-chamberlain commented 2 years ago

Otherwise, I do like the std feature idea, but if we do that then we need to revisit how the rest of the crate handles std.

Hmm, this might be overkill, but maybe with features = ["std"] we could provide an attribute macro like #[ctru::main], which handles setup and teardown by initializing commonly used services and tears them down at the end of main sort of like this:

fn main() {
    fn user_main() { .. }

    // setup...
    user_main();
    // teardown
}

I guess panicking programs would still not tear down those services, unless we also use safe wrappers with Drop impls there...

Meziu commented 2 years ago

Oh yeah, the #[ctru::main] to substitute ctru::init is a great idea, and it would solve many problems long term. Panicking programs are cleaned up too (it's more of a RAM wipe, but the OS doesn't have issues with it).

@AzureMarker what do you think?

AzureMarker commented 2 years ago

I'm hesitant about a #[ctru::main] macro. It should only be for convenience (not a requirement, no magic), and we should be clear about what it actually does (list the services it initializes). Most of the time in my experience, those kind of macros (like #[tokio::main]) end up only used for demonstrations, examples, and very simple programs, mainly because any non-trivial program will need to modify the things the macro sets up (in tokio's case, the runtime, or changing when it gets created).

Another concern is that it would hide the handles of important services like Gfx and Apt. This seems to be a big blocker.

I'm not sure how panicking comes into play, because our service wrappers implement Drop (which gets called when unwinding).

I think overall this problem of getting the user to initialize Ps might not be as bad as we thought. We can add documentation listing the various service wrappers and what they enable (ex. Hid for input, Ps for randomness and hash maps, etc). Beyond that we will have examples that use this functionality. We can even have a separate example for hash maps that notes the Ps service requirement. Since the setup work is pretty big already (set up graphics, game loop with apt.main_loop(), etc) I expect users will be looking at the examples when scaffolding their program.

Meziu commented 2 years ago

We have to be really clear about it. Let's see how far we get then.