Feature #1677
openallow terrain to have > 1 animal
0%
Description
In terrain.ruleset, animal="..." currently only supports 1 animal. I would like to use a comma separated list.
Files
Updated by Dean Brown 9 days ago
- File 1677.patch added
- File 1677Main.patch added
Causes a change to network packets, so expecting target 3.3.0.
I wanted to change the "animal = ..." to "animals = ..." in the terrain.ruleset files, but that breaks freeciv-ruleup. In file
server/ruleset/ruleload.c, routine load_ruleset_terrain() would have to try to lookup both "%s.animal" and "%s.animals" to make freeciv-ruleup work.
I can update the wiki page [[http://www.freeciv.org/wiki/How_to_update_a_ruleset_from_3.2_to_3.3]] when this is done.
Updated by Marko Lindqvist 8 days ago
- Category changed from Rulesets to General
- Target version set to 3.4.0-d3f
- Since this is not going to S3_2 or older, use nullptr instead of NULL
- As you said: "animal" needs to be changed to "animals"
- Style: Empty line between variable declarations and first line of code within the block
- Style: Some broken indentation
- It would be better to store num_animals in the terrain structure (just a few integers more memory taken) than to burn CPU to figure out the count every time it's needed
- In the client side terrain.animals must be initially initialized to nullptr, and in the server side it wouldn't hurt (so can be done in common code). Otherwise we may have non-nullptr animals, when in fact there's no animals
- 'FALSE' and 'TRUE', not 'false' and 'true' (Yeah, that's something we probably should change in coming versions, but I'd rather concentrate NULL > nullptr migration for now, instead of starting another migration already) Style: "if (! ok) {" -> "if (!ok) {" (no space after '!')
Updated by Dean Brown 1 day ago
- File 1677.patch 1677.patch added
- File 1677Main.patch 1677Main.patch added
Thanks for the feedback, here's updated better patches. I figured out how to use rscompat to avoid breaking freeciv-ruleup.
I originally made the terrain->animals to be null-terminated like the terrain->resources, but I agree it's better to also have
terrain->num_animals. It might also be better if terrain->resources were to be re-done similarly, but that's out of scope for this issue.