[DONE] Clean up

Re: Clean up

Postby oln » 23 Jul 2011, 20:20

Then I think we should go with using the source, and somehow make it build automatically using the bundled cmake script when the game is built.
What projects did you look at? I saw Rigs of rods is using it, though it seems that in that game it has to be built and sorted manually to get the game to compile .
User avatar
oln
 
Posts: 1020
Joined: 26 Oct 2010, 22:16
Location: Norway

Re: Clean up

Postby svenskmand » 23 Jul 2011, 20:57

Given that I now feel pretty comfortable with the whole packaging thing I would not mind making a package for AngelScript. It would be allot easier if we plan to use every update to AngelScript they release, but if we do not do that then it does not really matter if we just copy it into our source directory. If you think we should always use the newest AngelScript version then we should have a package, else I it would not be necessary.
Jamendo.com - The best music store on the net, uses CC licenses.
User avatar
svenskmand
OD Moderator
 
Posts: 1850
Joined: 09 Dec 2009, 00:07
Location: Denmark

Re: Clean up

Postby StefanP.MUC » 23 Jul 2011, 21:04

@oln
I googled for angelscript includes, there were quite a lot of smaller projects. Warsow is also using it, to name another big example. On the AngelScript website there's also a list of projects using it (free and commercial ones).

@svenskmand
Yeah, I planned to always be up to date with AS. (actually I think we should do this with all our dependencyies if it's possible without breaking functionality)
StefanP.MUC
 

Re: Clean up

Postby svenskmand » 23 Jul 2011, 21:19

Then it will be a good idea to make a package for it, in my opinion. Else you would have to put it into our repository all the time when there is a change to their codebase. But you guys decide.
Jamendo.com - The best music store on the net, uses CC licenses.
User avatar
svenskmand
OD Moderator
 
Posts: 1850
Joined: 09 Dec 2009, 00:07
Location: Denmark

Re: Clean up

Postby oln » 23 Jul 2011, 21:43

A package would be nice as well. I couldn't find an existing one, so I guess it could be useful for other projects as well.
For windows we could then do as we have done with pthreads, put the compiled lib files and/or source in the dep folder.
User avatar
oln
 
Posts: 1020
Joined: 26 Oct 2010, 22:16
Location: Norway

Re: Clean up

Postby StefanP.MUC » 24 Jul 2011, 09:07

Sounds good.

So, if it's ok, I'll push the AngelScript package to /dep, and then oln prepares the cmake and package stuff?
StefanP.MUC
 

Re: Clean up

Postby StefanP.MUC » 24 Jul 2011, 10:54

Just pushed the changes (remove AS from include and src and put the full sdk including addons into /dep). It doesn't build now, because cmake doesn't know about angelscript. When building works again we'll be able to export some functionality to AngelScript scripting language.
StefanP.MUC
 

Re: Clean up

Postby StefanP.MUC » 24 Jul 2011, 20:42

Hope this is the correct way: I just build (MingW 4.5.2, Win32) and pushed libangelscript.a to the repository (its in dep/angelscript/angelscript/lib).
I'll try now and see if I can somehow get cmake to use it.
StefanP.MUC
 

Re: Clean up

Postby charlie » 24 Jul 2011, 22:42

I would just import AngelScript source in to SVN. It is designed to be used this way.

The problem with coming up with some automatic updating mechanism is that you may get caught out by upstream changes breaking things in subtle, unexpected ways and - if you are busy working on other challenges - this may lead to confusion.
Free Gamer - it's the dogz
Vexi - web UI platform
User avatar
charlie
Global Moderator
 
Posts: 2066
Joined: 02 Dec 2009, 11:56
Location: Manchester, UK

Re: Clean up

Postby StefanP.MUC » 25 Jul 2011, 09:34

Actually it is designed to be compiled to a static lib before usage. It comes with a large number of predefined project files for a lot of IDEs and build systems (all possible Visual Studios, cmake, CodeBlocks, devcpp, ...) and their documentation starts with the chapter "Compiling AS".

But to be honest, I personally agree with charlie here. I don't really see the advantage of having a .lib/.a file over directly including their source code. After all, we have their source code in our repo anyways, and the lib is directly build from it, so for the final OD binary it doesn't matter - and we can't provide lib's for all possible compilers and systems.

But because I'm not very experienced with all this cross platform building/linking stuff, this opinion could be horribly wrong. So for the time being I stick to svenskmand's and oln's way of doing it. ;)

edit: I just pushed the comit that makes MingW compiling again with AngelScript. For VC and linux system somebody else has to adjust it. With static linking to the AS lib the OD binary is over 5 MB big, with directly including the source it's only about 3 MB.
StefanP.MUC
 

Re: Clean up

Postby svenskmand » 25 Jul 2011, 20:24

The main advantage of having the library seperate is that it is easier to switch to another version if we discover a bug that interferes with our usage. That is cumbersome when you copy the source into our repository
Jamendo.com - The best music store on the net, uses CC licenses.
User avatar
svenskmand
OD Moderator
 
Posts: 1850
Joined: 09 Dec 2009, 00:07
Location: Denmark

Re: Clean up

Postby StefanP.MUC » 25 Jul 2011, 20:44

This is good for big dependencies and dependencies that provide pre-compiled libs. But AngelScript is very small and does not provide pre-compiled libs. I don't think it's really worth the effort to maintain different versions and platforms for AS ourselves.
StefanP.MUC
 

Re: Clean up

Postby svenskmand » 25 Jul 2011, 21:16

Ok then lets just keep it in the git repo as we have it now.
Jamendo.com - The best music store on the net, uses CC licenses.
User avatar
svenskmand
OD Moderator
 
Posts: 1850
Joined: 09 Dec 2009, 00:07
Location: Denmark

Re: Clean up

Postby StefanP.MUC » 26 Jul 2011, 09:02

So, how should we handle it then with the inclusion and linking?

Currently only MingW works, because I uploaded the lib for it. Building on Linux and Visual Studio won't work until someone builds and uploads the libs and changes our cmake scripts to link them (that's what I meant with "it's not worth the effort").
To make it work for all platform's I could change cmake so that it directly uses the sources.
StefanP.MUC
 

Re: Clean up

Postby svenskmand » 28 Jul 2011, 13:15

If we are going to have the AngelScript source in our repo anyway then there is no point in compiling to a binary, put it into the repo and then link it, I do not see any benefits with that, only problems. So just compile and link it every time the game is compiled and linked.
Jamendo.com - The best music store on the net, uses CC licenses.
User avatar
svenskmand
OD Moderator
 
Posts: 1850
Joined: 09 Dec 2009, 00:07
Location: Denmark

Re: Clean up

Postby StefanP.MUC » 28 Jul 2011, 16:55

Ok, by tomorrow evening (most likely earlier) I should have found some time to adjust the cmake files.
StefanP.MUC
 

Re: Clean up

Postby StefanP.MUC » 28 Jul 2011, 19:51

OK, pushed the commit. AS is now in dep and directly build from the source into the OD binary.

By the way, I added a new point to the clean up list: cleaning up the cmakelists. Looks like these could need some tidy up work, too.
StefanP.MUC
 

Re: Clean up

Postby svenskmand » 29 Jul 2011, 02:07

Ok nice :)
Jamendo.com - The best music store on the net, uses CC licenses.
User avatar
svenskmand
OD Moderator
 
Posts: 1850
Joined: 09 Dec 2009, 00:07
Location: Denmark

Re: Clean up

Postby StefanP.MUC » 29 Jul 2011, 08:55

About the cmake clean up: What's the reason that we have three cMakeLists.txt (root, /src, /include)? Does this bring some compile speed? Why is linking setup in the /src one?

Wouldn't it be better to have only one unified cmake file with a clean structure:
- starting with project details (name, version, ...)
- then the required packages
- then then the file list (*.cpp/*.h)
- then the linking
StefanP.MUC
 

Re: Clean up

Postby StefanP.MUC » 30 Jul 2011, 09:22

Just started some of the clean up of the cmake files (nothing major, only ordering and structure and removing of some really old outcommented stuff).

I think, in the end, th cmake file in /src should only contain the .cpp file list.
And is the cmake file in /include really needed? It's never referenced, after all.

all the linking stuff shoudl happen in the central cmake files in the project root.
StefanP.MUC
 

Re: Clean up

Postby svenskmand » 30 Jul 2011, 13:15

I know nothing about cmake, sorry, I think only you and oln know cmake.

When browsing the cmake files it looks very different from make, cmake looks like a sort of Lisp variantion, in syntax.
Jamendo.com - The best music store on the net, uses CC licenses.
User avatar
svenskmand
OD Moderator
 
Posts: 1850
Joined: 09 Dec 2009, 00:07
Location: Denmark

Re: Clean up

Postby oln » 31 Jul 2011, 15:27

I can't remember if having a cmakelists.txt file in the source folder or not is required, but that seems to be what other projects do at least.
There is more than just the source files in the /src cmakelists.txt, as the list containing them is referenced there. The file in /include doesn't seem to be used, so we could try removing it.
User avatar
oln
 
Posts: 1020
Joined: 26 Oct 2010, 22:16
Location: Norway

Re: Clean up

Postby StefanP.MUC » 31 Jul 2011, 16:04

I'm currently working my way through some cmake docs and tutorials and through our cmake files to see what is really needed and where it should be.

With the last commit I already did put some things into better places. For example, before the commit there were three places that add include folders. I merged them into one single command (but there are still some conditional includes left in strange places).
Sometimes there are also variables defined that are only used one time in the following line instead of directly using the value.

I think, if I understand the docs and tutorials correct, the common practice is to have one cmakelists in the include folder and one in the source folder (if they are not the same folder) that contain only the file lists (I did not find out why the .h files need to be listed in a cmake variable, but this seems to be common practice, too - I'm still reseacrhing this) and one cmake file in the root folder containing the actual cmake code. But what's not common practice, it seems to me, is to have cmake code in the cmakelists from source and include folders. Currently we have the linking setup in src/cmakelists.txt - this should be ordered, cleaned up and put into the root cmake file.

Next week I think I will have learned enough about cmake to do some further clean up. Maybe I can then also change it so that the AngelScript cmake script is used.
StefanP.MUC
 

Re: Clean up

Postby StefanP.MUC » 01 Aug 2011, 15:18

Just pushed a big cmake change:

  • deleted the one in /includes (listing not need, because we specifiy all the include dirs directly)
  • merged the one from /src into the root one and deleted the src one (best practice according to docs and tuorials: one CMakeLists per target)
  • consistent cmake commands (lower case as the docs they, before they had mixed case and random spaces)
  • some more comments for better understanding
  • some conditionals are more logical now (example: before there was a if(NOT WIN32) foloowed by a if(WIN32), is now: if(WIN32)-else())
  • merged, if it was possible, all the clustered "inlcude_directories()" and "link_directories()" into one

What's left to do?
- I didn't touch the last third of the file because it contains some stuff that I'm currently not sure what it does exactly. But it looks like there's also some messy stuff laying around.
- the AngelScript linking. Now that the structure is pretty clean, I'll have a look again if we can now use the original AS cmake file. If it works than we could automatically link it statically (as in: link to lib, but let the lib build automtically if non-existant, without having to explictily adding the source files to our code base). This would be the most flexible solution.
- add support for automatic version changing between source and cmake
- maybe add some different compile versions (like "Release" and "Debug" or something, with different compiler flags for faster test buildings and more optimized release buildings)
StefanP.MUC
 

Re: Clean up

Postby StefanP.MUC » 01 Aug 2011, 19:39

Update:

Just managed to use the cmake file from AngelScript (so nor more manual adding of AS stuff to our cmake file). This also shortened our cmake file. Building now works like this:
Load AS cmake file at beginning of OD cmake file, build AS lib if non-existant, build OpenDungeons, link OD to Angelscript lib.

The reason why it didn't work before was an error in the cmake file fromAS (I fixed it for us, maybe the next AS version will have fixed it officially).
StefanP.MUC
 

Who is online

Users browsing this forum: No registered users and 0 guests