patchFreeciv - Patches: patch #3974, Replace terrain.ruleset's...

 
 
Show feedback again

patch #3974: Replace terrain.ruleset's (pollution|fallout)_(food|shield|trade)_penalty with yet another output effect

Submitted by:  Sveinung Kvilhaugsvik <sveinung>
Submitted on:  Fri Jul 5 18:45:01 2013  
 
Category: generalPriority: 5 - Normal
Status: DonePrivacy: Public
Assigned to: Marko Lindqvist <cazfi>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)

Tue Jul 23 21:11:37 2013, comment #11:

Committed, though I noticed that documentation in README.effects could be better - Output_Per_Tile and Output_Tile_Punish_Pct have just opposite wording with no indication why they are both needed instead of giving just negative value to the other.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Tue Jul 23 21:02:40 2013, SVN revision 23099:

Replaced penalties defined for Polluted and Fallout tiles with new Output_Tile_Punish_Pct
effect.

Patch by Sveinung Kvilhaugsvik

See patch #3974

(Browse SVN revision 23099)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Fri Jul 19 09:50:37 2013, comment #9:

Patch #3990 changed code used as context for this patch. Make it apply again. No other changes.

(file #18348)

Anonymous
Fri Jul 19 02:09:22 2013, comment #8:

> Of those options I think one that uses nreqs exclusively makes things more obvious,

Agreed.

Tested all rule sets in autogames. civ2civ3 had a different result sometime after turn 600. multiplayer wasn't completed but was the same (except the order of the stored vars) at turn 600. The rest had the same result.

Anonymous
Tue Jul 16 21:20:05 2013, comment #7:

Of those options I think one that uses nreqs exclusively makes things more obvious, both for human reader and maybe for helptext generation if we implement it for this feature.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Thu Jul 11 14:57:45 2013, comment #6:

I don't mind changing it. (I stopped believing that using a minus sign would be easier to read when you missed it)

Attached are two alternatives. One use nreqs exclusively. The other use nreqs and addition.

I haven't tested them in auto games yet. I can do that for the one you prefer.

(file #18254, file #18255)

Sveinung Kvilhaugsvik <sveinung>
Project Member
Wed Jul 10 19:31:08 2013, comment #5:

Sorry I missed the minus sign. For question if nreqs or current method of subtraction should be used I think I would slightly prefer nreqs for consistency - that's the way other similar situations in our rulesets have been handled, so it's what people are used to and expect (just like I did). But as "slight" preference, any opposing opinion expressed is likely to get honoured.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Wed Jul 10 16:15:45 2013, comment #4:

> If tile has both Pollution and Fallout, all three effects are active, resulting in 50% + 50% + 25% = 125% penalty?

That is 50% + 50% + -25% = 75% penalty.

Fun fact: I started using nreqs. Then I rewrote the effect definitions using negative numbers. I believed that would be easier to read.

Should I rewrite the effect definitions using nreqs and no addition or subtraction at all?

Sveinung Kvilhaugsvik <sveinung>
Project Member
Wed Jul 10 14:03:12 2013, comment #3:

Looking the effects definitions I wonder why there's no nreqs for anything. If tile has both Pollution and Fallout, all three effects are active, resulting in 50% + 50% + 25% = 125% penalty? If effect that has only "Pollution" as req would have nreq "Fallout" (or vice versa), this would be 50% + 25% = 75% instead.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Mon Jul 8 01:44:33 2013, comment #2:

> On the contrary, this is needed as part of generic extras to get rid of the way pollution is hardcoded to be S_POLLUTION & S_FALLOUT.

Good. I hoped that would be the case.

> I saw only one coding style issue: "output" was declared in the middle of the block it now is in (variables must be declared before any code lines in the same block)

Fixed in attached version.

It has been tested by running an autogame with and without the patch until the game ended or turn 500 (or above) was reached. The end save files were then compared. The following rule sets was tested: alien, civ2, civ2civ3, classic and multiplayer. Civ1 refused to run auto games with and without it.

(file #18208)

Sveinung Kvilhaugsvik <sveinung>
Project Member
Sat Jul 6 07:04:38 2013, comment #1:

> If this path delays the extra generalization feel free to
> reject it.


On the contrary, this is needed as part of generic extras to get rid of the way pollution is hardcoded to be S_POLLUTION & S_FALLOUT.

I saw only one coding style issue: "output" was declared in the middle of the block it now is in (variables must be declared before any code lines in the same block)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Fri Jul 5 18:45:01 2013, original submission:

Replace terrain.ruleset's (pollution|fallout)_(food|shield|trade)_penalty with yet another output effect and remove the hard coded reference to pollution and fallout when applying the output penalty.

To avoid having to rebalance rule sets the new effect is in the location where the current pollution penalty is applied. To promote the same goal it decides how much to subtract in stead of how much to keep. (I have a version where the effect decides how much to keep. That results in 3 becoming 1 in stead of 3 becoming 2)

The current settings are applied in two steps: pollution and fallout. The effect is applied in one step. Because conversions to integer only happens once a tile that has pollution + fallout may produce a different result than the current code depending on the percentage to subtract and the amount of output.

This patch applies on top of patch 3965 and patch 3967. It has been tested using the experimental rule set. An auto game gave the same save file. The pollution, fallout and their combination were verified manually.

If this path delays the extra generalization feel free to reject it. If not let me know so I can test it for the other rule sets as well.

Sveinung Kvilhaugsvik <sveinung>
Project Member

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #18348:  pollutedTile-5.patch added by None (28kB - text/x-patch)
file #18255:  pollutedTile-nreqs.patch added by sveinung (28kB - text/x-patch)
file #18208:  pollutedTile-2.patch added by sveinung (25kB - text/x-patch)
file #18205:  pollutedTile.patch added by sveinung (25kB - text/x-patch)

 

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

    Date Changed By Updated Field Previous Value => Replaced By
    Tue Jul 23 21:11:37 2013cazfiStatusReady For Test=>Done
      Assigned toNone=>cazfi
      Open/ClosedOpen=>Closed
    Fri Jul 19 09:50:37 2013NoneAttached File-=>Added pollutedTile-5.patch, #18348
    Tue Jul 16 21:20:05 2013cazfiCategoryNone=>general
      StatusNone=>Ready For Test
      Planned Release=>2.6.0
    Thu Jul 11 14:57:45 2013sveinungAttached File-=>Added pollutedTile-addition-nreqs.patch, #18254
      Attached File-=>Added pollutedTile-nreqs.patch, #18255
    Mon Jul 8 01:44:33 2013sveinungAttached File-=>Added pollutedTile-2.patch, #18208
    Fri Jul 5 18:45:01 2013sveinungAttached File-=>Added pollutedTile.patch, #18205
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup