Banderi / Ozymandias

An open source re-implementation of Pharaoh (1999) in the Julius/Augustus engine
GNU Affero General Public License v3.0
110 stars 10 forks source link

Adapt CMake to import SDL2_image on windows #11

Closed SandroWissmann closed 1 year ago

SandroWissmann commented 1 year ago

I tried to adapt cmake/FindSDL2_image.cmake and CmakeLists.txt in main folder so it can properly find SDL2_image.

I also put the SDL_Image in the SDL2 image:

Screenshot_20220920_165939

Now I tried to build on windows with:

mkdir build
cd build
cmake .. -G "MinGW Makefiles"

Which untortunately still givesd us this error: Screenshot_20220920_170132

SandroWissmann commented 1 year ago

Do you maybe have an idea @crudelios ?

SandroWissmann commented 1 year ago

I tried the change but still gives this:

Screenshot_20220920_180824

crudelios commented 1 year ago

That means it found the headers and not the libraries. It's an improvement!

I'll see what else needs doing.

SandroWissmann commented 1 year ago

It works now. I deleted the build folder and added one by one under ext/SDL2 the files.

I then could just run

cmake .. -G "MinGW Makefiles"

@Banderi Can you check out this branch and try to build like this:

Download: https://github.com/libsdl-org/SDL_image/releases/download/release-2.6.2/SDL2_image-devel-2.6.2-mingw.zip

unpack it under ext/SDL2

probably you need to do the same for SDL2 and mixer.

In the end it looks like this: Screenshot_20220920_185939

then delete your build folder

then

mkdir build cd build cmake .. -G "MinGW Makefiles"

It should work now out of the box without specify LIB.

SandroWissmann commented 1 year ago

I would also add one more commit to describe these steps in the readme so other people can get it to tun. For now it looks like we have these both ways. Still no clue why it still does not work with the lib way...

SandroWissmann commented 1 year ago

actually now both ways work after delete clean the build folder:

Either with

cmake -DCMAKE_PREFIX_PATH=LIB64 -DYSTEM_LIBS=false .. -G "MinGW Makefiles"

or just cmake .. -G "MinGW Makefiles"

with the fixed branch. if you supply in ext/SDL2 all SDL2 Mixer and Image.

The question which way we want to use here. Or just keep both for now and I will document it.

Banderi commented 1 year ago

That works too for me! Better to have it all in ext as it was originally with the other libs, so I can finally remove the LIB folder. I'll test asap!

SandroWissmann commented 1 year ago

Ok I will just add one more commit to update the readme how to build under windows / linux for other people who maybe want to build it and then from my site we could merge this. I will let you know.

SandroWissmann commented 1 year ago

Ok I added one more commit with instructions how to build in the Readme. Please check it. Then you can merge it from my point of view.

Banderi commented 1 year ago

I'm testing the files with the ext folder procedure (might need to clean up the README typos a bit btw, the links are also broken!), but it can not find SDL2_image for some reason. The rest - at least SDL2 related libs - it finds just fine: image Downloaded from here and all together is as such: image

SandroWissmann commented 1 year ago

stupid question.

Are you on the branch windows-sdl2_image-fix?

There the image cmake should be updated now and make it work.

Also delete the build folder complete and try again. The error looks exactly like the one I had before I deleted it.

I can check fix it maybe today afternoon.

SandroWissmann commented 1 year ago

My bad I forgot to push the change

GET_SDL_EXT_DIR(SDL_EXT_DIR "image")

@crudelios suggested.

I force pushed it now. You can fetch the whole branch again.

Probably you have to reset it from origin like this:

git checkout master git pull git checkout windows-sdl2_image-fix git reset --hard origin/windows-sdl2_image-fix

and then try out again. Hopefully it works. The readme I fix then tonight after work.

Banderi commented 1 year ago

Cleaned up and pulled, now it finds everything and CMake generates correctly, however the build fails as it can not find SDL_image.h: image image SDL_mixer.h and the rest are found and included correctly where needed.

And no worries about the README, I will also update the build instructions myself after merging!

Banderi commented 1 year ago

Yep, now it works! -- note also that with newer SDL version it will fail to build because I haven't fixed the USE_RENDER_GEOMETRY functions, so just to quickly compile for the moment you need to disable it.

SandroWissmann commented 1 year ago

Maybe you can just add these changes to make it work to the branch. If not I can also add them when fix the readme later today.

Banderi commented 1 year ago

I was actually curious if that could be done. Github shows "Add more commits to this pull request" but I thought just as you can't push to my master, I couldn't push to your fork/branch (unless permissions allowed it I imagine). What's the correct way to do it?

EDIT: Nevermind, I guess Github automatically gives permission to a PR branch. That's neat to know! If you don't mind, I'll go ahead and fix the README as well right away!

SandroWissmann commented 1 year ago

Ok no problem for me. I will try out the branch again with your fix if it builds and after that I think you could merge it.

SandroWissmann commented 1 year ago

Ok I can confirm that building works now with your last commit. Just the readme needs to be still fixed for the links not working.

Doi I do it or do you want to do it. There are just two extra "." in two links which need to be removed then it should work.

Banderi commented 1 year ago

Yep I'm fixing it right now, also cleaning up the syntax/grammar and expanding the instructions for the needed dynamic libraries required for running the engine. Will merge as soon as I'm done!

SandroWissmann commented 1 year ago

perfect after this is done i will build my other pull reqeusts and also try them out on windows.