WhipAssist Fixes & Enhancements

[Dresden]

This was originally going to be a simple bug report, but then I found a second bug.... and then when I looked in the SDK source to suggest fixes for those I came upon a suggestion and a potential new feature too. So since this has grown monstrously out-of-hand, I'm making it a new topic.

The Current Calculation
For reference, here is how CvMainInterface currently calculates whip overflow for display in the city screen. The CDA and the Civ4lerts each make essentially the same calculation too. (Perhaps it should be farmed out to a utility function.)

iHurryOverflow = pHeadSelectedCity.hurryProduction(HURRY_WHIP) - (pHeadSelectedCity.getProductionNeeded() - pHeadSelectedCity.getProduction())

Bug #1: Production Modifers
If the current build has any production modifiers, those get "reversed" when the overflow gets calculated. Here are 2 examples from my Noble/Epic NCXI game:

For Yekaterinburg, the Library has 42/135 hammers. The 2-pop whip adds 180 resulting in the BUG estimated overflow of 180 - (135 - 42) = 87. But after whipping, the actual overflow turns out to be just 47. Similarly for Yaroslavl', the estimated overflow is 59 but the actual is just 31. The majority of the discrepancy comes from not reversing those +100% modifiers.

The modifier reversal can thus be done in Python with the following that I essentially just converted from the SDK's CvCity::changeOverflowProduction() function:

100 * iHurryOverflow / city.getBaseYieldRateModifier(gc.getInfoTypeForString("YIELD_PRODUCTION"), city.getProductionModifier())

If that is used on the above two examples, the revised estimate for Yekaterinburg is 43 (actual was 47) and the revised estimate for Yaroslavl' is 29 (actual is 31). Pretty close now. This leads us into....

Suggestion: Add Current Production
The difference in those previous estimates comes from the fact that the city still contributes actual hammers to the build after the whip. In Yekaterinburg, my people were still producing 8 hammers after the whipping so the raw overflow was really 95; half of which gives us the 47 actual overflow. In Yaroslavl' I whipped away part of the production so they only added 4 more hammers giving a raw overflow of 63, resulting in the actual overflow of 31.

While you could MM the city to change the production any way you want after whipping, I expect that most people don't bother. And I claim without any evidence whatsoever that the post-whip production will normally be pretty close to the pre-whip production or at least it'll be a lot closer than assuming zero post-whip production. So why not factor that pre-whip production into the overlap like below?

iHurryOverflow = pHeadSelectedCity.hurryProduction(HURRY_WHIP) - pHeadSelectedCity.productionLeft() + pHeadSelectedCity.getCurrentProductionDifference(True, False)

Note there's also a small optimization there calling productionLeft() instead of using getProductionNeeded() - getProduction(). I'm sure it's essentially meaningless in terms of speed but why do two function calls when you can do one and make the C++ function do the subtraction? For Yekaterinburg, the estimate would now be spot on. For Yaroslavl, it'd be one over at 32. I think this will provide a better estimate than ignoring next turn's production more often than not. The sharp-eyed might notice that this estimate is slightly worse for settlers/workers since that True is telling getCurrentProductionDifference() to always ignore food surpluses, but again the estimate should still be better than completely ignoring next turn's production. However, I'm not done screwing with the overflow estimation...

Bug #2: Overflow Cap
The current estimates ignore the overflow cap. Somewhere along the line (I think in a vanilla patch) Firaxis implemented an overflow cap to reduce "pre-build" exploits and we need to factor this in too. The maximum overflow you can get is the production cost of the current build. Here's another example:

This time there are no production modifiers in play but we still get way less overflow. The reason is the cap. An epic scout costs 22 hammers, so our raw overflow of 49 (44 from the whip and 5 from next turn's production) becomes a measly 22. Those bastards! What'd they do with my 27 hammers? Let's come back to that in a minute. For now, enforcing the cap, which is nice and simple:

iMaxOverflow = min(pHeadSelectedCity.getProductionNeeded(), iHurryOverflow)

That'll take care of the cap and report a proper 22 for the scout situation.

Feature Request: Free Money
So what happened to those 27 hammers I lost? Did they just go away? Note the Treasury portion of my image. The turn of the whip, I had 105 gold and was losing 3 per turn, so the next turn I should have had 102. But instead, I have 129. There's my hammers; they got turned into gold!

An oddity about this is that this gold amount is calculated before the modifier reversal that I talked about first. As a result there is actually a fairly exploitive strategy based on this where you crank up a super-producing military city to build the cheapest unit available like a chariot and just delete them after production. Because of modifiers like the HE and a military academy you get a huge overflow that gets turned into money after the overflow cap is applied and the result is substantially better than if you had simply built Wealth. This has been discussed in overflow mechanics topics and I first saw it in an OCC game report. Despite that I can see how you might rule this part of overflow knowledge to be too obscure or too revealing of internal mechanics, but since I'm probably putting it in my Frankenstein version (BUGenstein?) anyway I figure I should propose it for general use.

This would require a tweaking of the alert and city bar output and either a bigger column or second column on the CDA. The money is calculated based on a global define so let's do that, again using similar logic to the SDK code.

iOverflowGold = max(0, iHurryOverflow - iMaxOverflow) * gc.getDefineINT("MAXED_UNIT_GOLD_PERCENT") / 100

So, if all fixes and suggestions are implemented, here's what the CvMainInterface.py code would look like:

						if (BugCityScreen.isShowWhipAssist() and pHeadSelectedCity.canHurry(HURRY_WHIP, False)):
							iHurryPop = pHeadSelectedCity.hurryPopulation(HURRY_WHIP)
							[COLOR="Red"][s]#iHurryOverflow = pHeadSelectedCity.hurryProduction(HURRY_WHIP) - (pHeadSelectedCity.getProductionNeeded() - pHeadSelectedCity.getProduction())[/s][/COLOR]
							[COLOR="Green"]iHurryOverflow = pHeadSelectedCity.hurryProduction(HURRY_WHIP) - pHeadSelectedCity.productionLeft() + pHeadSelectedCity.getCurrentProductionDifference(True, False)
							iMaxOverflow = min(pHeadSelectedCity.getProductionNeeded(), iHurryOverflow)
							iOverflowGold = max(0, iHurryOverflow - iMaxOverflow) * gc.getDefineINT("MAXED_UNIT_GOLD_PERCENT") / 100
							iHurryOverflow =  100 * iMaxOverflow / pHeadSelectedCity.getBaseYieldRateModifier(gc.getInfoTypeForString("YIELD_PRODUCTION"), pHeadSelectedCity.getProductionModifier())
							if (iOverflowGold > 0):
								szBuffer = localText.getText("INTERFACE_CITY_PRODUCTION_WHIP_PLUS_GOLD", (pHeadSelectedCity.getProductionNameKey(), pHeadSelectedCity.getProductionTurnsLeft(), iHurryPop, iHurryOverflow, iOverflowGold))
							else:[/COLOR]
								szBuffer = localText.getText("INTERFACE_CITY_PRODUCTION_WHIP", (pHeadSelectedCity.getProductionNameKey(), pHeadSelectedCity.getProductionTurnsLeft(), iHurryPop, iHurryOverflow))

Along with a new XML entry:

	<TEXT>
		<Tag>INTERFACE_CITY_PRODUCTION_WHIP_PLUS_GOLD</Tag>
		<English>%s1 (%d2)   %d3[ICON_ANGRYPOP] %d4[ICON_PRODUCTION] %D5[ICON_GOLD]</English>
		<French>%s1 (%d2)   %d3[ICON_ANGRYPOP] %d4[ICON_PRODUCTION] %D5[ICON_GOLD]</French>
		<German>%s1 (%d2)   %d3[ICON_ANGRYPOP] %d4[ICON_PRODUCTION] %D5[ICON_GOLD]</German>
		<Italian>%s1 (%d2)   %d3[ICON_ANGRYPOP] %d4[ICON_PRODUCTION] %D5[ICON_GOLD]</Italian>
		<Spanish>%s1 (%d2)   %d3[ICON_ANGRYPOP] %d4[ICON_PRODUCTION] %D5[ICON_GOLD]</Spanish>
	</TEXT>

And the results in-game on two of my above examples:

 

[EmperorFool]

Great work! I'll probably add an option whether or not to consider this turn's production. My assumption is that it will rarely be the same after whipping, mostly because I assign citizens to food-heavy tiles to regrow the population. But I agree that others may not play the same way and want the option.

Any idea why the gold calculation was off? It looks like a rounding error, but your code won't round up (which would cause an error). However, it's possible it is calculating this value differently which causes rounding in the opposite direction. Did you get the calculation from the SDK or mimic it?

 

[Dresden]

Great work! I'll probably add an option whether or not to consider this turn's production. My assumption is that it will rarely be the same after whipping, mostly because I assign citizens to food-heavy tiles to regrow the population. But I agree that others may not play the same way and want the option.

Yeah, an option would make sense there; you'd basically be choosing between which of the two extremes most closely resembles your play style.

Any idea why the gold calculation was off? It looks like a rounding error, but your code won't round up (which would cause an error). However, it's possible it is calculating this value differently which causes rounding in the opposite direction. Did you get the calculation from the SDK or mimic it?

I mimiced the calculation. The gold was off because of the production difference. Pre-whip that city was generating 6 hammers, post-whip it was only 5. With the "factor in current production" option , you get an estimated gold of 22+6=28; without it, you'd get an estimate of 22; and the actual value turned out to be 27.


EDIT:
By the way, the overflow gold can occur on just normal overflow too so something to consider is to separate that part out and display it whenever a normal build will cross the overflow cap too. That might wind up being a little too confusing though, especially when combined with stuff like the display for gold hurrying.

 

[EmperorFool]

I still want to expand WhipAssist into my original idea: to help you plan when to whip in the future -- not just decide whether or not to whip now. I wrote up a thread on this in the basic Civ4 forum I think.

In short, it would show you all of the crossover points where the number of whipped citizens decreases so you can see where you can maximize your overflow. I would add a third bar below the production bar to display this.

In the meantime, I could at least turn this into a new bar and show something like this:

Whip: [##X Pop#########++++Y Overflow++++++------Z Gold-------]

Where # would be the production color (blue-gray), + some other color, and - yellow (gold).

Not only would this keep the production bar text from overflowing its bar (as it will with long build names and some gold overflow), but I think it would be clearer what's happening. I could even probably add hover text to the new bar with full details.

You seem to be pretty handy with C++ and Python. Care to code some of these things you've suggested and post the files?

 

[Dresden]

You seem to be pretty handy with C++ and Python. Care to code some of these things you've suggested and post the files?

Sure I can do that. Although I'd need to stop hard-coding stuff since most of my suggestions would work best if they were optional (e.g. the "include current production" part of the above.) Is their a crash-course on how the BUG options mechanism works so that I can make things easier to merge?

 

[EmperorFool]

I've spent the past two weeks completely rewriting the BUG options and initialization code precisely to make stuff like this easier. Since converting from the old model to the new would take all of 5 minutes for a single option, there's no harm in doing it now and converting it later.

Heck, I'll go ahead and describe the new method along with the old for comparison. It will serve as a starter doc for when I write up docs for the new core.

[ Don't copy-n-paste code from here as I've used spaces to indent to make it more legible in the post. Spaces BAD! ]

Step 1: Define the Option

Start with this existing checkbox option (copy-n-paste it below itself in BugCityScreenOptions):

self.addOption(Option([B]"City_WhipAssist"[/B],
                      "City Screen", [B]"Whip Assist"[/B], True,
                      [I]"Whip Assist"[/I],
                      [I]"When checked, displays the population cost and overflow production for whipping and the gold cost for buying the item being produced."[/I],
                      InterfaceDirtyBits.CityScreen_DIRTY_BIT))

The parameters for the Option constructor are

1. ID - Must be unique across all options, preferably by using a prefix to group related options like "City_" or "Alerts_"
2. Section - Location within the INI file
3. Key - Actual INI key
4. Default - Used when no value is present in the INI file
5. Label - English label for the BUG Options screen
6. Help - English hover text for the BUG Options screen
7. Dirty Bit - Optional bit to set when the option is changed

Recommendation values for the bold parameters: "City_WhipAssistCountCurrentTurn" and "Whip Assist Count Current Turn". The label and help are optional as they are now pulled out of XML for translation. That change was made some time ago, not with the work I'm doing now.

Add an accessor function toward the bottom of the file:

def isShowWhipAssist(self):
    return self.getBoolean('City_WhipAssist')

[B]def isCountCurrentTurnInWhipAssist(self):
    return self.getBoolean('City_WhipAssistCountCurrentTurn')[/B]

New Core Method: XML

Here's the same option definition from the new BUG:

<option id="WhipAssist" key="Whip Assist" 
        type="boolean" default="True" 
        get="isShowWhipAssist" dirtyBit="CityScreen"/>

The "City_" ID prefix and "City Screen" section come from elsewhere in the XML file, shared by related options. Two methods of accessing the option are created for you by the new core (see below).

Step 2: Add it to the INI File

Add the default value to the INI file to make it easier for users to modify the INI file instead of using the options screen.

# Show whip pop/overflow and gold cost for hurrying
Whip Assist = 1

[B]# ...
Whip Assist Count Current Turn = 1[/B]

New Core Method: Nothing

You don't have to do this as the new core will create all the INI files automatically as they are needed.

Step 3: Create the Label and Hover Help Text

Add two <TEXT> tag blocks to BUGOptions_CIV4GameText.xml. Copy-n-paste this one below itself:

<TEXT>
	<Tag>TXT_KEY_BUG_OPT_CITY__WHIPASSIST_TEXT</Tag>
	<English>Whip Assist</English>
	<French>Whip Assist</French>
	<German>Anzeige für die Beschleunigung der Produktion</German>
	<Italian>Barra di produzione con accelerazione</Italian>
	<Spanish>Whip Assist</Spanish>
</TEXT>
<TEXT>
	<Tag>TXT_KEY_BUG_OPT_CITY__WHIPASSIST_HOVER</Tag>
	<English>When checked, displays the population cost and overflow production for whipping and the gold cost for buying the item being produced.</English>
	<French>When checked, displays the population cost and overflow production for whipping and the gold cost for buying the item being produced.</French>
	<German>Zeigt die Anzahl der Bürger, die für eine Beschleunigung der Produktion geopfert werden müssen, und den daraus entstehenden Überschuss an Produktion an sowie die Kosten, die anfallen, wenn das derzeitige Bauprojekt mit einer Goldzahlung beschleunigt werden kann</German>
	<Italian>Quando è selezionato, la Barra della Pruduzione della Schermata Cittadina mostra il costo in popolazione ed il surplus di produzione per l'accelerazione di produzione mediante sacrifici e il costo in oro per l'accelerazione di produzione mediante pagamenti relativi all'oggetto attualmente costruito.</Italian>
	<Spanish>When checked, displays the population cost and overflow production for whipping and the gold cost for buying the item being produced.</Spanish>
</TEXT>

Simply use the English phrases for all languages, and the translators will see the changes from the commit logs and translate the ones you haven't.

New Core Method: Unchanged

Sorry, no automatic Babelfish translation code yet.

Step 4: Access the Option from Your Code

Now the fun part.

iHurryOverflow = pHeadSelectedCity.hurryProduction(HURRY_WHIP) - pHeadSelectedCity.productionLeft() [s]+ pHeadSelectedCity.getCurrentProductionDifference(True, False)[/s]
[B]if BugCityScreen.isCountCurrentTurnInWhipAssist():
    iHurryOverflow += pHeadSelectedCity.getCurrentProductionDifference(True, False)[/B]

New Core Method: Unchanged

The above code will work just fine in the new BUG. If you do not provide an accessor function name, an appropriate one (getFoo or isFoo for booleans) is created for you.

As I mentioned, a different access style is created as well. This one doesn't use functions (or so it appears to the user):

if [B]BugOpt.City.WhipAssistCountCurrentTurn[/B]:
    ...

This second method has one programmatic advantage: it can be accessed before it has been defined. Some modules (CvMainInterface specifically) are loaded before BUG is allowed to initialize itself. The workaround is ugly (but works), so I may switch BUG to use the new style.

Please make sure to SVN update to the latest CvMainInterface.py before posting your version. You can post just the new option definition and XML tags, and I'll put them in the correct spot. Thanks!

 

[Dresden]

Thanks!

Since I had to test all the additions anyhow, I made all the changes and attached all the relevant files; didn't bother mirroring the directory structure since I figure you know where everything is. Everything uses revision 1122 as a base. Changes made:

• BugCityScreenOptions.py contains the new options definition and accessor function.
• BUGOptions_CIV4GameText.xml has the label & hover text entries
• BUGGeneralOptionsTab.py has the checkbox addition (that part wasn't mentioned but I figured it out)
• BUG_CIV4GameText.xml has a new entry for the whip assist text if there is gold overflow.
• Civ4lerts_CIV4GameText.xml has a new entry for the pop rush civ4lert text if there is gold overflow.
• CvMainInterface.py adds potential gold overflow and the options processing for the whip assist display.
• Civ4lerts.py adds potential gold overflow and the options processing for the "can whip" event; it uses the same option as the Main Interface which means I imported BugCityScreenOptions too.
• CvCustomizableDomesticAdvisor.py adds the options processing for the whip overflow calculation subroutine; it uses the same option as the Main Interface which means I imported BugCityScreenOptions too. I did not add anything about the gold overflow to this one because I wasn't sure how to handle it.

The options screen:

Civ4lert with gold overflow:

Comparison between option off & option on:

And here's the entry added to BUG Mod.ini:

# Include current production in overflow estimates
Overflow Count Current Production = 1

EDIT: Attachment removed; feature was merged into BUG

 

[EmperorFool]

Awesome, I knew I forgot a step. I'm hoping to move those tabs to XML as well, but that's a bit more work and I fear it won't be any more self-explanatory than it already is[n't].

I dropped the + in the WhipAssist gold overflow bar to minimize overflowing the bar. To compensate, I was going to change the Buy It Now text to "-X <stack of red coins icon>", but I don't see the icon in the font. I see the single red commerce coin, but that would be misleading. For now I added the minus sign, and I'll ask NikNaks to draw a stack of red gold coins.

If you want to tackle adding an overflow gold column to the CDA, here's a rough sketch . . . I got halfway through the instructions and realized it had already taken longer than it would to just add the column. If you want to see how to add other columns, take a look at the diff in SVN.

Edit: Committed

 

[Dresden]

Revisiting this quote for an off-topic question:

[ Don't copy-n-paste code from here as I've used spaces to indent to make it more legible in the post. Spaces BAD! ]

So is the official BUG convention to use tabs for all indenting? Or is consistency within a file all that matters? I ask because some files (e.g. AttitudeUtils.py) use blocks of 4-spaces instead. I use emacs for editing and it pretty well rolls with whatever the file is currently using but I figure I should ask before submitting code.

 

[EmperorFool]

So is the official BUG convention to use tabs for all indenting? Or is consistency within a file all that matters?

Be consistent within a file. Most of the Civ code uses tabs, but I've found a few in the past that mixed in some spaces. You can do it if you're careful, but best to simply avoid it.

Note that I sometimes will use spaces with tabs (oh my) to line up long parameter lists or list initializers. It looks nicer, but I should get out of that habit.

I ask because some files (e.g. AttitudeUtils.py) use blocks of 4-spaces instead.

Ack, this is very bad. For all BUG-created files, tabs should be the norm. I'll fix that file. Have you seen any others?

 

[Dresden]

That's the only one I've noticed but I haven't really been looking for that previously since I haven't had any indentation problems (knock on wood).

 

[EmperorFool]

Thanks. The offender has been severely reprimanded and has fixed his editor.

 

[ruff_hi]

Thanks. The offender has been severely reprimanded and has fixed his editor.

That was me. My python editor uses what ever is in the file (tabs or spaces) but uses spaces for brand new files. AttUtils.py was the first brand new file that I've created for BUG ... all others were just mods of existing files. I've changed my options so that it now uses tabs (and displays them too!).

 

[·Imhotep·]

In a totally unrelated matter , how do you set the Editor in Eclipse to cut the text on the right side of the window so that one doesn't have to scroll all the time ? Using the "Format" command is tiring...

 

[EmperorFool]

@Imhotep - I'm not entirely sure I get your question. Do you want it to word wrap lines that are too long, or automatically hit return for you as you type like old typewriters that ding as you get close to the margin?

I'm running at 800x600 (gasp! I know), so I have set up all the Eclipse non-editor panes (package explorer, outline view, console, etc) to be fast views. They appear as icons around the editor pane (you can move them) and appear when you click the icon. Clicking the editor hides the open fast view.

You can also simply minimize them to hide them. Clicking the icon brings it back and keeps it there.

I suggest playing around with the view possibilities before embarking on word wrap. However, I do not know if Eclipse has that option. One problem is that in whitespace is syntax in Python, and word wrapping could get confusing.

BTW, Google is your friend. Feel free to PM me some cookies.

 

[·Imhotep·]

Hi EF,

I've installed the plug-in - and it works ! Thanks !

 

[EmperorFool]

I've installed the plug-in - and it works ! Thanks !

Yes, me too. Just be aware that it messes up the line numbers -- the lines that wrap are counted again. For text it's no big deal, but if you turn on wrapping and then jump to line (ctrl-L) 125, that's why it doesn't match the exception stack trace you're reading.

It doesn't alter the file though, just the display of line numbers, so there's no danger.