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

OSX crash fixes #24587

Merged
merged 2 commits into from Aug 5, 2018

Conversation

Projects
None yet
6 participants
@vircung
Copy link
Contributor

commented Aug 2, 2018

I went through OSX crashes issues as well as logs attached in comments.

Master branch manual builds are compromised when SOUND flag was set:
make app NATIVE=osx RELEASE=1 OSX_MIN=10.13 TILES=1 CLANG=1 LUA=1 SOUND=1
make app NATIVE=osx RELEASE=1 OSX_MIN=10.13 TILES=1 CLANG=1 LUA=1 SOUND=1 FRAMEWORK=1

Builds without sound enabled worked just fine. That made me dig through ``makefileand lead to discovery thatSDL2_mixer.framework``` and it's content was copied over bot not linked nor included.

After changes flags FRAMEWORK and SOUND build doesn't result with compromised Cataclysm.app build. Moreover build result with omitted FRAMEWORK is suitable for installation via homebrew. Would be great

Fixes #23927, #23171, #24588

vircung added some commits Aug 1, 2018

Add support for sounds from libSDL2 to OSX
Link SDL2_mixer and copy over .dylib
Add support for sounds from SDL2 to OSX
Link and include missing SDL2_mixer.framework
@narc0tiq

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2018

Good find! The whole osxcross setup is foreign to me, so I didn't even have a way to debug it -- thanks for taking the time to look into it.

@vircung

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2018

Actually it's more native osx build. Not sure yet if it'll fix issue with osxcross but for sure this PR affects builds with flag NATIVE=os.
Was kind of late yesterday so I'll try to verify cross build.

@narc0tiq

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2018

It should work, in as much as the osxcross environment seems to have the SDL2_mixer.framework and its dependencies Ogg and Vorbis. Beyond that I couldn't say either, but if it helps, here's the build script that Jenkins uses; let me know if there are any changes I should make there -- the experimental OSX builds have been broken for a long time.

@vircung

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2018

As far as I can tell mainline-build.zsh and Frameworks.tar.gz from oxcross directory looks fine and dandy. We can give it a try. Jenkins didn't pass build for this PR because of diff in Makefile 😕

@Inglonias

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2018

I must stress that if this PR works, it should be merged ASAP, as this is one of the issues mentioned in the 0.D release project. I don't have access to an OSX computer for testing, or I'd test this myself...

@vircung

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2018

@Inglonias I'm on OSX so that's one of reasons that I've managed to "fix it locally".

Here dropbox directory with both libSDL (no FRAMEWORK=1 flag) and Framework (FRAMEWORK=1 flag) builds with sound for testing.

@vircung

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2018

@Leland

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2018

You sure this closes #23839?

problem: Game crashes when I attempt to switch tile pack/zoom in

@vircung

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2018

@Leland You're correct, it's doesn't. Sorry for confusion.

@vircung

This comment has been minimized.

Copy link
Contributor Author

commented Aug 5, 2018

Is there something that this PR is missing to be merged?

@Night-Pryanik

This comment has been minimized.

Copy link
Member

commented Aug 5, 2018

@vircung only lack of devs' free time is preventing this (and other) PRs from being reviewed and merged.

@ZhilkinSerg ZhilkinSerg merged commit 87fc4b7 into CleverRaven:master Aug 5, 2018

3 of 4 checks passed

gorgon-ghprb Build finished.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.2%) to 23.853%
Details
@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2018

Fortunately I have some time now.

@vircung vircung deleted the vircung:osx-build-fixes branch Sep 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.