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 30 Jun 2012 10:33:47 AM UTC  
 
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.0

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

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

 

Mon 09 Sep 2013 09:52:45 PM UTC, 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 09 Sep 2013 09:52:38 PM UTC, 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 07 Sep 2013 08:32:53 AM UTC, comment #2:

Untested patches

(file #18905, file #18906)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat 30 Jun 2012 10:53:06 AM UTC, 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 30 Jun 2012 10:33:47 AM UTC, 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.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 9 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 09 Sep 2013 09:52:57 PM UTCcazfiStatusReady For Test=>Fixed
      Assigned toNone=>cazfi
      Open/ClosedOpen=>Closed
    Sat 07 Sep 2013 08:32:53 AM UTCcazfiAttached 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 30 Jun 2012 10:33:47 AM UTCjtnCarbon-Copy-=>Added bardo
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup