Update of patch #6945 (project freeciv):
Category: None => general
Planned Release: => 3.0.0
_______________________________________________________
Follow-up Comment #3:
Some comments about resource clustering part. I'd prefer to keep fracture
generator separate (and in original patch #6882).
+bool terrain_is_too_flat(struct tile *ptile,
+ int thill, int my_height);
+struct terrain *pick_terrain(enum mapgen_terrain_property target,
+ enum mapgen_terrain_property prefer,
+ enum mapgen_terrain_property avoid);
There's a point in that you keep these out from mapgen.h (which would be
logical header for mapgen.c funcions) since it could be considered outside-API
for map generator code. Still, these should go to some header and not only to
the C-files using them. As they are not static any more, and there can be
outside linkage to them, they should be findable from headers.
I'll create a new ticket about cleaning current situation a bit, and providing
resolution to this in the process.
+ if (MAPGEN_FRACTURE == game.map.server.generator)
+ make_fracture_relief();
+ else
+ make_relief(); /* base relief on map */
->
if (MAPGEN_FRACTURE == game.map.server.generator) {
make_fracture_relief();
} else {
make_relief(); /* base relief on map */
}
Similarly for all the similar constructs.
+ make_fracture_map(MAX(1, 1 + get_sqsize()
+ - (MAPSTARTPOS_DEFAULT !=
game.map.server.startpos
+ ? player_count() / 4 : 0)));
While this is not performance-critical path, please assign "1 + get_sqsize()
- (MAPSTARTPOS_DEFAULT != game.map.server.startpos ? player_count() / 4 : 0)"
to a variable outside MAX() macro and use that varialble in there. Otherwise
we will be repeatedly checking "that MAX() invocation where complex things are
involved" since it's such a macro that the parameters get evaluated multiple
times (twice, in case of MAX())
+ const struct terrain *ters[256]; /* 256 worth of pointers */
MAX_NUM_TERRAINS
+ static const struct resource_type **get_resource_list()
As this is C-code (not C++), use 'void' parameter list.
+ ters[pterrain->identifier & 0xff] = pterrain;
Use terrain_index()
Equivalent comments about resources.
+static bool tile_is_legal_resource(struct tile *pt, struct resource_type
*rs)
Implement using terrain_has_resource()
+ et = rt->self;
+ if (et)
resource_extra_get()
Resources are strictly subset of extras, so "if (et) {" is redundant, but ok.
Could use "fc_assert(et != NULL);"
+ res_cnt[iter] = (game.server.max_players >> 1) + fc_rand(3);
Having dependency on current value of max_players setting doesn't sound good.
Regardless of actual number of players it will be almost always 150 (the
default), but can be set as low as 1.
+ "patttern
-t
+#define SPECENUM_VALUE9 EC_CLUSTER
+#define SPECENUM_VALUE9NAME "Cluster"
This new EC_CLUSTER is not used anywhere...
The new cluster_chance property of extras should be documented in the comments
of supplied rulesets (it's tiresome to copy same comments to all of them, but
those making their modified versions usually don't look far from the very
files they copied as a baseline)
circle_iterate() could be better for shaping the cluster than
square_iterate()
There seems to be nothing preventing add_resources() from going through the
100 iterations of whole map in currently typical situation that no extra has
cluster_chance above zero.
I think having a boolean server setting (or integer multiplier, like with
disaster chance) controlling if clustering is enabled or not.
_______________________________________________________
Reply to this item at:
<http://gna.org/patch/?6945>
_______________________________________________
Message sent via/by Gna!
http://gna.org/
_______________________________________________
Freeciv-dev mailing list
[email protected]
https://mail.gna.org/listinfo/freeciv-dev
