bugFreeciv - Bugs: bug #19874, Effects affecting cities can't...

 
 
Show feedback again

bug #19874: Effects affecting cities can't depend on city tile properties

Submitted by:  Jacob Nevins <jtn>
Submitted on:  Sat Jun 30 10:33:47 2012  
 
Category: generalSeverity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Assigned to: Marko Lindqvist <cazfi>Open/Closed: Closed
Release: Operating System: Any
Planned Release: 2.5.0, 2.6.0Contains string changes: None

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Mon Sep 9 21:52:45 2013, SVN revision 23329:

Support city tile related requirements for effects targeting the city.

Reported by David Fernandez

See bug #19874

(Browse SVN revision 23329)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Mon Sep 9 21:52:38 2013, SVN revision 23328:

Support city tile related requirements for effects targeting the city.

Reported by David Fernandez

See bug #19874

(Browse SVN revision 23328)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat Sep 7 08:32:53 2013, comment #2:

Untested patches

(file #18905, file #18906)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat Jun 30 10:53:06 2012, comment #1:

> It seems to me that it could easily pass city_tile(pcity) and
> thus enable effects such as the above. Is there any reason not
> to do so?


It seems that these get_xxx_bonus() functions were originally written so that just the effects in our own rulesets would work, as opposed to make them more generic. I've considered such failures to pass obvious parameters as bugs in the past (fix for this one should certainly go to TRUNK and S2_4, I'm a bit undecided about S2_3: it would create incentive for ruleset authors to create rulesets that do not work in earlier 2.3.x releases)

> As usual with effects, the tricky part is edge conditions --
> whenever tile properties change, knock-on effects on all the
> above need to be reviewed.


That's one reason why Max_Trade_Routes is defined never to cause removal of existing traderoutes. I would just document the restrictions in the first phase. Passing the tile would anyway make it work better (in my view that failing to do so is a bug, not missing feature)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat Jun 30 10:33:47 2012, original submission:

In bug #16308 (comment 5), David Fernandez complains:

'When I use the requeriment "Special", "River", "Adjacent" in my rulesets, it seems to return always false, even when the city is placed near or over a river.

'This code from effects.ruleset never works (it is supposed to give aqueduct bonus in cities near rivers):'

I've confimed this. The reason seems simple: get_city_bonus() passes the tile argument of get_target_bonus_effects() as NULL.

It seems to me that it could easily pass city_tile(pcity) and thus enable effects such as the above. Is there any reason not to do so?

I believe this affects the following effects:

  • Capital_City, Gov_Center
  • Size_Adj, Size_Unlimit
  • Growth_Food
  • Rapture_Grow
  • City_Radius_Sq, City_Vision_Radius_Sq
  • Health_Pct
  • Migration_Pct
  • Trade_Revenue_Bonus, Max_Trade_Routes
  • Pollu_Prod_Pct
  • City_Build_Slots
  • Airlift
  • Happiness_To_Gold
  • Make_Content_Mil_Per, Make_Content_Mil
  • Martial_Law_Each, Martial_Law_Max
  • Make_Happy, No_Unhappy, Make_Content, Force_Content
  • Unhappy_Factor
  • Defend_Bonus
  • Visible_Walls, City_Image
  • Unit_No_Lose_Pop
  • Nuke_Proof
  • Incite_Cost_Pct, No_Incite, Spy_Resistant
  • HP_Regen

(As usual with effects, the tricky part is edge conditions -- whenever tile properties change, knock-on effects on all the above need to be reviewed. Perhaps that will mostly Just Work, but I guess it needs an audit.)

Jacob Nevins <jtn>
Project Administrator

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #18905:  CityBonusTile.patch added by cazfi (442B - text/x-diff)
file #18906:  CityBonusTile-S2_5.patch added by cazfi (408B - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by cazfi (Posted a comment)
  • -unavailable- added by jtn (Submitted the item)
  • -unavailable- added by jtn
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Error: not logged in

     

     

    Follow 9 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon Sep 9 21:52:57 2013cazfiStatusReady For Test=>Fixed
      Assigned toNone=>cazfi
      Open/ClosedOpen=>Closed
    Sat Sep 7 08:32:53 2013cazfiAttached File-=>Added CityBonusTile.patch, #18905
      Attached File-=>Added CityBonusTile-S2_5.patch, #18906
      CategoryNone=>general
      StatusNone=>Ready For Test
      Planned Release=>2.5.0, 2.6.0
    Sat Jun 30 10:33:47 2012jtnCarbon-Copy-=>Added bardo
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup