Addition of OSX compilation support #130

Closed
Socapex opened this Issue Feb 23, 2013 · 32 comments

2 participants

@Socapex

I am now confident that the ValyriaTear.xcodeproj is safe to remove. I have created a new /Xcode/ValyriaTear.xcodeproj. This folder will eventually include everything needed for a release build (dmgcanvas etc.) so any Mac developer can hop in easily.

If this isn't the way you like to work with platform specific projects, please advise otherwise (and let me know exactly what you prefer).

P.S. My project doesn't even try to build the editor, but since it uses QT it shouldn't be too hard to add down the road. I also haven't tried to integrate translations, but I will definitely get to them since this project was extremely easy to port (allacrost is already multi-platform).

@Bertram25
Valyria Tear member

Hi Socapex,

Creating a folder for that is perfectly fine for me :) And yeah, the editor can wait.

As for the color problem, you might have to make a fix there (I suppose it's a problem linked with the endianess? And we both might want to test the fix against both kind of architectures):
https://github.com/Bertram25/ValyriaTear/blob/master/src/engine/video/image_base.cpp#L117

Please, make a merge request when you ready, and we'll close this issue at that moment.

Thanks a LOT for the OSX port and for the work done.

Best regards,

@Socapex

Hey Bertram. The image bug was fixed with commit 111575c (I updated image_base to current Allacrost version and also updated it to libpng15). All is working perfect now. Knitter on your forums is testing it out and he seems happy. All is looking good for a release pretty soon ;)

@Bertram25
Valyria Tear member

Hi Socapex, cool to hear :)

Yet, well, to me image_base.cpp in allacrost isn't forcefully in a better shape than it is for Valyria, IMHO.
Please:

  • Don't use tabs for indentation, use four spaces instead,
  • Could you apply only the needed changes in image base? Not copying the file as a whole as it seems you did in this commit?

I'm trying, as everyone else in is own project, to keep it in a sane and consistent state, and this is not making your work mergeable atm, unfortunately.

Thanks a lot and best regards,

@Socapex

Hmmm, it seems my message wasn't saved. Anyways:

When I added image_base from Allacrost, it was more of a test to see if the issue was the png loading. I have now confirmed that. Unfortunately I am not sure exactly where is the problem, maybe you have a better idea?

I was expecting a merge nightmare, since I also updated luabind to git version (maybe you could try and see if that breaks anything for you, since the 0.9.1 version cannot work on OSX). When everything is ready, I'll rebase and send you a pull request.

@Bertram25
Valyria Tear member

Hi Socapex, :)

When I added image_base from Allacrost, it was more of a test to see if the issue was the png loading. I have now confirmed that. Unfortunately I am not sure exactly where is the problem, maybe you have a better idea?

I'll make sure to see the difference between the two version with the assurance it must come from the loadImage() function since the color channel are clearly inverted in your screenshot. :) I'll handle it on master first if you want.

I was expecting a merge nightmare, since I also updated luabind to git version (maybe you could try and see if that breaks anything for you, since the 0.9.1 version cannot work on OSX). When everything is ready, I'll rebase and send you a pull request.

Hmm, I also updated the given luabind version to the git one, but only merging what was to be merged. I don't think the difference between the git version and mine should be that big. Where can I see that?

In any case, I owe you a big one for porting VT on OSX. Thanks a lot!

@Socapex

So I can use your luabind then? It'll make things much more easy, since I can simply fix the image_base if your repo has git luabind :)

Edit: I'll re-clone your repo once you upload a luabind update, and work from that fixing only the correct parts to image_base. Then maybe some translation testing (sigh, headaches) ;)

@Bertram25
Valyria Tear member

@Socapex

So I can use your luabind then? It'll make things much more easy, since I can simply fix the image_base if your repo has git luabind :)

Yep, normally, I've locally done the job :)

Edit: I'll re-clone your repo once you upload a luabind update, and work from that fixing only the correct parts to image_base. Then maybe some translation testing (sigh, headaches) ;)

upload a luabind update? Normally, I don't need to, could you tell me what's preventing you from using it?

As for image_base, it shouldn't be that hard, don't worry, but you should get some Halloweenish results along the way XD

@Bertram25
Valyria Tear member

Hi @Socapex :D

I've rechecked a bit more thoroughly the changes related to OSX porting on your own branch. From what I could see, only the image_base.cpp changes and luabind ones aren't trivial.

From what I could see in luabind, most of the changes were:
1. Added useless C++0X compilation options <-- I'm sure we don't need that.
2. Added changes in the examples and a few regressions from the current luabind in the master repo.

On the counter side, the changes made related on the XCode files changes are interesting and should be merged in master.

Thus, could you do another attempt and do those steps:
1. Make a commit adding the XCode related files and changes.
2. Another commit for the static libs alone.
3. Try to compile using the luabind version provided in master without changing it. If it can't be compiled, I'd like to see the log as I'm quite sure the changes needed in luabind to be able to compile on OSX are trivial.
4. Try to tell me whether your system is big endian or not and tell me there in the code in which order the pixels should be assigned to get a correct loading of images data:
https://github.com/Bertram25/ValyriaTear/blob/master/src/engine/video/image_base.cpp#L117

I'm looking forward to your answer and hope you're going well :)

Best regards,

@Socapex

Hi @Bertram25,

I was going to write to you soon :) My semester is ending so I am in exams right now. I plan to get back to the port this summer! Here is what I had planned:

I'll re-clone and checkout your latest tag. First, I'll work with your version of luabind, and open up a new issue so maybe you can help me figure out what was the problem/s. Then I'll figure out what order is needed for the pixels (as far as I know, OSX is little endian since they've switched to intel…).

So keep an eye open, I'll be coming back with some compile problems lol. I'll also move my current branch to an OSX_backup, since it is actually working :)

See you soon,

@Bertram25
Valyria Tear member

Hi @Socapex

Good luck for your exams. Thanks for the news, I'm sure you'll make it. My only advice will be to properly separate commits with what you're sure of and what you're less of, so I can merge all that back step by step and ease your next ports. See you this summer! 8)

@Socapex

Well well well, had 2 seconds. Seems luabind problems where simple namespace issues. I will send a pull request for you to review the small changes I made.

@Bertram25
Valyria Tear member

luabind part merged 👍

A path issue and the pixel problem are left to add. Thanks a lot @Socapex :D

@Socapex

Hi @Bertram25,

So I've made quite some progress and the ValyriaTear version for 10.7 and 10.8 is ready with translations :) Just send me emails when you have new releases (with the tag) so I can build them for mac. If you have an a server I could upload them there using SSH or I can send you Dropbox links (we can discuss all this by email).

Unfortunately, the 10.6 version will have to wait, as there is no define to enable translations. So, since I don't want to start breaking everything, do you think you could add a define (or ndefine) to disable calls to the libintl library? I could do this, but I think you are better postitioned to make the right decision.

Edit: All I see are 5 calls inside system.cpp

Have a good day,

@Bertram25
Valyria Tear member

Hi @Socapex :)

Thanks a lot for the quick and efficient work done on this! 👯

I'll tag a new intermediate release, hopefully in a few weeks, I'll tell you at that time when I'm ready (and I'll make RC btw) :)
As for the release packages, I can host them on sourceforge and indidb, so we can discuss how you give them to me before I put them in place if you want.

I'll sum up here all the remaining questions I've got:

  • What is the difference between the defines APPLE and MACH, do we need both? I assume the MACH is the old call of APPLE but there might be something more subtle, so... any idea?

  • Is it enough if I add a compile flag to disable l10n?

  • As for the minimap, if the problem is a pixel color problem, the solution might be simple and lying here:
    https://github.com/Bertram25/ValyriaTear/blob/master/src/modes/map/map_minimap.cpp#L154
    If it's not that, maybe you can provide a screenshot showing the problem (so I can test it's OS specific, for instance)?

Anyway, a BIG thanks for the help provided.

@Socapex

Hi @Bertram25,

I guess I should have updated my port earlier, but I thought it would be very long and complicated... I guess I learned a few things this year ;)

As far as I can tell, there is no difference between MACH and APPLE. I personnally prefer APPLE because it is more intuitive, and not everybody knows about the Mach layer in the Darwin kernel. If you don't mind, I'll push a commit to replace MACH with APPLE.

A compile flag is perfect. Anything works really :)

Here is a screenshot:
Untitled

It looks like the screenshot on your blog. I thought it was broken, but basically I think you haven't implemented the tiles yet?

Anyways, see you around

@Bertram25
Valyria Tear member

Hey :)

As far as I can tell, there is no difference between MACH and APPLE. I personnally prefer APPLE because it is more intuitive, and not everybody knows about the Mach layer in the Darwin kernel. If you don't mind, I'll push a commit to replace MACH with APPLE.

Np :) Go ahead!

A compile flag is perfect. Anything works really :)

Ok, I'll do it.

It looks like the screenshot on your blog. I thought it was broken, but basically I think you haven't implemented the tiles yet?

Or found the right texture to apply there instead of this dirty one. The idea is not to reproduce the map tile per tile, but to give its collision outline. If it's working, it's already great to hear. :D

@Bertram25
Valyria Tear member

Update: I've seen according to your screenshot that you're using normal map tiles. Try to go in the video settings and use smoothed tiles. It should improve the map mode.
(Maybe I should even remove this 'feature')

@Bertram25
Valyria Tear member

Hey @Socapex :)

Disabling translations support has been added in: ac770bf

I can also change MACH to APPLE if you want me to.

Btw, is there anything left to add/remove for OSX support to be ready?

@Socapex

Hi @Bertram25,

Great to hear! I'll work on the 10.6 version later today. I pushed a commit replacing MACH already ^_^. Maybe you can pull it when I push my changes for the 10.6 build? I'll send you an email with links for the downloads, and update the forums once everything is ready :)

@Bertram25
Valyria Tear member

\o/

@Socapex

Hi @Bertram25, here are the releases :D

Lion: https://www.dropbox.com/s/8gm107qlq76mwzz/ValyriaTear-lion-2013_04_24.dmg
Snow-Leopard: https://www.dropbox.com/s/hne3nl5acjeuezv/ValyriaTear-snow-2013_04_24.dmg

There may be a crash for people who played with the last version though (I am not sure if it will reproduce on the release version). The solution is to clear the application data (the crash is because of the minimap addition). Users can use a free tool like AppCleaner to do so. But unfortunately will loose saves... I am not sure of the save game location, maybe we could have users back it up and re-add it if they encounter the crash?

@Bertram25
Valyria Tear member

Hi @Socapex

I'll upload the links and make a piece of news tomorow :D

As for the crash, can you get a backtrace or something and why not open an issue about it?

I'm wondering whether the crash is happening because IIRC a temp image is created. Does this explains the crash, for instance?

@Bertram25
Valyria Tear member

Hi Again @Socapex :)

Unfortunately, I just found three bugs that could be blockers for players willing to test the game.
I'll need to make a full play through again but at least I'm sure people can go 'til the 1st dungeon end now.

Could you do new packages now those bugs are out of the way?

@Socapex

No problem. I'll send you the links when its ready :)

@Bertram25
Valyria Tear member

Hi again :)

Sorry, I know I'm demanding but I guess people will prefer a finisheable version.

Thanks again!

@Socapex

Hi @Bertram25, what do you mean by a finisheable version? I don't really understand...

@Bertram25
Valyria Tear member

The dungeon couldn't be finished before those bugfixes. That's why I asked for the new packages. Have anice evening! :)

@Socapex

So do you still want new builds? Or you want me to wait?

@Bertram25 I'm on IRC @evol-dev if you want to chat :)

@Bertram25
Valyria Tear member

Nope, you can create new builds, it should be ok now, thanks :)

@Socapex

Hi @Bertram25 , no problem. I haden't had time to build them anyways :)

Lion: https://www.dropbox.com/s/h1aef2rw1pm09vz/ValyriaTear-lion-2013_04_27.dmg
Snow Leopard: https://www.dropbox.com/s/o2w851d5valaw5g/ValyriaTear-snow-2013_04_27.dmg

I guess you can close this thread now, as ValyriaTear is now ported to OSX :D In the future, just send me an email with the tag you want me to build. And I'll send you links for the builds. Please make sure the tag is ok so I don't have to build 2 or 3 times :P

Always a pleasure working with you, see you around :)

@Bertram25
Valyria Tear member

Please make sure the tag is ok so I don't have to build 2 or 3 times :P

Don't worry :)

Always a pleasure working with you, see you around :)

My pleasure! :D

@Bertram25
Valyria Tear member

I guess you can close this thread now, as ValyriaTear is now ported to OSX

Closing!

@Bertram25 Bertram25 closed this Apr 28, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment