Feature #1737
openimprove reporting of appearing/disappearing extras
0%
Description
In srv_main.c routine end_turn(), the code that handles appearing and disappearing extras only notifies the tile owner, and has a comment
/* TODO: Should notify players nearby even when borders disabled,
* like in case of barbarian uprising */
Easy fix to notify all players for whom the tile is visible.
Files
Updated by Dean Brown 7 days ago
- File 1737_S3_2&S3_3.patch 1737_S3_2&S3_3.patch added
- File 1737Main.patch 1737Main.patch added
Updated by Marko Lindqvist 5 days ago
- Target version set to 3.3.0
The proposed patch is a rule change in that the tile owner no longer gets to know extra (dis)appearances unless they also see the tile.
At least I'm not opposing that in future versions -> targeting 3.3.
Updated by Marko Lindqvist 5 days ago
- Status changed from New to In Review
- Assignee set to Marko Lindqvist
Updated by Dean Brown 5 days ago
- File 1737Better_S3_2&S3_3.patch 1737Better_S3_2&S3_3.patch added
- File 1737BetterMain.patch 1737BetterMain.patch added
Can a tile really be not visible to its owner? Did not think of that, and I'm not sure. Whatever, easy to accommodate - here's better patches.
Updated by Marko Lindqvist 5 days ago
Dean Brown wrote in #note-4:
Can a tile really be not visible to its owner? Did not think of that, and I'm not sure. Whatever, easy to accommodate - here's better patches.
This forgets the original "TODO" that you were set to fix. The "rumor" system does not work when the borders are disabled.
Borders extend from cities far further than city vision range, so it's common situation that owner does not see some tiles. Unless you have set 'borders' to SEE_INSIDE or EXPAND.
Updated by Marko Lindqvist 2 days ago
So I think we continue with the original patches (that's what I'm going to push if nobody disagrees)
Updated by Dean Brown 2 days ago
I prefer the better patches - adds to the existing functionality instead of replacing it. But I'm OK either way.
Updated by Marko Lindqvist 2 days ago
Dean Brown wrote in #note-7:
I prefer the better patches - adds to the existing functionality instead of replacing it. But I'm OK either way.
Then they need to be updated to address the original TODO issue, or at least the TODO comment should not be removed if it's not addressed.
Updated by Dean Brown 1 day ago
The "rumor" system does not work when the borders are disabled.
Are you referring to this call in create_barbarian_player()?
notify_player(NULL, NULL, E_UPRISING, ftc_server,
_("%s gain a leader by the name %s. Dangerous "
"times may lie ahead."),
nation_plural_for_player(barbarians),
player_name(barbarians));
It looks to me like that call happens regardless of disabled borders.
Interpreting that comment is a bit unclear -
1 The comparison to barbarian uprising isn't good because barbarian uprisings get announced to all players regardless of tile or disabled borders.
2 What does "nearby" mean exactly? If "nearby" means adjacent or visible, then tile_is_seen() is true and it gets announced, and the comment is addressed. If it means "up to num tiles away" then I'm not sure if it's a good idea to announce about a tile that is neither owned nor visible. But that's assuming tile_is_seen() is not affected by disabled borders - my testing seems to show that's true.