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

Libretro upstreaming #1464

Merged
merged 30 commits into from May 11, 2019

Conversation

@Ghabry
Copy link
Member

commented Oct 29, 2018

I'm slowly annoyed by always rebasing this therefore I would like to finally upstream the libretro code.

The code is not very invasive, only adds required libretro_ui and _audio files and "USE_LIBRETRO" where needed.

CMake considerations:
The CMakeLists is sufficient to build a easyrpg_libretro.a/.so when you have all dependencies installed (I refuse to add dependency source code in the upstream repo), simply compile with -DPLAYER_TARGET_PLATFORM="libretro" -DBUILD_SHARED_LIBS=ON and you get a library that works with at least Linux RetroArch.

The exception is libretro-common which I added as a submodule with an appropriate CMakeLists.txt.

I'm a bit surprised how simple it was to add a non-SDL2 build with not too many changes (also a nice base now for other platforms) as I didn't intended that use-case when writing the file...

Biggest changes to the CMake file is the new "PLAYER_TARGET_PLATFORM"
Almost everything else is just adding tabulators because code was moved into if() but this renders badly in the diff viewer :/.

@Ghabry Ghabry added the Building label Oct 29, 2018

@Ghabry Ghabry referenced this pull request Oct 29, 2018
0 of 4 tasks complete

@Ghabry Ghabry force-pushed the libretro:libretro_upstreaming branch from c284aab to dfd618f Oct 30, 2018

@Ghabry

This comment has been minimized.

Copy link
Member Author

commented Oct 30, 2018

updated the readme with minimal instructions

@Ghabry Ghabry referenced this pull request Nov 19, 2018

@Ghabry Ghabry force-pushed the libretro:libretro_upstreaming branch from dfd618f to 2a2b736 Nov 20, 2018

@Ghabry

This comment has been minimized.

Copy link
Member Author

commented Nov 20, 2018

should be ready now.

src/player.h Outdated Show resolved Hide resolved

@Ghabry Ghabry force-pushed the libretro:libretro_upstreaming branch 3 times, most recently from 871a0b0 to 864838f Nov 20, 2018

@carstene1ns
Copy link
Member

left a comment

Just had a quick glance, will look into this some more later.

src/player.cpp Show resolved Hide resolved
src/audio_libretro.h Outdated Show resolved Hide resolved
src/audio_libretro.cpp Outdated Show resolved Hide resolved
#else
# define Offset off_t
# define EasyRPG_Offset off_t

This comment has been minimized.

Copy link
@carstene1ns

carstene1ns Nov 21, 2018

Member

This is very ugly (also the other type bending in filefinder for PSVita). I hope we can change this to use the same types later and get rid of all these Sce stuff. The vitasdk should really abstract this away in their c library.

This comment has been minimized.

Copy link
@Ghabry

Ghabry Nov 21, 2018

Author Member

should be done in one go with the AMIGA symbol mess :/

README.md Show resolved Hide resolved

@Ghabry Ghabry force-pushed the libretro:libretro_upstreaming branch 3 times, most recently from 079663e to 04f0582 Nov 21, 2018

@Ghabry Ghabry force-pushed the libretro:libretro_upstreaming branch from d5d71f5 to 534fedc Apr 18, 2019

return false;
}

extract_directory(parent_dir, game->path, sizeof(parent_dir));

This comment has been minimized.

Copy link
@Ghabry

Ghabry Apr 18, 2019

Author Member

This crashes on Windows because path in game->path is not UTF-8. Don't tell me they use the local Codepage here m(

This comment has been minimized.

Copy link
@Ghabry

Ghabry Apr 18, 2019

Author Member

Nevermind, the path is UTF-8 encoded but liblcf can't handle such paths on Windows. In the GameBrowser we work around this with "SetCurrentDirectory" but I have a proper solution for this in my VFS branch, so I won't fix this. This is only a problem for the filename of the main game folder (where liblcf must have access to).

@Ghabry

This comment has been minimized.

Copy link
Member Author

commented Apr 23, 2019

This is also ready now. There were no new crashes or anything reported by libretro users.

The Player still crashes on Android when closing but I blame RetroArch for this, even my start/stop simplification didn't help here.

@Ghabry

This comment has been minimized.

Copy link
Member Author

commented Apr 26, 2019

I have an idea how to fix the audio thread crashes on core shutdown on Windows and Android (these are RetroArch bugs because they call code in the unloaded dll/so) but I need to wait for there awesome buildbot infrastructure first before I can test it ^^'

@Ghabry

This comment has been minimized.

Copy link
Member Author

commented Apr 27, 2019

okay the workaround works, will only update libretro-common to the latest submodule version, then we are good to go.

libretro: Workaround Audio crash on shutdown in Win & Android, Rename…
… Keyboard config -> Input, update submodule

@Ghabry Ghabry force-pushed the libretro:libretro_upstreaming branch from 73e4cd5 to c09b4f8 Apr 27, 2019

src/libretro_ui.h Outdated Show resolved Hide resolved
src/player.cpp Outdated Show resolved Hide resolved
src/system.h Outdated Show resolved Hide resolved

Ghabry added some commits Apr 28, 2019

CLI Parser: Don't use argc from CommandLineToArgvW, use the provided …
…argc instead, otherwise this mismatches when e.g. libretro uses (nullptr, 0) and crashes.

Remove dead code
Fix memory leak, the Windows CLI was not deleted
@carstene1ns
Copy link
Member

left a comment

I do not really like it, but i guess i cannot defend Player anymore 🤐

@carstene1ns carstene1ns merged commit c4d475c into EasyRPG:master May 11, 2019

7 checks passed

Android (armeabi-v7a) Build finished.
Details
GNU/Linux Build finished.
Details
OSX Build finished.
Details
Wii (SDL1) Build finished.
Details
Windows (x64) Build finished.
Details
Windows (x86) Build finished.
Details
web Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.