Project

General

Profile

Bug #736822

City map in city dialog: working tile sometimes not updated

Added by Pierre R over 2 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Client
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:

Description

Launch Freeciv 2.6.0-beta2, gui-gtk-3.0 client.
- Open attached savegame, take player pi and start
- Open Gao's city dialog
- In the city map, click on the city, this organizes tiles automatically
- Click on the grassland working tile to free it
=> nothing happens, the grassland still appears working.
- Close the city dialog and reopen it. The grassland working tile has changed. The actions worked as expected but the city map was not redrawn because production didn't change.

freeciv-T0001-Y-3950-auto.sav.gz (16.8 KB) freeciv-T0001-Y-3950-auto.sav.gz Pierre R, 2018-03-03 04:45 PM
25_736822.sav.bz2 (13.7 KB) 25_736822.sav.bz2 S2_5 equivalent savegame created in editor Jacob Nevins, 2018-03-04 01:36 AM
m-30-26-25-refresh-city-dialog-worked-tiles.patch (3.93 KB) m-30-26-25-refresh-city-dialog-worked-tiles.patch Jacob Nevins, 2018-03-04 04:06 PM

History

#1 Updated by Jacob Nevins over 2 years ago

Reproduced.

What seems to be going wrong for me is that the initial click on the city centre to auto-arrange citizens does not cause the city map to be updated. For me, the savegame starts with the right-center tile (due east of city centre) worked; auto-arrange changes this to the tile southeast of the city centre (which I can see on the main map immediately with View / City Production Levels enabled), but the map in the city dialog doesn't reflect this. If I subsequently click on the grassland that appears to be work (east) nothing happens, of course, because it isn't. Clicking on the tile that's really worked (SE) does update the display.

It seems to be client-independent; I can reproduce it with all of freeciv-gtk3, -gtk2, -qt, and -sdl2.

#2 Updated by Jacob Nevins over 2 years ago

It's reproducible in S2_5, too, having set up a city in a 2.5 savegame with the same coordinates and terrain with the editor.

#3 Updated by Jacob Nevins over 2 years ago

Here is a guess at what is going on.

PACKET_TILE_INFOs convey information about changes to which tiles are worked by which cities, which will be a natural consequence of auto-arranging workers.

I went looking for the mechanism by which PACKET_TILE_INFOs could cause an open city dialog to be refreshed, and failed to find such a mechanism. CITY_INFO and CITY_SHORT_INFO do, though. (I think the key function that causes a city dialog refresh is refresh_city_dialog().)

In this case, the tile previously chosen (E) and the tile chosen by auto-arrange (SE), while different, have the exact same terrain so give the exact same bonus to the city. So it's plausible that there is no net change to the city parameters at all, so we never get as far as triggering refresh_city_dialog() from a city packet.

And indeed, if I use the editor to make the SE tile different (add Resources), then this symptom goes away, supporting this theory.

We need to make changes to worked tiles (and in principle other tile changes) able to trigger a city dialog refresh. For protocol-frozen branches the only way is to have handle_tile_info() look at whether it's a tile relevant to a current city dialog or something.
(On later branches, I vaguely wondered about changing the protocol to put a bitmap of worked tiles in the city packet, but in principle the city dialog cares about other tile changes -- if the terrain changed, or another city started working a tile, the city dialog needs a refresh. So that's probably a non-starter.)

#4 Updated by Jacob Nevins over 2 years ago

have handle_tile_info() look at whether it's a tile relevant to a current city dialog or something

The trick here would be finding out which cities have ptile in their map cheaply. But I can't think of a cheap way to do that; I think we'd have to either iterate over all cities or iterate outward from ptile to the max city radius. Neither sounds like a thing we'd want the client doing on every PACKET_TILE_INFO.

However, in this specific case we already know the IDs of the affected cities (from the 'worked' field), so we can just call refresh_city_dialog() on those. That is pretty cheap; it sets a flag in the city structure, and the GUI's idle callback will eventually iterate over all cities looking for that flag (which was probably going to happen anyway).

The attached patch does this. I've briefly tested that it resolves the original symptom, but not tested it any more thoroughly that than. If it is to go in 2.5.11 it could do with some more exercise than that.

I think that should completely fix the common case of clicking auto-arrange in a city dialog not updating that dialog. It may not update adjacent cities' dialogs though, and there are certainly remaining situations where a tile change ought to be reflected in an open city dialog but isn't.

It might also improve the odds of a city dialog being updated if a hostile unit moves onto a worked city tile. That should trigger a worked-tile change.

#5 Updated by Jacob Nevins over 2 years ago

  • Assignee deleted (Jacob Nevins)

I won't be able to commit this in the next few days. If anyone wants to test it further then commit it so it gets even more exercise, feel free.
(I still intend for it to be committed before the next, now delayed, releases.)

#6 Updated by Jacob Nevins over 2 years ago

  • Target version changed from 2.5.11 to 2.5.12

#7 Updated by Jacob Nevins over 2 years ago

  • Status changed from Resolved to Closed
  • Assignee set to Jacob Nevins

Also available in: Atom PDF