SandroWissmann commented 2 years ago

Hi,

I tried to follow the link on the readme to the instructions how to build the project but it does not work.

Can you fix it or give instructions how to run it?

Im maybe interested to collaborate on this.

Best regards Sandro Wißmann

SandroWissmann commented 2 years ago

I tried to build like agustus described here https://github.com/Keriew/augustus/blob/master/doc/BUILDING.md

But this gives these errors:

/home/sandro/Documents/Ozymandias/Ozymandias/src/platform/file_manager.cpp: In function ‘int platform_file_manager_list_directory_contents(const char*, int, const char*, int (*)(const char*))’:
/home/sandro/Documents/Ozymandias/Ozymandias/src/platform/file_manager.cpp:140:82: error: ‘errno’ was not declared in this scope
  140 |             log_error("platform_file_manager_list_directory_contents:", strerror(errno), errno);
      |                                                                                  ^~~~~
/home/sandro/Documents/Ozymandias/Ozymandias/src/platform/file_manager.cpp:83:1: note: ‘errno’ is defined in header ‘<cerrno>’; did you forget to ‘#include <cerrno>’?
   82 | #include <unistd.h>
  +++ |+#include <cerrno>
   83 | 
/home/sandro/Documents/Ozymandias/Ozymandias/src/platform/file_manager.cpp: At global scope:
/home/sandro/Documents/Ozymandias/Ozymandias/src/platform/file_manager.cpp:214:5: error: ambiguating new declaration of ‘int platform_file_manager_remove_file(const char*)’
  214 | int platform_file_manager_remove_file(const char *filename) {
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/sandro/Documents/Ozymandias/Ozymandias/src/platform/file_manager.cpp:1:
/home/sandro/Documents/Ozymandias/Ozymandias/src/platform/file_manager.h:57:6: note: old declaration ‘bool platform_file_manager_remove_file(const char*)’
   57 | bool platform_file_manager_remove_file(const char *filename);
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: note: unrecognized command-line option ‘-Wno-c++11-narrowing’ may have been intended to silence earlier diagnostics
make[2]: *** [CMakeFiles/ozymandias.dir/build.make:90: CMakeFiles/ozymandias.dir/src/platform/file_manager.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:83: CMakeFiles/ozymandias.dir/all] Error 2
make: *** [Makefile:136: all] Error 2
 ✘ sandro@sandro-pc  ~/Documents/Ozymandias/Ozymandias/build   master ±  make 
Consolidate compiler generated dependencies of target ozymandias
[  0%] Building CXX object CMakeFiles/ozymandias.dir/src/platform/file_manager.cpp.o
/home/sandro/Documents/Ozymandias/Ozymandias/src/platform/file_manager.cpp:215:5: error: ambiguating new declaration of ‘int platform_file_manager_remove_file(const char*)’
  215 | int platform_file_manager_remove_file(const char *filename) {
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/sandro/Documents/Ozymandias/Ozymandias/src/platform/file_manager.cpp:1:
/home/sandro/Documents/Ozymandias/Ozymandias/src/platform/file_manager.h:57:6: note: old declaration ‘bool platform_file_manager_remove_file(const char*)’
   57 | bool platform_file_manager_remove_file(const char *filename);
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: note: unrecognized command-line option ‘-Wno-c++11-narrowing’ may have been intended to silence earlier diagnostics
make[2]: *** [CMakeFiles/ozymandias.dir/build.make:90: CMakeFiles/ozymandias.dir/src/platform/file_manager.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:83: CMakeFiles/ozymandias.dir/all] Error 2

SandroWissmann commented 2 years ago

Just to add. I could build julius just fine so libs etc should work. There must be some issue here.

I'm under latest manjaro.

crudelios commented 2 years ago

I'm not the developer of Ozymandias, but from what I gather this is still a work in progress and is only building on the compiler @Banderi is working with (which I don't remember what it is).

Getting this to work on more compilers would be a great step forward though!

SandroWissmann commented 2 years ago

Would be nice to know and maybe adapt. This could also help to get more people involved.

I tried it with the latest gcc available in manjaro . Can look up the version before.

Annother solution could be also to run it with a dockerfile with supported compiler version.

Banderi commented 2 years ago

Hey there! Yeah, the work so far is a huge mess and it was very loose and experimental since I was the only one around; I've been looking recently into how to move the project to a proper build system, but that's a big endeavor so it will take some time. On my end I've been working with CLion on Windows and MinGW, to give you an idea of the headaches. I am a bit far away from this at the moment, but I'd love to try and help as best I can!

These errors seem related to my code specifically (not Augustus), in particular things that work in Windows but not on other systems because I wasn't being particularly careful. The first error seems to need #include <cerrno> at the top of file_manager.cpp since Linux needs that header.

The second error (and if it's what I think it is, might be more of a big problem...) seems that int and bool are not compatible, and therefore the compilation fails. This occurs because the original engine was written in C and I tried to my best to convert it to C++ with bool wherever I could, but new places keep popping up everywhere.

There's also many many other things that I threw together and barely made it work for Windows, and left the macro conditionals for Linux untouched/outdated/incorrect/unimplemented, so... I fear to make this compile possibly could take a good amount of work.

I'm gonna try and finish with some other works and resume work on Ozymandias soon, so I can also have a better look myself, and maybe test on some linux VMs. Feel free to reach here in the meantime though, and I'll try to help as best as I can!

SandroWissmann commented 2 years ago

Hi,

I already fixed now these two issues in file_manager.cpp. I suggest I will continue try the compile on linux and try to fix the follow up errors as good as I can. Would be nice if we get it running again. Maybe create a branch for it and the pieces im unsure cannot fix I can point you too it.

So on windows you have already some functionality?

Is your plan to convert it complete to modern C++ ?

regards Sandro

SandroWissmann commented 2 years ago

Ok I have a local branch but I cannot push it to upstream because I have no rights. Shuould I fork and create a pull request?

Also I hit a dead end which could be maybe fixed with renaming.

This error:

In file included from /home/sandro/Documents/Ozymandias/Ozymandias/src/building/destruction.cpp:21:
/home/sandro/Documents/Ozymandias/Ozymandias/src/core/random.h:13:16: error: redefinition of ‘struct random_data’
   13 | typedef struct random_data {
      |                ^~~~~~~~~~~
In file included from /usr/include/c++/12.2.0/cstdlib:75,
                 from /usr/include/c++/12.2.0/stdlib.h:36,
                 from /usr/include/SDL2/SDL_stdinc.h:46,
                 from /usr/include/SDL2/SDL_render.h:51,
                 from /home/sandro/Documents/Ozymandias/Ozymandias/src/graphics/image.h:4,
                 from /home/sandro/Documents/Ozymandias/Ozymandias/src/building/destruction.cpp:6:
/usr/include/stdlib.h:424:8: note: previous definition of ‘struct random_data’
  424 | struct random_data
      |        ^~~~~~~~~~~
cc1plus: note: unrecognized command-line option ‘-Wno-c++11-narrowin

looks like random_data is already defined in one of the headers for linux.

Could be fixed with renaming or adding a basic namespace. I think anyway this class should be turned into a proper c++ class which has the methods as members...

Let me know what you think.

SandroWissmann commented 2 years ago

Ok I renamed random_data temporary to randomdata to fix this. Now I could build the project.

I will try to fix some more warnings GCC 12 trows.

Then the question is if in theory I could already try out if something works in the game?

crudelios commented 2 years ago

If you can build it, you definitely can try it out.

When I tested the game on Visual Studio (3 or 4 months ago), it crashed pretty fast (different compilers can cause that).

So I think a good first step is checking for general stability.

Banderi commented 2 years ago

That's awesome to hear! If you want to do any pull requests, go ahead and I'll check it out. Or should I make a public branch? I'm not entirely sure what the best practice is with git projects and collaborative efforts, since this is my first time.

And yeah I was trying to make it into an OOP/C++ style, since dealing with plain C is a nightmare for me haha. I wasn't necessarily aiming for a perfect rewrite into proper, modern C++ since I don't have that kind of expertise or need, moreso I just really really needed some classes and OOP restructuring to be able to work with it, at least.

As for the engine, there's lots of things missing still but a good starting bit is present and working, e.g.:

loading up any saves/map files
campaign progression
starting custom scenario
farms, floods, gathering & production of some of the initial resources
some of the religion mechanics etc.

One more thing, from my "quick and dirty" hacks I did in the beginning, I've disabled the original internals for automatically saving the folder setting files (there was a mini "library" if I recall that is inside Augustus, but I forget the name of) and so I've set it to manually write the folder setting files to the engine's binary folder. This is because on Windows portability is everything, and using Registry/AppData should always be avoided (on Linux, exactly the opposite; everything is installed, and extra assets like mods are kept separate from the engine folder, which always kinda baffles me) so that might be another thing that on Linux breaks!

SandroWissmann commented 2 years ago

From my professional work we just do a branch from the original one but I think here on github it works like I should fork the project and then do my changes there and do a pull request. Then after that one gets merged by you you get my changes. I will try to do that soon. Maybe also try before out if the program runs.

In my diff will be probably some TODO comments we still need to solve.

If possible I would try to bring the project into modern c++ style (C++17). I think It will also reduce code a lot from c to c++ and make many things easier.

My background is that I started with C but nowadays in my professional work I work with C++ / QT on Embedded Linux on bigger projects. So I can help you with porting / getting it in a more modern style.

As always only time is the issue since I work fulltime. So lets see what we can do.

PS: regarding this file saving. It can be probably heavy simplified with std::filesystem but its c++17. Also many of this low level char* string stuff could be just replaced with std::string. Just some quick findings i stumbled on.

Also one question about these defines:

define GROUP_TOP_MENU_SIDEBAR IMAGE_COLLECTION_GENERAL, 11

What does this mean? GROUP_TOP_MENU_SIDEBAR gets the value from IMAGE_COLLECTION_GENERAL ? I can make no sense out of the 11 at the end. Or is it GROUP_TOP_MENU_SIDEBAR == 11?

Banderi commented 2 years ago

Ah yes... that's one of the many "hacks" and really "naughty" macros I've been using for making life easier on my end. Long story short, the image fetching system uses a global list of image groups, but in the file system they are packed into different files; so the function takes in the pack as well as the local group index, and each of those macros define a single global image group that converts to a pair of arguments for that function. The reason why is complicated, but basically I would need to rewrite a good chunk of the internal systems in order to "fix" that, which might as well be done when I start fresh.

And speaking of such, as a final note: there's a very real possibility I'm going to mostly abandon this repo entirely and start anew with a robust engine I'm planning to write from scratch; most things I would be able to port over as is, and as I port the systems bit by bit I will re-implement them in a way that fit with the paradigm I'm using, because for the past 2 years I did nothing but refactor and re-implement stuff rather than actually making any progress, so it's not really worth it. Also, there's a few things that usually come with modern C++ that I will always resist, like heavy usage of templates, lambda functions, heavy/complex object inheritance etc. since they make the code completely unreadable for me, unfortunately. Other than that, no objections from me!

SandroWissmann commented 2 years ago

Good morning,

regarding your C++ concerns.

My goal is always to keep the code simple, let classes do only one thing well and have it easy readable.

Regarding these topics:

Template usage: -I know there can be the horror of template meta programming which is like a language in a language. It can maybe make some stuff 10% faster but is also for me 90% more unreadable. I would only very rarely use simple templates like when a functions needs to do the same things for many types. But it should be not used often.

-Lambdas

Here the rule is also. If you realize a lambda is doing anything from not trivial. Instead make it a function with a descriptive name.
The thing were small lambdas come in handy is with std library.
For example with https://en.cppreference.com/w/cpp/algorithm/find. It could be used without lambda but then you need to write a functor class.
For sure it should be avoided to write big brainfucks with this...

-heavy / complex OOP -First of all I would avoid inheritance in most cases. Very often composition is better than inheriatnce. -These heavy trees you mention you can do in any modern language but should be also avoided. -My goal here is always to keep the classes simple. If classes do to much better split them into separate classes.

Maybe after linux ran I can refactor some area you need and then we can see if the style is acceptable or can be discussed.

It would also help over time to create some markdown with basic coding guidelines. I know in C++ you can do many things but not all really should be done.

Important is to keep the code simple and readable especially in big projects.

So I will first check if we get the linux back running.

Have a nice day

crudelios commented 2 years ago

I think some simple inheritance like

class TaxCollector : public Figure;

class Colosseum: public Building;

Wouldn't be such a bad idea as it would work much better that the current giant switch statements and else cases spread everywhere.

Multiple inheritance, though, that's a different story. My opinion is, kill it with fire.

SandroWissmann commented 2 years ago

Hi I agree. Sane inheritance can be used to get rid of crazy switches like that.

I assume it was more meant like crazy multi inheritance trees.

Aim should be to make the code simple.

SandroWissmann commented 2 years ago

Ok I opened the pull request please check it. Now game at least makes it until the select family screen.

Banderi commented 2 years ago

That's great! I hope I can check it out soon. And yes, I agree completely regarding the above; simple, clean and readable is always the best, even though I tend to do some pretty improper/inconsistent things sometimes, I admit!

SandroWissmann commented 2 years ago

I can imagine that its frustrating getting everything to work in such a big code base. But from my experience if we do thinks hacky in the end we end up do them twice and it takes longer.

I think there is still a lot of C to convert which then could be work as a base if you plan to start from scratch and slowly build it up again.

Banderi commented 1 year ago

For sure, there's still tons of C style code that clashes hard with the rest of my hacky stuff. I think if you wanted to focus on that it's great, I have no objections and at all and yes, it would be a huge help for porting later!

By the way, if you're interested, the other is this one: https://github.com/Banderi/Antimony-III/ Currently, it's just empty; I'm just looking very closely at how to properly set up compilation with CMake + a manager like Hunter or suchalikes, so that noone would ever have any more troubles with SDL2 inclusions and compilation. I've tried a few things so far, but CMake on its own right is a enormous pain in the butt. Suffice to say, compilation with C/C++ in general is hard because Linux and Windows have very different ecosystems and their idea of library/file handling is not compatible most of the time. SDL2 is sadly a prime example of this. Hopefully, I'll be able to find a compromise.

SandroWissmann commented 1 year ago

Hi agree the CMake stuff can time and can be hard to learn. Its normal to get frustrated about it.

Regarding what to do. Since I don't know so much about the game logic I would first really try to refactor these skeleton logic like the settings and the stuff around it into proper c++ it should make out life easier. I think we will need these pieces anyway.

Banderi / Ozymandias

Build instructions cannot be opened. #6

define GROUP_TOP_MENU_SIDEBAR IMAGE_COLLECTION_GENERAL, 11