#44158: broadcast_city_info has apparently illogical code Open Date: 2022-03-22 07:06 Last Update: 2022-03-23 18:25 URL for this Ticket: https://osdn.net//projects/freeciv/ticket/44158 RSS feed for this Ticket: https://osdn.net/ticket/ticket_rss.php?group_id=12505&tid=44158 --------------------------------------------------------------------- Last Changes/Comment on this Ticket: 2022-03-23 18:25 Updated by: cazfi Comment: After more careful checking I think the fact that info is sent to powner instead of pplayer is the only actual bug. The code could be better arranged, and the variable 'send_city_suppressed' more accurately named (my assumption about is was causing me to think there to be other bugs) - 'send_city_suppressed' name does not make it clear it applies to send to *owner* only. One would assume that if info is sent to anyone despite that flag, it would owner who still gets latest information - It's a bit weird that 'send_city_suppressed' check is only inside 'can_player_see_city_internals()' block, not affecting else and 'can_player_see_city_externals()' block. This is not an actual bug simply because the city owner always 'can see city internals', so owner-specific 'send_city_suppressed' is never relevant in the else branch. I may open a new ticket about renaming 'send_city_suppressed'. In this ticket I plan to change information to be sent to pplayer, and to move "if (!send_city_suppressed ..." check outside whole 'if (can_player_see_city_internals()) {} else {}' construct. Both for clarity and performance (no point to figure out can_player_see_city_internals() if the simpler checks are about to prevent the send anyway. --------------------------------------------------------------------- Ticket Status: Reporter: lexxie9952 Owner: (None) Type: Bugs Status: Open Priority: 5 - Medium MileStone: 3.0.1 Component: Server Severity: 5 - Medium Resolution: None --------------------------------------------------------------------- Ticket details: in freeciv/freeciv master branch, citytools.c, broadcast_city_info Line 2182, https://github.com/freeciv/freeciv/blob/5bcfc01b1cd0a721764ff64b8706a037826294d1/server/citytools.c#L2182 We iterate all players to see if they can see inside the city, presumably so they can "see" the internal city data. But each time they qualify to see it, we make that info accessible to the city **owner.** So, for every pplayer who can see inside the city, we repeatedly and redundantly allow the city owner access to see inside their city. Not the pplayer who is supposedly able to see. This ticket submitted by request... copy-paste of cazfi response: "Can you open a ticket about that? The code makes no sense, while it's less clear what exactly is wrong with it (seems to me that it has actually multiple problems, but need to confirm that with more time)" NOTE: for the purposes FCW was able to achieve, changing powner to pplayer worked. If there are other issues when all this gets rewritten, please send me a courtesy notification. Thanks! -- Ticket information of Freeciv project Freeciv Project is hosted on OSDN Project URL: https://osdn.net/projects/freeciv/ OSDN: https://osdn.net URL for this Ticket: https://osdn.net/projects/freeciv/ticket/44158 RSS feed for this Ticket: https://osdn.net/ticket/ticket_rss.php?group_id=12505&tid=44158