otto-link / Hesiod

A desktop application for node-based procedural terrain generation.
https://hesioddoc.readthedocs.io/
GNU General Public License v3.0
111 stars 5 forks source link

Project File Restructuring. #106

Closed PatchByte closed 8 months ago

PatchByte commented 8 months ago

Hey I have already started working on the new serialization, but before that I would like to restructure the project a little bit. This should be an open pr for now, will commit more but I do want feedback! I noticed the CMakeList was quite "wild". I cleaned up everything a little bit.

Changes (Please do still manually review and verify):

PatchByte commented 8 months ago

Please review @otto-link

otto-link commented 8 months ago

Thanks for the PR, a different perspective and a bit of house keeping are likely to improve the code. I won't unfortunately be able to review it on details before next week but the overall direction seems sound.

PatchByte commented 8 months ago

I also added a github workflow that automatically builds an AppImage and also a linux binary (for ubuntu) but only with gcc because clang has problems. Windows will be included later (N/A)

PatchByte commented 8 months ago

Problem right now would be only that the workflow takes approx. 40 minutes.

otto-link commented 8 months ago

I quickly browse your commits and it looks very clean. Adding the githib workflow is also a huge plus! I'll review/merge as soon as I can. And we're circling back to the compiling time issue 😅

PatchByte commented 8 months ago

Yeah I will take a look into the compiler issue, I already might have an idea

PatchByte commented 8 months ago

So now it is starting to get weird, it builds for me.

PatchByte commented 8 months ago

@otto-link what if we merge the generate_node_snapshot project into the hesiod project, it would be a waste of time recompiling everything again. I am thinking about a start argument like ./Hesiod ---generate_node_snapshot

PatchByte commented 8 months ago

Most of the things right now are ready to merge!

otto-link commented 8 months ago

Good idea for the generate_snapshot but this was something "quick and dirty" I made to generate some snapshots for the documentation and it has not been really used so far. At this point you might as well remove it, your call.

Edit - just saw you already implemented generate_snapshot as an optional argument, let's keep it this way

PatchByte commented 8 months ago

Yeah its kinda hacky but it works...

PatchByte commented 8 months ago

Alright Doxygen is fixed, I tried generating the doxygen docs and apparently it works fine!

PatchByte commented 8 months ago

This should now be ready to merge!

otto-link commented 8 months ago

Really nice, I'll be able to work on it next monday, the PR should be merged pretty quickly to the main after that.

PatchByte commented 8 months ago

I will also try to get a mingw workflow running to build the windows executable