Project

General

Profile

Actions

Feature #1677

open

allow terrain to have > 1 animal

Added by Dean Brown about 1 month ago. Updated 1 day ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
General
Target version:
Start date:
09/08/2025
Due date:
% Done:

0%

Estimated time:

Description

In terrain.ruleset, animal="..." currently only supports 1 animal. I would like to use a comma separated list.


Files

1677Main.patch (52.1 KB) 1677Main.patch Dean Brown, 10/07/2025 11:30 PM
1677.patch (52.8 KB) 1677.patch Dean Brown, 10/07/2025 11:30 PM
Actions #1

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.

Actions #2

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 '!')

Actions #3

Updated by Dean Brown 1 day ago

  • File deleted (1677.patch)
Actions #4

Updated by Dean Brown 1 day ago

  • File deleted (1677Main.patch)

Updated by Dean Brown 1 day ago

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.

Actions

Also available in: Atom PDF