Mindwerks / worldengine

World generator using simulation of plates, rain shadow, erosion, etc.
MIT License
988 stars 129 forks source link

WIP: Variable ocean levels #134

Open tcld opened 8 years ago

tcld commented 8 years ago

Continuation from here: https://github.com/Mindwerks/worldengine/pull/125

A small fix to one of the test-files is still missing. I just want to put this here so the tests can have a go (on my system everything except the "ocean_level == serialized_ocean_leve?"-test worked fine) and maybe somebody else can look if I completely misunderstood things. Maybe it would be appropriate to rename the variable "ocean_level" to "ocean_area" or "ocean_surface". But I understand that would be a bit of work.

psi29a commented 8 years ago

Hahahahahhaha....

AssertionError: 0.65 != 0.6499999761581421

it is shit like this that makes me face palm so hard...

try assertAlmostEqual.

tcld commented 8 years ago

Good call.^^

EDIT: Problem with this is that the ocean is filled up by fill_ocean() and that is not an exact science. I gave it a little wiggle-room when trying to find the optimal ocean height since I didn't want to run into problems with varying floats there.

EDIT2: A failed test again. This one I have no clue about. Could this be a github hiccup?

psi29a commented 8 years ago
AssertionError: <worldengine.world.World object at 0x7f970b748110> != <worldengine.world.World object at 0x7f970af2ed10>

You are comparing two different objects, so of course it fails. So you'll either need to compare the data members of the two objects or you'll need do the Python equivalent of == overload in the World class. Something like:

def __eq__():
    something something comparing members
tcld commented 8 years ago

I know of this problem. I already said it somewhere else, but I need to make some small changes that are already touched in the perf_opt branch. It would probably cause merge-problems if I did that twice. So for now the tests won't be passed.

(I don't have much experience with git, there might be a way around this.)

psi29a commented 8 years ago

Can't you just merge the two branches?

Or, if you are ready... I can merge in your perf_opt branch and you can rebase this one on master.

tcld commented 8 years ago

Like this? Then I would have to close the pull request again, wouldn't I?

-master
     \
      \
        perf_opt -- ocean_level
psi29a commented 8 years ago

Not at all!

We can do two things: 1) You do a rebase of ocean_level based on perf_opt.

git checkout ocean_level
git rebase perf_opt
git push --force

2) I merge in perf_opt. You do a:

git checkout master
git pull
git checkout ocean_level
git rebase master
git push --force

Either way is good, it is up to you. :)

tcld commented 8 years ago

Mmh, that looks reasonable. But, of course, I would prefer option 2. :P (And thank you for the instructions. :) )

psi29a commented 8 years ago

Right, give me a few and I'll give it a good eye-ball.

tcld commented 8 years ago

A few what?

This is perf_opt, by the way: https://github.com/Mindwerks/worldengine/pull/132

psi29a commented 8 years ago

minutes/hours ;)

I'm also working atm...

tcld commented 8 years ago

I'm going to buy into approach 1, then. No unnecessary hurry.

psi29a commented 8 years ago

Sure, that works too.

tcld commented 8 years ago

I tried this:

git checkout ocean_level
git rebase perf_opt
git push --force

Now I am curious what happens. On my system all tests passed etc.

EDIT: Mmh, this pull request now includes all the commits of the perf_opt-branch, too. I want to see how that plays out.

EDIT2: Yay, the checks are finally passed!

psi29a commented 8 years ago

:)

What will happen is that I'll merge in the perf_opt when it is ready.

This pull request (github) is smart enough to know that those commits already exist in master and will only merge in the new commits into master when this branch is ready.

The only problem (small one, really) is if you push a new commit to the perf_opt branch. Then you'll just have to rebase this branch against perf_opt again. No big deal.

tcld commented 8 years ago

I won't push anything there. It's perfect. ;P

psi29a commented 8 years ago

Indeed. :P It was merged without prejudice.

psi29a commented 8 years ago

You still working here?

psi29a commented 8 years ago

I'm rebuilding so that it uses the newly merged stuff in master.

tcld commented 8 years ago

I am currently testing something, better wait a bit with a merge (at least an hour).

After I did some more work yesterday I generated a really big map (3000x2000) and it had some artifacts on it. I now have to find out which commit introduced those (highly expect it to be the last one, but I still want to recheck this ocean_level-branch).

I will tell you once that test is done and how to proceed. :)

tcld commented 8 years ago

My latest test still had some artifacts. I will start pulling the latest changes to worldengine, rebase this branch and try some more things.

tcld commented 8 years ago

So here is my problem, maybe somebody can help: Two maps generated with the same seed, one using the current master-branch, one using the ocean_level_fix-branch. The latter one will look different due to this commit https://github.com/tcld/worldengine/commit/3e955e2dd9e09236e6f8c6726c1c3f5eef24f9ee (platecs output depends on the sea_level-parameter it is passed, i.e. the amount of surface covered with water.)

The first image has an ocean-coverage of ~78% (used Gimp to find that out). For the second one that's ~71%. seed_13050_elevation seed_13050_elevation

The second map is ugly, and I am not even sure if it is due to a problem in the code. When draining parts of the ocean, certain spots of the elevation map will start to show up as land. And even the first image shows that there are many shallow places in the ocean that would probably be above water were the ocean a little less deep.

Maybe somebody can give some input about this. Meanwhile I may cherry-pick the commits of this branch that are useful for the numpy-branch so it can be merged in before the ocean-levels. If no better solution is found.

tcld commented 8 years ago

I just realized something: I was using 9 plates for platec. That must have been the reason for the land-masses being distributed so equally. Here two images (master then ocean_level_fix) for 3 plates:

seed_13051_elevation seed_13051_elevation

Maybe the problem is not a problem? Maybe a planet cramped with plates will have a lot of islands?

tcld commented 8 years ago

Right now this branch is most likely incompatible with the numpy-changes. Should the "problems" above find their solution, I will rebase this branch and make everything nice and clean.

EDIT: Rebasing is done, the actual changes this branch introduces are very few.

tcld commented 8 years ago

Rebased, now the list of changes is really short. I didn't check the output, though, since I expect the previous problem to still exist.