numpy-replacements for image-drawing #161
Conversation
| c = numpy.empty(e.shape, dtype=numpy.float) | ||
| c[numpy.invert(world.ocean)] = (e[numpy.invert(world.ocean)] - min_elev_land) * 127 / elev_delta_land + 128 | ||
| c[world.ocean] = (e[world.ocean] - min_elev_sea) * 127 / elev_delta_sea | ||
| c = numpy.rint(c).astype(dtype=numpy.int32) # proper rounding |
There was a problem hiding this comment.
This was not rounded before, which might even be preferable.
| @@ -590,7 +588,8 @@ def _draw_a_mountain(pixels, x, y, w=3, h=3): | |||
| pixels[x + modx, y + mody] = mcr | |||
|
|
|||
|
|
|||
| def _pseudo_random_land_pos(world, i): | |||
| def _pseudo_random_land_pos(world, i): # What is this good for? Why is it not truly random? | |||
There was a problem hiding this comment.
What is this method actually doing that cannot be done by world.random_land(), a truly random function?
There was a problem hiding this comment.
I should remove my comment then.^^ Due to the recent changes to the RNG this function could technically be removed, I guess. It would require a couple of small changes on top of it, though.
|
we still want to merge this, right? Rerunning the tests |
|
Sure.^^ I closed the PR that doesn't have to be merged, these new ones are the replacements. |
| @@ -351,7 +350,7 @@ def sea_level(self): | |||
| # Tiles around | |||
| # | |||
|
|
|||
| def on_tiles_around_factor(self, factor, pos, radius=1, action=None): | |||
| def on_tiles_around_factor(self, factor, pos, action, radius=1): | |||
There was a problem hiding this comment.
action=None seemed unsafe since that case wasn't even handled by the function. In other words: action was a required parameter disguised as an optional one. (The same goes for line 364.)
There was a problem hiding this comment.
ok, have you checked all the invokation of this method, given we are changing the order of the parameters? Same goes for the following function
There was a problem hiding this comment.
I have, and they were all called using the keyword.
Although I didn't find a call to on_tiles_around.
| if max_elev is None or e > max_elev: | ||
| max_elev = e | ||
| # elev_delta = max_elev - min_elev # TODO: no longer used? | ||
| #min_elev = world.elevation['data'].min() |
There was a problem hiding this comment.
mmm why do we keep code commented out?
There was a problem hiding this comment.
It was there before, I didn't know if I could remove it. (In the previous version of the code the loop was still run, even though the variables weren't needed anywhere. So it looked less dead than it was.)
|
Updated. |
|
Except for the difference in rounding of the heightmaps (line 229 of this PR) this passes the tests and is otherwise finished. I added another PR to worldengine-data Mindwerks/worldengine-data#8 (even though the heightmaps will be changed again when moving them to 16 Bit) and then this is as ready is it gets (aside from code-critique). |
|
Rebased and conflicts solved. |
| mask = numpy.ma.array(e, mask = False) # the whole map | ||
| min_elev_land = mask.min() | ||
| max_elev_land = mask.max() | ||
| elev_delta_land = (max_elev_land - min_elev_land) / 11.0 |
|
Some minor comments but it seems good to go, sorry if it took a while to review it |
|
Changes are up, hopefully it's fine now. |
|
Oh, and don't forget: For this to pass the tests, worldengine-data needs to be updated (Mindwerks/worldengine-data#8). |
|
Ok, I ran the code locally with the patch for worldengine-data and this one and tests pass. Merging both. |
numpy-replacements for image-drawing
Another portion of the code in #141 . Some very small changes and quite a lot of changes in the draw- and drawing_functions-modules.
This again is based off of #146 .
Tests will fail for now due to some differences in rounding for the grayscale-map.