Commit 90b72a8 ("Factor out a sys crate") changed the first argument for RetroEnvironment::set_raw from &self to &mut self. However, RetroCore still receives non-mut references to RetroEnvironment in all of its functions. As a result, it's not possible to call set_raw any more.

First off, thanks for this crate! I was able to get a toy libretro core up and running in no time, even as a Rust newbie.

That said, having to return an instance of my core in the init method was unpleasant since it meant deferring initialization of some fields. Removing init and returning the core instance in the return value of load_game in my fork of libretro-rs streamlined my core's code.

To the best of my knowledge, anything that can be done in retro_init can also be done in retro_load_game. I'm relatively new to libretro development so it's possible I overlooked something, but I couldn't find any options in libretro.h for which that wasn't the case. I also found a comment in the libretro Discord server that confirms this: "I think the idea is that retro_init should do stuff relevant for all content, and front doesn't call deinit+init if it loads new content in same core, to save some time. Not sure if any cores actually do that and benefit from it in practice. It's perfectly fine to do everything in load_game.".

Moreover, it's not possible to report errors to the frontend until retro_load_game is called since retro_init lacks a return value. It's hard for me to imagine a situation where partially initializing a core in retro_init would provide a benefit over doing all of the initialization in retro_load_game. The same user quoted above also said: "retro_init returning void is also generally considered a mistake in hindsight. the standard recommendation is set a global variable that makes retro_load_game instafail." I'd go as far as to say there probably isn't a good reason to use retro_init even in C code due to this.

While this would be a breaking change to libretro-rs, I believe it'd make the crate easier to use for most (all?) users without any loss of generality. If you want, I can open a pull request with my changes.

The example project is neat, but I think having a concrete example would be better. I have a CHIP-8 emulator to offer up for this purpose, it's simple and uses more functionality than the current example. Thoughts?

/cc @InquisitiveCoder

Exposes all retro_* callbacks in RetroCore. This is necessary to future-proof the crate's API, since environment commands can be specific to certain callbacks. In my opinion it also makes the crate easier to learn since it'll have a close mapping to the C API.

Also makes some slight alterations to the existing RetroCore methods, like removing the redundant size parameter and using associated types instead of just u32 for user-defined enums.

Starting with the most benign changes before I move on to API updates.

• Updated .gitignore.
• Updated bindgen version.
• Addressed clippy lints.
• Added traits and #[non_exhaustive] to enums affected by lints.

When the environment callback is called with RETRO_ENVIRONMENT_SET_SUPPORT_NO_GAME is set to true, retro_load_game() will be called with NULL rather than a pointer to a retro_game_info. However, this currently can't be used in practice since:

• libretro_core! and RetroInstance::on_load_game use &retro_game_info as the type of its argument.
• RetroCore uses RetroGame as the type of its argument. Neither of these types are allowed to be null.

At a minimum libretro_core and on_load_game need a different type to accommodate the NULL value. As for RetroCore, I'm not sure if you'd prefer changing the argument to Option<RetroGame> or having a separate trait (e.g. NoGameRetroCore) that doesn't have the RetroGame argument at all.

My core is a game rather than an emulator and I won't be loading any of the game data from an external file so this use case is directly relevant to me. In particular, not having this forces me to create a 0 byte "rom" just so RetroArch will load my core.

@InquisitiveCoder

This is the approach I've had in mind for how to restrict which environment methods a given callback can use. Accomplished by adding a phantom type parameter to RetroEnvironment, and then one empty enum for each callback. Enum is used here to prevent a user from creating instances, an empty struct (struct LoadGame;) could still be created with let x = LoadGame;.

We then provide an impl for each RetroEnvironment<Enum> that just forwards the call along to the corresponding definition. This impl layer will be tedious, so I imagine we'll want a macro to do it for us: retro_env!(Global, get_core_assets_directory, get_content_directory).

To make converting between the different phantom types easily, an associated method is provided called into_state() which exists on all instances of RetroEnvironment<T>.

I see this approach being easy to follow, as you can just read the macro invocation to see which environment calls a particular callback has access to.

Let me know what you think!

Introduction

You'll probably want to turn off whitespace changes to weed out indentation changes in the code.

This PR is a collection of miscellaneous API improvements I tinkered with over the weekend. The goals for these changes, in rough order of importance, were:

1. Expose the full functionality of the libretro API.
2. Future-proof the API against new environment commands.
3. Enforce more of libretro's rules using types.
4. Make the crate's abstractions as close to zero-cost as possible.
5. Minor project housekeeping (e.g. update the gitignore.)

I wanted to get these breaking changes out of the way now before moving on to adding more environment commands and features.

Major Changes

Environment overhaul

Scoped environments

Addresses #2 and lays the groundwork for #16. The new implementation exposes an environment trait for every retro_* function, so that we can limit which non-raw environment commands are available. The *_raw methods are always available, since they're the user's escape hatch for functionality the crate doesn't implement yet.

I initially tried to implement this by adding a PhantomData parameter to the environment, but this required casting in every RetroInstance callback as well as polluting the API with a lot of marker structs. I found the trait-based implementation much more straightfoward, since the C callback can implement all of the environment traits simultaneously, and it allows users to unit test their RetroCore since they no longer have a hard-coded dependency on the C function pointer.

Reworked raw methods

The base environment trait defines its raw_* methods in terms of Rust types, not pointers, and there's a new method fn mut_ref_raw<T>(&mut self, key: u32, data: &mut T) for commands that mutate structs.

All raw methods are unsafe

All raw methods should be unsafe since they allow the user to violate libretro's requirements and we can't account for future additions to the environment callback.

RetroCore API improvements

Exposed every retro_* function in RetroCore

Because libretro can add new environment commands at any time, and those commands may require being called during specific retro_* callbacks, the only way to expose libretro's full functionality and future-proof the API is to surface all of the retro_* to the user.

Other benefits of this change are less glue code in RetroInstance and a much tighter correspondence between the Rust API and C API, which should help users cross-reference the documentation.

Removed redundant method parameters.

Some functions in the C API pass an array pointer and the array's size as separate parameters. In RetroCore, the array pointers were translated into Rust slices, but the redundant size parameter wasn't removed. Since slices carry their length, I've removed these size parameters.

Refine c_uint parameters.

libretro's ABI uses unsigned int for all integers, even if its parameters would fit into a byte or short. To the best of my ability, I've replaced most c_uint/u32 parameters with the narrowest fixed-size type that will accept all of its values.

Additionally, two of these parameters pass user-defined values that are established in environment commands. These have been replaced with two associated types:

pub trait RetroCore: Sized {
  type SpecialGameType: TryFrom<u8>;
  type SubsystemMemoryType: TryFrom<u8>;

  fn load_game_special(&mut self, env: &mut impl LoadGameSpecialEnvironment, game_type: Self::SpecialGameType, info: RetroGame) -> bool;

  fn get_memory_data(&mut self, env: &mut impl GetMemoryDataEnvironment, id: RetroMemoryType<Self::SubsystemMemoryType>) -> Option<&mut [u8]>;
  // etc.

This will allow users to use their own enums, provided the enum is convertible into a u8. Cores that have no need for this functionality can use the provided empty tuple struct NotApplicable:

impl RetroCore for Emulator {
  type SpecialGameType = NotApplicable;
  type SubsystemMemoryType = NotApplicable;
  // etc.
}

Overhauled string handling

Some C strings returned by libretro are not guaranteed to be UTF-8 encoded. The crate should expose these as &CStr and let the user decide if they want to assume they're UTF-8 or handle them some other way. Convenience methods have been provided to convert Option<&CStr> to Option<&str> in one step.

Additionally, the API uses the CUtf8 crate when exposing UTF-8 C Strings, since this type preserves the fact that the string slice is both UTF-8 and nul-terminated. This allows for trivial conversions to both &CStr or &str. &str can't do this because once a &CStr is converted to &str, you've discarded the fact that it was originally nul-terminated.

Internal changes

Zero-cost struct conversions

Rust versions of the C structs generated by bindgen are now implemented as #declare(transparent) newtype wrappers. The public API still uses idiomatic Rust types, but the conversion to a libretro struct should be a no-op:

/// Rust interface for [retro_system_timing].
#[repr(transparent)]
#[derive(Clone, Debug)]
pub struct RetroSystemTiming(retro_system_timing);

impl RetroSystemTiming {
  /// Main constructor.
  pub fn new(fps: f64, sample_rate: f64) -> Self { Self(retro_system_timing { fps, sample_rate }) }
  pub fn fps(&self) -> f64 { self.0.fps }
  pub fn sample_rate(&self) -> f64 { self.0.sample_rate }
  pub fn into_inner(self) -> retro_system_timing { self.0 }
}

Modularized the crate

lib.rs was getting crowded, so wherever I overhauled a type I moved it to a new file. For the sake of making the diff easier to verify, any code that only required minor changes was left in lib.rs.

Removed libc

To the best of my knowledge, we weren't using it for anything that core::ffi::* didn't already provide.

Updated bindgen version

I updated bindgen to the latest version. This resulted in 2 or 3 lines in RetroInstance having some casts adjusted.

core over std

I've replaced references to std with core wherever possible, which should help us be more conscious of places where we're doing allocations and may allow the crate to function in a no_std context.

Afterword

Sorry for the big PR. I wasn't expecting to submit this much at once, but as I was experimenting one thing just led to another. As usual, let me know where you think a change missed the mark.

Commit 9b4c3f0 ("various cleanups and improvements") changed the first argument of RetroRuntime::is_joypad_button_pressed from u32 to RetroDevicePort, but RetroDevicePort doesn't have a public constructor.

Addesses Issue #3: Move RetroCore instantiation to load_game for better ergonomics. Below is a summary of the changes:

• retro_init and retro_deinit are now no-ops.
• Removed RetroCore::init.
• RetroLoadGameResult contains an additional field for the RetroCore instance.
• RetroCore::load_game is now a function.
• Removed RetroInstance::on_init and on_deinit. The code has been moved to on_load_game and on_unload_game respectively.

With these changes, users of the crate return a fully initialized instance from RetroCore::load_game rather than return a partially initialized instance from init and updating it in load_game.

Since this isn't a backwards-compatible change, I've updated the minor digit of the version number.

Certain uses of the environment callback can only be done during retro_set_environment; however, users of the crate lack the ability to execute code during that callback. Out of several such options, the libretro_core! macro only handles SET_SUPPORT_NO_GAME. However, this approach requires updating the macro and trait every time a new option is added to the environment callback that can only be handled from retro_set_environment. A more flexible approach that'd support all current and future options would be to add a set_environment() function to RetroCore which the macro then calls.

This is a follow-up to #20. The two associated types that were introduced in that PR have been replaced with type parameters with defaults. With this change, users that don't need load_game_special or get_memory are no longer required to explicitly define types for parameters to methods they don't implement.

First of all, sorry if there was a way to ask this without creating a new issue, but I couldn't think of one.

I noticed that RetroRuntime only exposes a function to obtain joypad input (is_joypad_button_pressed). Since input_state is private, I cannot call it with RETRO_DEVICE_KEYBOARD to obtain keyboard input. Is there another way to do this?

Since things have changed, documenting the changes by updating the README.md before releasing 0.2.0 will be required. This should be done as late as possible (i.e. right before release) so that the full scope of changes can be reviewed for the update.

Many of the calls for get_raw, set_raw, and cmd_raw aren't modeled properly. Effort should be focused on getting them exposed in an idiomatic way so that more of the unsafe API surface is reduced. This doesn't need to be as restrictive as #2 for the moment, but laying the groundwork for it would be nice.

It would be neat to showcase some projects that are using libretro-rs, and it would be a fun way to help those projects get some more exposure, as well as get exposure for this project. Win/win.

If you'd like your project added to README.md, add a comment below.

Frontends can provide a logging callback via RETRO_ENVIRONMENT_GET_LOG_INTERFACE, which is currently available thanks to RetroEnvironment::set_raw, but it requires using FFI types. Ideally the crate would provide a native Rust interface for logging, as well as a fallback implementation in case the frontend doesn't support logging.

I already have code in my core to handle this, so this issue is primarily here for tracking purposes while I migrate it.