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

Fix cutscene only showing a grey screen on macOS. #533

Merged
merged 2 commits into from Jun 26, 2018

Conversation

Projects
None yet
4 participants
@christoph-heiss
Copy link
Contributor

christoph-heiss commented Jun 26, 2018

[Fixes #378]

Since the author of #509 apparently went MIA, I though I would quickly fix this up since I own a macOS machine.

After some debugging across two machines, I found the root cause.
Basically the issue was caused by a difference how the different C++ standard library implementations process formatted input.

Here is a minimal reproducer of the problem:

#include <iostream>
#include <sstream>
int main()
{
        float f;
        std::stringstream{"1234f"} >> f;
        std::cout << f << std::endl;
        return 0;
}

On a macOS machine, this returns 0. On a linux box, 1234 (as the previous code assumed).

I also included a small build failure fix when using a combination of gcc/clang and the different filesystem libraries, which I had laying around for a while now.

christoph-heiss added some commits Jun 26, 2018

rwengine: Fix cutscenes only showing a grey screen on macOS.
This is caused by different handling of formatted input in the C++
standard library on macOS than on a linux box.
On linux, characters after a number are ignored (such as 'f' in this
case), on macOS this leads to an error on the stream.

@christoph-heiss christoph-heiss force-pushed the christoph-heiss:fix-cutscenes-macos branch from 768bfce to 359d61e Jun 26, 2018

@JayFoxRox

This comment has been minimized.

Copy link
Collaborator

JayFoxRox commented Jun 26, 2018

Skimmed over code, LGTM. Did not test it yet.

I don't fully understand our cmake_configure.cmake (for stdlib, this looks too compiler specific), but I'll assume the changes are good, too.

@madebr

This comment has been minimized.

Copy link
Collaborator

madebr commented Jun 26, 2018

LGTM, is the CMAKE_CXX_COMPILER_ID on mac equal to Clang?
Can the test for AppleClang be removed from line 8 from cmake_configure.cmake?

@madebr

This comment has been minimized.

Copy link
Collaborator

madebr commented Jun 26, 2018

That would be nice. Since the topic is Apple. 🍏

@christoph-heiss

This comment has been minimized.

Copy link
Contributor

christoph-heiss commented Jun 26, 2018

No, it is AppleClang (at least for the system compiler provided by XCode).

For clang installed by Homebrew, it is just Clang (which is an obscure edge-case).
I came across this when I test-compiled using g++ on macOS.

(Apple really has some platform-specific weirdness.)

@madebr

This comment has been minimized.

Copy link
Collaborator

madebr commented Jun 26, 2018

Thanks for verifying!
Did not know this.

@JayFoxRox

This comment has been minimized.

Copy link
Collaborator

JayFoxRox commented Jun 26, 2018

@madebr review sounds like it only reviewed the code, much like mine.
Aside from @christoph-heiss , did any other macOS user (and at least one gcc Linux user) run this for testing?

I'll probably hit merge once CI passed, if we have confirmed these touched platforms still run fine.

@madebr

This comment has been minimized.

Copy link
Collaborator

madebr commented Jun 26, 2018

Just tested it on linux, gcc 8. rwgame still works, rwtests shows no difference in behavior.

@tsjost

This comment has been minimized.

Copy link
Member

tsjost commented Jun 26, 2018

Tested on MacOS 10.13.4 with "Apple LLVM version 9.1.0 (clang-902.0.39.1)"; current master 7061813 indeed has broken initial cutscene, while this patch fixes it 👍

@JayFoxRox JayFoxRox merged commit 2b59f8b into rwengine:master Jun 26, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@christoph-heiss christoph-heiss deleted the christoph-heiss:fix-cutscenes-macos branch Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment