WinRT, MSVC compilation

[albertofustinoni]

Opening this to have a frame of reference for discussion.

I am attempting to get this to compile in visual studio, but am having trouble with burn.cpp: more specifically, TCHAR being ifdef's as char (in burner/libretro/tchar.h) when it would be a wchar_t is causing errors: my issue is that I cannot understand the meaning behind doing so, since burn.cpp uses both chars and wchar_t, as does the rest of the codebase.

There are also instances of #include<tchar.h> and #include "tchar.h", which always end up referencing the tchar.h included in the project's source tree. I would assume this is the intended effect if not for burner/libretro/tchar.h in having #include<tchar.h>, which makes no sense if that is the case.

Can someone help/explain?

[inactive123]

Inviting @barbudreadmon here since he more or less maintains this core.

[albertofustinoni]

@twinaphex Thank you very much

[barbudreadmon]

I'll take a look this week-end

[albertofustinoni]

@barbudreadmon Thank you for your help :)

[barbudreadmon]

• The attribute optimize thing not building on msvc makes sense
• Using fseek instead of rewind makes sense (i noticed upstream replaced most of their rewind already, except the one in hiscore.cpp and one another in neocdsel.cpp, so perhaps there is a reason, i'll ask)

What about your remaining "fixes" ? Could you confirm they are necessary for fbalpha-libretro to work with WinRT and msvc ? Most of them doesn't feel like it to me.

[barbudreadmon]

I'm also wondering, is megadrive emulation working with your fix ?

[albertofustinoni]

@barbudreadmon Are you in libretro's IRC now? Maybe easier to discus in real time there

As for the edits I made:

• The new files added are libretro_comon code: I need that to make file IO work. fopen etc do not work in UWP/WinRT, so I have to virtualize the file system. To do so in a way that is standard across libretro cores and that will allow Retroarch to take advantage of this work too, I am routing file IO within the core through file_stream in libretro_common.
• Changes like turning

  struct retro_variable vars[nbr_vars + nbr_dips + nbr_macros + 1];

  into

  std::vector<retro_variable> vars(nbr_vars + nbr_dips + nbr_macros + 1); 

and

normal_input_descriptors.push_back((retro_input_descriptor){ port, device, index, id, description }); 

into

retro_input_descriptor descriptor{ port, device, index, id, description }; 
normal_input_descriptors.push_back(descriptor); 

Are needed to make the code compile in Visual Studio

As for Megadrive emulation, I cannot get the core to compile right now because of the TCHAR issue from the first post, so I don't know.

[barbudreadmon]

Removing the TCHAR define and adding the following to the else part of the msvc condition in tchar.h would make sense :

ifdef _UNICODE

typedef wchar_t TCHAR;

else

typedef char TCHAR;

endif

I don't know much about this part of the fbalpha-libretro code, it was already there when i took over the project.

[barbudreadmon]

I can't get the libretro-common files (file_stream.c and file_stream_transforms.c) to build, do you have the same issue ?

[albertofustinoni]

No, those files work fine for me, in this as well as other cores.

The build issues I am having all originate from char and whcar mismatches.

From: barbudreadmon Sent: 02 September 2017 01:27 To: libretro/fbalpha Cc: Alberto Fustinoni; Author Subject: Re: [libretro/fbalpha] WinRT, MSVC compilation (#152)

I can't get the libretro-common files (file_stream.c and file_stream_transforms.c) to build, do you have the same issue ? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[barbudreadmon]

How are you building this ? You didn't add file_stream.c and file_stream_transforms.c to the SOURCES_C in makefile.libretro_common, and you didn't include src/burner/libretro/libretro-common/include/ to the include dirs either.

[barbudreadmon]

Could you tell me on which project did you see a working implementation of file_stream.c and file_stream_transforms.c ?

I could find a few references to filestream.c in a few other projects, but without any reference to the actual filestream* functions in their code. I found no file_stream_transforms.c in other projects. Are you sure those functions are needed ? Did you try to build this project with USE_FILE32API set or unset ?

[albertofustinoni]

There are a few cores using file_stream and file_stream_transforms now:

• GenesisPlusGX
• Beetle family of cores (PSX, NeoGeoPocket, Wonderswan, PCEngine, PCFX)
• MelonDS

The code is also in libretro_common

You won't see the functions in file_stream_transofrms called directly since the purpose of that file is to #define fopen() and similar to use file_stream. This is both easier and less intrusive than changing each function call individually.

As for how I compile the code and why the discrepancies in the makefile, I work in Visual Studio and replicate what I see in the makefiles in my own Visual C++ 2017 projects. I usually wait to update the makefiles until I got something that I know works and open PRs for merging - this is still a work in progress (why I am asking for help to begin with) so I haven't done it yet.

For FBA, I am including building withthese pre-processor defines

LSB_FIRST
FRONTEND_SUPPORTS_RGB565
PTR64
__LIBRETRO__
__LIBRETRO_OPTIMIZATIONS__
USE_SPEEDHACKS
WANT_NEOGEOCD
NDEBUG
_WINDLL

[barbudreadmon]

You need the USE_FILE32API preprocessor define, file-stream and file_stream_transforms are unneeded, fopen/fseek/ftell are available to your platform, fceumm/nestopia/snes9x use them and are available for your platform.

[albertofustinoni]

Ok, I'll add USE_FILE32API to my pre-processor defines.

file_stream and transforms are very much needed: while fopen/fseek/ftell are available in UWP they only work to open files inside the app's install folder, which is read only. Everything else in the filesystem is only available via the WinRT file IO APIs.

I have had to create an implementation of file_stream that does just that, and which I substitute to the one you see in the repo in my own builds. Libretro's maintainers are also interested in adding UWP as a platform and we have agreed it is going to be easier for everyone if file access within cores is routed through file_stream.

The reason why I got fceumm/nestopia/snes9x to work without this is that they can accept game rom data as an in-memory array, avoiding having to access the file system at all that way.

[barbudreadmon]

I'm done for now, i commited part of your changes, what i didn't :

• Your c++11 code in libretro.cpp
• I made your calls to file_stream_transforms optional, they break vita/android/wiiu, but i introduced a new preprocessor define, so you can try setting USE_LIBRETRO_FILE32API (it still needs USE_FILE32API) in your msvc project file.
• I removed all *printf functions from file_stream/file_stream_transforms, they break build for me, there is probably a reason why they are not included in all the projects you mentioned.

[inactive123]

@barbudreadmon With regards to breaking Vita/Android/WiiU, what do you mean by breaking? Do you mean it no longer compiles or that it no longer works at runtime?

[barbudreadmon]

They break building of ioapi.c. You can try setting USE_LIBRETRO_FILE32API=1 if you want the logs of the error (something about losing reference in fclose iirc). I think we'll need to write some custom ioapi.c/ioapi.h files for libretro-fbalpha and blacklist the normal ones if we want to use the file_stream/file_stream_transform libraries.

[barbudreadmon]

@twinaphex @albertofustinoni : i took a closer look at the file_stream_transform files and the issues i got from them, and i have a few questions :

• Why is there no drop-in replacement for "ferror" ? I think trying to pass a RFILE to the ferror of stdio is basically what break building of fbalpha-libretro on android/vita/wiiu.
• Why is the second argument of rfopen a "char" and not a "const char" ? That makes the function not a real drop-in for fopen, and generate a lot of warnings.
• Why does the version of those files implementing the various *printf functions not include stdarg.h in file_stream.c ? It's needed for va_list and won't build on my gcc without it.

[barbudreadmon]

libretro-fbalpha needs to stay c++98 compatible, and as i mentioned previously, your changes on libretro.cpp breaks that, i won't accept changes that break building on systems that are not c++11 compatible.

Also, why do you change tchar.h's name ? There is no need.

[albertofustinoni]

What changes have I made that break C++98 compatibility?

I'm actually trying to go through your objections and fix what fix those issues.

As for tchar, I am renaming it because otherwise the system's tchar.h is never included. I don't know if FCC does things differently, but for me #include actually includes the one in the burner folder.

[albertofustinoni]

Never mind, I figured out what you were objecting to and changed it. Should be C++98 compatible now.

[barbudreadmon]

That's weird, unlike #include "tchar.h", #include should search for system header files first. If you can't get msvc's way of searching for header files right, you'll have to remove #include and implement your own code to replace it, probably something like :

ifdef UNICODE

typedef wchar_t TCHAR;

else

typedef char TCHAR;

endif

Should be enough, i don't think there is anything else needed from system's tchar.h .

The reason why i ask you this is that i intend to push codebase changes upstream once we fix your issues, so i don't want to change tchar.h's name (it would mean upstream changes for all frontends).

[barbudreadmon]

I did some search, it seems you are right, the header files including in msvc is kinda messed up.

[albertofustinoni]

OK, renamed fba_tchar to just tchar and removed the system's tchar reference from the one in libretro.

Project now builds for me. Does it work for you?

Since I have fixed the points you raised with filestream_transforms, can we remove the extra #USE_LIBRETRO_FILE32API and have all libretro builds use file_stream (like what other cores are doing)?

If there are issues to be found, it'd be best to find them earlier and fix them.

[barbudreadmon]

I think there is actually an issue with 7z support. I'll take a closer look when i get back from work.

[barbudreadmon]

I fixed a few build errors and commited. Let me know if i broke something on MSVC. Also, perhaps it would be a good idea to add INCLUDE_7Z_SUPPORT to your preprocessor defines.

[barbudreadmon]

It seems there is some libretro-common files for 7z support, shouldn't we use them ?

[albertofustinoni]

@barbudreadmon IT WORKS! Compiles AND runs - tested with a CPS1 and a NeoGeo game and they both seem to work great.

One extra change I made is to have the core fall-back looking for archives in the Libretro's system folder if they cannot be found in the rom folder. This is in keeping with the convention the other libretro cores follow (and on which I too depend) of locating BIOS files.

As for the 7zip files in libretro-common: I have no experience with them and do not know how much work it would be to integrate them. As for whether we should or not, I think we should ask @twinaphex

[albertofustinoni]

@barbudreadmon Thank you very much for merging this and for all the help you have given.

[barbudreadmon]

By the way, i had to undef a few things in file_stream_transforms.h, or building on some platform would throw tons of warnings. Also, i think you'll have to commit the msvc "makefiles" (*.vcxproj and such) if we want the buildbot to build the library.