patchFreeciv - Patches: patch #1341, Replace Improvement replaced_by...

 
 
Show feedback again

patch #1341: Replace Improvement replaced_by with a requirements vector

Submitted by:  pepeto <pepeto>
Submitted on:  Sat Oct 17 14:20:27 2009  
 
Category: rulesetsPriority: 1 - Later
Status: DonePrivacy: Public
Assigned to: Emmet Hikory <persia>Open/Closed: Closed
Planned Release: 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.

 

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

Sat Jan 3 11:30:01 2015, comment #9:

> once this is in we can clean up rulesets for not protecting
> against the rare case of city having two Barracks at once
> (guards against getting double effects)


Now patch #5650

Marko Lindqvist <cazfi>
Project Administrator
Mon Jun 30 05:51:06 2014, SVN revision 25333:

Use obsolete_by vector for Improvement replaced_by

See patch #1341

(Browse SVN revision 25333)

Emmet Hikory <persia>
Project MemberIn charge of this item.
Sun Jun 29 09:15:54 2014, comment #7:

Ah, heh, indeed. Behaviour (2) seems so obviously correct I hadn't realised the bug was present before. Do we have a bug number for that? Does it exist/should it be fixed for 2.4.3/2.5.0?

Emmet Hikory <persia>
Project MemberIn charge of this item.
Sun Jun 29 06:15:30 2014, comment #6:

So (2) has behavior changed. That's probably only good, and means that once this is in we can clean up rulesets for not protecting against the rare case of city having two Barracks at once (guards against getting double effects)

Marko Lindqvist <cazfi>
Project Administrator
Sun Jun 29 05:24:31 2014, comment #5:

1) is now tested, and indeed works as expected: learning Gunpowder causes one to sell all Barracks, even if one has not built any Barracks II

2) Is also tested: attempts to force worklist inclusion of the obsolete Barracks I in a city containing Barracks II by a player ignorant of Gunpowder are ignored.

Further, I've inspected the codepath more closely: while there are guards in the AI and the client, the server enforces the restriction that only non-obsolete things may be built: while can_player_build_improvement_direct() is sometimes called without the caller checking improvement_obsolete(), this is always checked at some point in the call stack within cityturn.c between the can_player_build_improvement_direct() call and actually processing the building.

Emmet Hikory <persia>
Project MemberIn charge of this item.
Sun Jun 29 00:49:18 2014, comment #4:

I'll test these conditions to verify, but from code interpretation:

1) Because, unlike other requirements vectors, obsolete_by is disjunctive, Barracks is interpreted as obsolete by improvement_obsolete(), so handled appropriately. Thinking about this, I suppose another way to consider bug #21419 would have been to change the semantics to "sustained_until" or similar, and have all the requirements negated, but if we did that, the result would end up the same.

2) All calls to building_upgrades_to() are from constructions that check can_city_build_improvement_foo(), so that one would build Barracks in that city. I suspect this is a bug, and that this behaviour should be prevented. I don't beleive the client offers the choice, or the AI tries to do it because of improvement redundancy, but that's not sufficient guard (one could construct a hacked client that would miss a client-only guard).

Emmet Hikory <persia>
Project MemberIn charge of this item.
Sat Jun 28 21:53:38 2014, comment #3:

Does this cause behavior changes?

What happens in these situations:
1) You learn Gunpowder, but haven't built any Barracks II yet (all existing Barracks should be sold)
2) You conquer city with Barracks II, but don't know Gunpowder yourself, so you (try to) build Barracks

Marko Lindqvist <cazfi>
Project Administrator
Sat Jun 28 16:13:00 2014, comment #2:

As it turns out, the obsolete_by vector can also replace replaced_by, with a couple helper functions for parsing (in large part thanks to the work in bug #21419).

Note that in the event a ruleset author adds multiple buildings to the obsolete_by vector, only the first of them will be considered a sensible target for automatic worklist upgrades: I'm not sure where this ought be documented (if anywhere).

(file #21187)

Emmet Hikory <persia>
Project MemberIn charge of this item.
Thu Jan 2 17:11:31 2014, comment #1:

obsolete_by done by patch #3941.
replaced_by still not done.

Jacob Nevins <jtn>
Project Administrator
Sat Oct 17 14:20:27 2009, original submission:

Remove the following fields from the 'struct impr_type':

  • obsolete_by
  • replaced_by

Replace them by 'requirement_vector nreqs'.

pepeto <pepeto>
Project Member

 

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

Attach File(s):
   
   
Comment:
   

Attached Files

 

Digest:
   patch dependencies.

Digest:
   patch dependencies.

 

Carbon-Copy List
  • -unavailable- added by cazfi (Posted a comment)
  • -unavailable- added by persia (Updated the item)
  • -unavailable- added by jtn (Updated the item)
  • -unavailable- added by dmarks (Updated the item)
  • -unavailable- added by pepeto (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.

     

    Error: not logged in

     

     

    Follow 13 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon Jun 30 06:07:28 2014persiaStatusReady For Test=>Done
      Open/ClosedOpen=>Closed
    Sat Jun 28 16:13:00 2014persiaAttached File-=>Added use-obsolete_by-vector-for-improvement-replaced_by.patch, #21187
      StatusNone=>Ready For Test
      Assigned toNone=>persia
      Planned Release=>2.6.0
      SummaryImprovement negated requirements=>Replace Improvement replaced_by with a requirements vector
    Thu Jan 2 17:11:31 2014jtnDependencies-=>Depends on patch #3941
    Mon Jun 18 01:15:23 2012jtnPlanned Release2.4.0=>
    Wed Oct 27 13:54:13 2010pepetoPlanned Release2.3.0=>2.4.0
    Sun Dec 20 09:41:11 2009dmarksPlanned Release=>2.3.0
    Sat Oct 17 14:31:36 2009pepetoDependencies-=>patch #1338 is dependent
    Sat Oct 17 14:20:36 2009pepetoPriority5 - Normal=>1 - Later
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup