Project

General

Profile

Bug #690756

Make Lua city_size_change signal consistent wrt (City).size

Added by Jacob Nevins 3 months ago. Updated about 1 month ago.

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

0%

Estimated time:

Description

When the "city_size_change" Lua signal was originally added in gna bug #24115, cazfi wrote:

Note that city_size() is practically undefined at the time of the signal execution (the change might be already applied, or not)

Looking at the code, this is still true, but it doesn't look like it has to be; on a quick look, all the signal sites now look to me like they could easily be rearranged to go one way or the other (this code has been rearranged a bit since it first went in).
  • city_reduce_size: signal before change
  • city_change_size: signal before change
  • city_populate: signal after change
  • do_city_migration (1): signal before change
  • do_city_migration (2): signal after change
  • city_add_unit: signal before change

I think we should probably standardise on signal before change (avoiding awkwardness when the change is to size 0).

If it's harder than it looks, we should instead add the above comment to the Lua reference manual (which I was about to do when I disappeared down this rabbit hole).

We should probably get this stable before 2.6.0.


Related issues

Related to Freeciv - Bug #692477: 25x city_growth incompleteClosed

History

#1 Updated by frank e 3 months ago

  • Related to Bug #692477: 25x city_growth incomplete added

#2 Updated by Marko Lindqvist 3 months ago

Jacob Nevins wrote:

I think we should probably standardise on signal before change (avoiding awkwardness when the change is to size 0).

Is the signal ever emitted when city is first created, meaning change FROM size 0?

#3 Updated by Jacob Nevins 3 months ago

I don't think so (only "city_built").

#4 Updated by Jacob Nevins 3 months ago

(...and FTAOD I think that's correct; emitting city_size_change in this situation would be needlessly confusing. NB city_size_change is not emitted even if the unit builds a city of size>1, even though that's implemented internally as create-then-grow; a script that cared would have to hardcode behaviour or infer that this had happened from the city-builder's unit_lost signal.)

#5 Updated by frank e 3 months ago

Jacob Nevins wrote:

I don't think so (only "city_built").

Yes, for some time I simply mapped radius_city_built() to radius_city_growth( 1 ), just because size 2 wasn't reliable wrt "add to city" size changes. Later I replaced it by some "last biggest city size seen per player", because all I want is ONE tutorial message for biggest city size 2, 5, 8, ..., 17 per player.

Likewise I let radius_city_destroyed() call radius_partisans( city, loser, destroyer, 1, 1 ) if the loser wasn't the destroyer.

JFTR, because it's not obvious for me, and I wasn't aware of Gna! 24115: If I get a new city_size_change( city, change, reason ) I can emulate an old and now fixed city_growth() by calling an existing radius_city_growth( city.size + change ) for any change > 0. Please holler if that's wrong, I want the same script.lua working for 2.4+2.5+2.6 (and later 3.0), but of course with all deprecations honored.

#6 Updated by Marko Lindqvist about 2 months ago

  • Target version changed from 2.6.0 to 2.6.0-beta2

Jacob Nevins wrote:

I think we should probably standardise on signal before change (avoiding awkwardness when the change is to size 0).

Bug #700910 is now changing all the city growth cases to emit signal after the size change, when it's known that the change succeeded (and in case of partial success, 'change' value is what really happened)

This leaves size decrease the only case where signal is sent before the change.

#8 Updated by Marko Lindqvist about 1 month ago

  • Status changed from Resolved to Closed
  • Assignee set to Marko Lindqvist

Also available in: Atom PDF