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 libretro support #83

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Conversation

RobLoach
Copy link

@RobLoach RobLoach commented May 17, 2024

This introduces libretro.h virtual file system support for PhysFS, allowing to use PhysFS directly from a libretro core through RETRO_ENVIRONMENT_GET_VFS_INTERFACE.

I've put together a physfs_test_libretro core to test out the functionality. It was originally written for the chailove core.

A few considerations...

  1. In both Playdate, and libretro now, PHYSFS_init() requires you to pass in some user data pertaining to the environment as an argv0 argument. As you mentioned in the comments, it seems like a cheat. Would it make sense to introduce a function like PHYSFS_setPlatformData(void*) or something?
  2. I didn't include the CMakeLists.txt update, as this does depend on some of libretro-common, and libretro-common doesn't use CMake at all. Would be nice to include CMake support, but I'm unsure how to best accomplish it. For reference, it depends on the following from libretro-common...
    include/libretro.h
    rthreads/rthreads.c
    
  3. Haven't tested this with UTF-8 paths yet
  4. Unsure of the pref directory strategy so far

@RobLoach RobLoach marked this pull request as draft May 17, 2024 12:39
@icculus
Copy link
Owner

icculus commented May 17, 2024

It was originally written for the chailove core.

Before we go any further: is this code able to be contributed under a zlib license? ChaiLove uses the MIT license.

@RobLoach
Copy link
Author

Good call. As the original author of ChaiLove, I'm okay to re-license this code to zlib. I got the 👍 from phcoder too.


char *__PHYSFS_platformCalcBaseDir(const char *argv0)
{
/* TODO: Replace getcwd() with a libretro-common function call? */
Copy link
Author

@RobLoach RobLoach May 17, 2024

Choose a reason for hiding this comment

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

For the base directory, it's currently using getcwd(), but this may not be the best solution. Maybe use RETRO_ENVIRONMENT_GET_LIBRETRO_PATH instead? There's also RETRO_ENVIRONMENT_GET_GAME_INFO_EXT, but that may not be set when PHYSFS_init() is called.

Copy link
Owner

Choose a reason for hiding this comment

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

This is meant to be where to find game data files...which for a libretro core is either going to be the engine's data or the game/rom-specific data...I'd lean towards the latter, if they aren't the same thing.

Copy link
Author

@RobLoach RobLoach May 18, 2024

Choose a reason for hiding this comment

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

Good brainstorming, thanks. Made an update to...

  1. Try to get the game directory via GET_GAME_INFO_EXT, which is a directory in which the content file exists
  2. Otherwise, use GET_SYSTEM_DIRECTORY, as that may make sense for data files
  3. If both fail, use /

@icculus
Copy link
Owner

icculus commented May 17, 2024

In both Playdate, and libretro now

(AND android...this is a big mess, and I'll probably change this in PhysicsFS 4 to not take a const char *, but this API has been otherwise-stable for like...23 years now, so I'm hesitant to change it. :) Hack it in for now.)

@sezero
Copy link
Collaborator

sezero commented May 18, 2024

May I suggest something like the following to make it buildable in C90 mode and avoid errors if ELOOP isn't available:

diff --git a/src/physfs_platform_libretro.c b/src/physfs_platform_libretro.c
index 58694ba..dcc0c87 100644
--- a/src/physfs_platform_libretro.c
+++ b/src/physfs_platform_libretro.c
@@ -306,9 +306,10 @@ void *__PHYSFS_platformOpenAppend(const char *filename)
 PHYSFS_sint64 __PHYSFS_platformRead(void *opaque, void *buffer,
                                     PHYSFS_uint64 len)
 {
+    PHYSFS_sint64 retval;
     BAIL_IF(physfs_platform_libretro_vfs == NULL || physfs_platform_libretro_vfs->read == NULL, PHYSFS_ERR_NOT_INITIALIZED, -1);
     BAIL_IF(opaque == NULL || buffer == NULL, PHYSFS_ERR_INVALID_ARGUMENT, -1);
-    PHYSFS_sint64 retval = (PHYSFS_sint64) physfs_platform_libretro_vfs->read((struct retro_vfs_file_handle *) opaque, buffer, (uint64_t) len);
+    retval = (PHYSFS_sint64) physfs_platform_libretro_vfs->read((struct retro_vfs_file_handle *) opaque, buffer, (uint64_t) len);
     BAIL_IF(retval == -1, PHYSFS_ERR_IO, -1);
     return retval;
 } /* __PHYSFS_platformRead */
@@ -317,9 +318,10 @@ PHYSFS_sint64 __PHYSFS_platformRead(void *opaque, void *buffer,
 PHYSFS_sint64 __PHYSFS_platformWrite(void *opaque, const void *buffer,
                                      PHYSFS_uint64 len)
 {
+    PHYSFS_sint64 retval;
     BAIL_IF(physfs_platform_libretro_vfs == NULL || physfs_platform_libretro_vfs->write == NULL, PHYSFS_ERR_NOT_INITIALIZED, -1);
     BAIL_IF(opaque == NULL || buffer == NULL, PHYSFS_ERR_INVALID_ARGUMENT, -1);
-    PHYSFS_sint64 retval = (PHYSFS_sint64) physfs_platform_libretro_vfs->write((struct retro_vfs_file_handle *) opaque, buffer, (uint64_t)len);
+    retval = (PHYSFS_sint64) physfs_platform_libretro_vfs->write((struct retro_vfs_file_handle *) opaque, buffer, (uint64_t)len);
     BAIL_IF(retval == -1, PHYSFS_ERR_IO, -1);
     return retval;
 } /* __PHYSFS_platformWrite */
@@ -386,7 +388,9 @@ static PHYSFS_ErrorCode errcodeFromErrnoError(const int err)
         case WSAEDQUOT: return PHYSFS_ERR_NO_SPACE;
 #endif
         case EIO: return PHYSFS_ERR_IO;
+#if defined(ELOOP)
         case ELOOP: return PHYSFS_ERR_SYMLINK_LOOP;
+#endif
         case EMLINK: return PHYSFS_ERR_NO_SPACE;
         case ENAMETOOLONG: return PHYSFS_ERR_BAD_FILENAME;
         case ENOENT: return PHYSFS_ERR_NOT_FOUND;

@RobLoach RobLoach marked this pull request as ready for review May 25, 2024 21:13
@RobLoach
Copy link
Author

Did another test of this with https://github.com/robloach/physfs_test_libretro ... Looking pretty good. If there's anything else you can think of adding, feel free to let me know.

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.

3 participants