Project

General

Profile

Bug #700910

2.6+ city_size_change

Added by frank e 12 months ago. Updated 11 months ago.

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

0%

Estimated time:

Description

Noted while offline, maybe already fixed:  More than one year ago
(SVN 33766) server/cityturn.c bool city_size_change() emitted the
new "city_size_change" signal (args: city, delta, reason) even
if city_increase_size() later fails, e.g., city needs improvement.

IOW, the deprecated 2.5.x "city_growth" signal (args: city, size)
does not match "city_size_change" delta + city.size for delta > 0
if city_increase_size() later fails.

I thought that #692477 is already fixed by the new signal, but as
of SVN 33766 the new signal was _worse_ than the old "city_growth" 
for the purposes of tutorial scripts.

Suggested fix if still relevant:  All "city_size_change" signals
must indicate the effective non-zero delta after the city.size was
actually changed.  Likewise all effective size changes (including
new size 0 for any reason) should trigger a "city_size_change".

Apparently still wrong on GitHub as of today.

0004-Emit-city_size_change-signal-only-after-successfull-.patch (4.12 KB) 0004-Emit-city_size_change-signal-only-after-successfull-.patch Marko Lindqvist, 2017-10-03 01:12 AM
script.lua (16.3 KB) script.lua radius/script.lua frank e, 2017-10-03 01:43 AM

Related issues

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

History

#1 Updated by frank e 12 months ago

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

#2 Updated by Marko Lindqvist 12 months ago

  • Status changed from New to In Progress

#3 Updated by Marko Lindqvist 12 months ago

Attached patch makes it to emit city_size_change only if size increase succeeded.

Size change to 0 means the city gets destroyed, so there's no city to emit city_size_change signal about. Listen city_destroyed signal instead.

#4 Updated by frank e 12 months ago

First part looks good, but in a tutorial I'd be also interested in a city growth for unknown reasons (NULL). The NULL case could trigger a deprecated city_growth without city_size_change. So in my script I'll change (old):

function radius_city_size_change( city, delta, reason )
   if delta > 0   then                 -- 2.6.x delta instead of size
      return radius_city_growth( city, city.size + delta )
   end                                 -- call also checks magic(), this
end                                    -- kludge isn't consequent, sorry

if radius_vers < 260 or true           -- (disabled, cf. HRM #692477)
   then  signal.connect( "city_growth",      "radius_city_growth" )
   else  signal.connect( "city_size_change", "radius_city_size_change" )
end
to (new):
function radius_city_size_change( city, delta, reason )
   if delta > 0   then                 -- 2.6.x delta instead of size
      return radius_city_growth( city, city.size )
   end                                 -- call also checks magic(), this
end                                    -- kludge isn't consequent, sorry

if radius_vers < 260                   
   then  signal.connect( "city_growth",      "radius_city_growth" )
   else  signal.connect( "city_size_change", "radius_city_size_change" )
end

#5 Updated by Marko Lindqvist 12 months ago

frank e wrote:

First part looks good, but in a tutorial I'd be also interested in a city growth for unknown reasons (NULL).

There's always a reason city grows.

#6 Updated by Marko Lindqvist 12 months ago

  • Status changed from Resolved to Closed
  • Assignee set to Marko Lindqvist
  • Priority changed from High to Normal

#7 Updated by frank e 11 months ago

Marko Lindqvist wrote:

There's always a reason city grows.

Note to myself, maybe...

 
  if (real_change != 0 && reason != NULL) {
      int id = pcity->id;

      script_server_signal_emit("city_size_change", 3,

...should be...
  if (real_change != 0) {
      int id = pcity->id;

      fc_assert(reason);
      script_server_signal_emit("city_size_change", 3,

...to nail always.

#8 Updated by Marko Lindqvist 11 months ago

Note that C-code can internally use NULL reason to indicate called functions that signal should not be emitted. This is done at least to avoid duplicate signal (not sure if there's such a case any longer after my last rearrangements, though)

Also available in: Atom PDF