bugFreeciv - Bugs: bug #21403, Pillaging EF_ALWAYS_ON_CITY_CENTER...

 
 
Show feedback again

bug #21403: Pillaging EF_ALWAYS_ON_CITY_CENTER extras from city centers

Submitted by:  Marko Lindqvist <cazfi>
Submitted on:  Tue 31 Dec 2013 01:54:15 AM UTC  
 
Category: generalSeverity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Assigned to: Marko Lindqvist <cazfi>Open/Closed: Closed
Release: Operating System: None
Planned Release: 2.5.0-beta1, 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.

 

(Jump to the original submission Jump to the original submission)

Wed 22 Jan 2014 05:38:32 PM UTC, SVN revision 24215:

Respect "AlwaysOnCityCenter" and "AutoOnCityCenter" flags when determining
which roads or bases can be pillaged from city center.

See bug #21403

(Browse SVN revision 24215)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Wed 22 Jan 2014 05:37:59 PM UTC, SVN revision 24214:

Respect "AlwaysOnCityCenter" and "AutoOnCityCenter" flags when determining
which extras can be pillaged from city center.

See bug #21403

(Browse SVN revision 24214)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Thu 16 Jan 2014 11:11:28 PM UTC, comment #6:

- Do not allow pillage of Auto_On_City_Center extras if they would get rebuilt.
- As the code related to checking flags grow, made a separate function of it in TRUNK instead of having duplicate checks

(file #19765, file #19766)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Tue 07 Jan 2014 04:35:03 AM UTC, comment #5:

>> (destroying railroads before losing city)


>...that doesn't allow for this scorched-earth strategy. Hm.


It's not a clean rule anyway - something may, or may not, cause the extra pop back up before city then is actually lost. So I think you're right, pillage of AutoOnCityCenter extras should be forbidden if city currently has requirements for rebuilding it.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat 04 Jan 2014 08:07:49 PM UTC, comment #4:

> It's not guaranteed that the extra will pop up back, for
> example if city has new owner who does not know required
> tech to rebuild it

Good point -- in my world, the can-we-pillage test would have to include "would this extra currently be automatically added to a city center".

However:

> (destroying railroads before losing city)

...that doesn't allow for this scorched-earth strategy. Hm.

> or, in case of some potential uses of extras by ruleset
> author, is not to be penalised by bad extra

It does allow this, though (you liberate a city with a Rat Citadel due to the previous owner's "Government", "Slob", "Player" administration, and have to clean it up).

> OTOH the case where it does pop back up should work in a
> more reliable way than "next time new tech is discovered"
> -> new ticket

I've raised patch #4408 for my previous suggestions, maybe it covers this too. (As usual with requirements, it's not practical to test all transitions, so we have to pick the most useful ones.)

> Maybe we need to give ruleset author control over this,
> as it would be nice to protect user from pillaging "always
> pops back up" -extras.

I'm not sure what semantics it would have, since requirements specification is driven by current state and not transitions.
I suspect that the only practical way to allow the scorched-earth thing in a way we developers won't break in future is to add the extras via Lua script rather than AutoOnCityCenter -- that way the ruleset author can arrange that they appear only when a player discovers the relevant tech (and only the first time, in the presence of tech loss).

Jacob Nevins <jtn>
Project Administrator
Sat 04 Jan 2014 07:46:09 PM UTC, comment #3:

> Should it even be possible for units to remove AutoOnCityCenter
> extras?


1) Maybe it's semantics, but unlike with the "ALWAYS" -flag this should not be hard rule.

2) It's actually right thing to do in some situations. It's not guaranteed that the extra will pop up back, for example if city has new owner who does not know required tech to rebuild it (destroying railroads before losing city) or, in case of some potential uses of extras by ruleset author, is not to be penalised by bad extra. OTOH the case where it does pop back up should work in a more reliable way than "next time new tech is discovered" -> new ticket

Maybe we need to give ruleset author control over this, as it would be nice to protect user from pillaging "always pops back up" -extras.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat 04 Jan 2014 02:06:18 PM UTC, comment #2:

Should it even be possible for units to remove AutoOnCityCenter extras?
Since they are liable to pop into existence when a new tech is discovered, I tend to think they should not be removeable.
The distinction for me is that AlwaysOnCityCenter overrides other requirements (it would be equivalent to adding an "or CityTile(Center)" requirement to an AutoOnCityCenter extra, if it were possible to add "or" rather than "and" requirements). AutoOnCityCenter should be "whenever it's possible for extra to exist on city centre tile, it should exist there", I think?
(...maybe we should be checking upgrade_all_city_extras() on city tile terrain change; another ticket...)

(Sorry if there's a ticket about this already, but I didn't spot it -- patch #4188 is vaguely in the same area.)

Jacob Nevins <jtn>
Project Administrator
Sat 04 Jan 2014 06:41:33 AM UTC, comment #1:

New TRUNK version

- Updated against current svn
- Moved EF_ALWAYS_ON_CITY_CENTER check inside can_remove_extra() & player_can_remove_extra(). This is logically correct place, and also handles the theoretical case that someone has extra removable by some other means than pillaging with "AlwaysOnCityCenter" flag (Uncleanable pollution on city center?)

(file #19662)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Tue 31 Dec 2013 01:54:15 AM UTC, original submission:

Currently there's check that no road type gets pillaged from city center. That's no longer correct. It's both possible that some other extra must always be present in city center, and that some road needs not. This depends on EF_ALWAYS_ON_CITY_CENTER flag.

Fix attached

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #19765:  CenterPillage-3.patch added by cazfi (5kB - text/x-diff)
file #19662:  CenterPillage-2.patch added by cazfi (4kB - text/x-diff)
file #19576:  CenterPillage.patch added by cazfi (840B - text/x-diff)
file #19577:  CenterPillage-S2_5.patch added by cazfi (2kB - text/x-diff)
file #19575:  ExtraRmReqs.patch added by cazfi (12kB - text/x-diff)

 

Depends on the following items: None found

Digest:
   task dependencies.

 

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

    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 11 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 22 Jan 2014 05:38:41 PM UTCcazfiStatusReady For Test=>Fixed
      Assigned toNone=>cazfi
      Open/ClosedOpen=>Closed
    Thu 16 Jan 2014 11:11:28 PM UTCcazfiAttached File-=>Added CenterPillage-3.patch, #19765
      Attached File-=>Added CenterPillage-S2_5-3.patch, #19766
    Thu 09 Jan 2014 05:40:14 AM UTCcazfiPlanned Release2.5.0, 2.6.0=>2.5.0-beta1, 2.6.0
    Sat 04 Jan 2014 06:41:33 AM UTCcazfiAttached File-=>Added CenterPillage-2.patch, #19662
    Tue 31 Dec 2013 02:03:43 AM UTCcazfiAttached File-=>Added CenterPillage.patch, #19576
      Attached File-=>Added CenterPillage-S2_5.patch, #19577
      Planned Release2.6.0=>2.5.0, 2.6.0
    Tue 31 Dec 2013 01:54:15 AM UTCcazfiAttached File-=>Added ExtraRmReqs.patch, #19575
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup