search

mupen64plus / mupen64plus-video-rice

Video plugin for the Mupen64Plus v2.0 project, using OpenGL. This plugin is based on the RiceVideoLinux plugin from earlier versions of Mupen64Plus.
31 stars 40 forks source link

Fix build error in Android. #54

Closed littleguy77 closed 9 years ago

littleguy77 commented 9 years ago

This fixes the paths to match those used in the header, BMGUtils.h. Not sure how this was even building on the other platforms.

Narann commented 9 years ago

Seems fine to me but I'm very surprised it didn't break before. I would wait for other review of this. Maybe it's related to Windows or OSX, I don't know.

littleguy77 commented 9 years ago

Sounds good, thanks.

richard42 commented 9 years ago

This is going to break the Win32 build, and maybe linux too. I just changed this file last weekend, when I realized that MSVC apparently doesn't include the current directory of the file being compiled in the list of include directories. I had to add "../../src" to the include directory list in the project file, and change these paths to be relative to the "src" folder.

I see that the Unix makefile also includes "-I../../src" in CFLAGS, so this file will compile correctly with the current paths. Does the Android build not use the unix Makefile? If not, can you fix up your build system to add the "src" folder in the list of include directories?

littleguy77 commented 9 years ago

I just tested, this builds fine in Visual Studio 2013. Travis CI also passes. The proposed change actually makes the path syntax consistent with all the include declarations expressed elsewhere in the project.

littleguy77 commented 9 years ago

I see that the Unix makefile also includes "-I../../src" in CFLAGS, so this file will compile correctly with the current paths. Does the Android build not use the unix Makefile? If not, can you fix up your build system to add the "src" folder in the list of include directories?

Android uses its own unix-like makefile system, but you're right. We were using /src/liblinux in the list of include directories rather than /src. When I make that change, then everything builds fine and this PR is not necessary.

Still, for path declaration consistency you might consider merging this PR anyhow.

Narann commented 9 years ago

So, what you suggest is modify unix Makefile then merge this PR?

littleguy77 commented 9 years ago

Sorry, by "we" I meant mupen64plus-ae. No changes are necessary to the Unix makefile in mupen64plus-video-rice. I already made the change in the mupen64plus-ae repository, so this PR can be closed and does not need to be merged. I was just suggesting you might want to merge this PR anyhow because it makes the include declarations consistent between all files in the video-rice project.

Narann commented 9 years ago

lol thanks littleguy77 ^^

Still, for path declaration consistency you might consider merging this PR anyhow.

I agree. @richard42 state this could potentially break Win32 build but you state it didn't I will let it here, let Richard prepare the release and as soon the release is released (:D) I will merge it.

richard42 commented 9 years ago

ok I buy the consistency argument. I'll fix the msvc2013 project file if necessary.