C2C SVN Changelog

[Koshling]

Update

-Fixed the multiple occurrences of units in the city build selection popup bug.

@AIAndy/Koshling: I left a question for consideration in the CvDLLButtonPopup.cpp file. Please pm me if you can share any insight on that matter.

SO can breath a sigh of relief now that this annoying bug is resolved

I do not think your fix is really correct. Surely you WANT matching subcombats to show up. The correct fix (if I understand the intent anyway) would have been to check if the it is already present before doing the 'push_back' to add it to the list. Either change the kSortedUnits from a vector to a map (std::map<UnitTypes,szString>) so you can tell (though that would complicate the sorting code) or (easier) just add a bool array called bIsUnitIncluded or something (of size GC.getNumInitInfos()) and set the entry for each unit as it gets added, checking it' not already set before doing so.

 

[Thunderbrd]

I do not think your fix is really correct. Surely you WANT matching subcombats to show up. The correct fix (if I understand the intent anyway) would have been to check if the it is already present before doing the 'push_back' to add it to the list. Either change the kSortedUnits from a vector to a map (std::map<UnitTypes,szString>) so you can tell (though that would complicate the sorting code) or (easier) just add a bool array called bIsUnitIncluded or something (of size GC.getNumInitInfos()) and set the entry for each unit as it gets added, checking it' not already set before doing so.

Let me ask you this though... is that built string:

szBuffer.Format(L"%d %s (%d) - [%d]", GC.getUnitCombatInfo(eUnitCombat).getDescription(), GC.getUnitInfo(eLoopUnit).getDescription(), iTurns, iI);

even used anywhere aside from to be what is saved as the unit is listed? The # of turns to completion will show up the same on multiple combat classes as it will on just one because it draws on data where the multiple combat classes have already been fully factored in.

If I could add a new line and display a breakdown for each and how the build modifiers are affecting the unit in each case individually it'd be worthwhile but without being able to use .append in that file (for whatever reason that is) it seems to me pointless that this string compile is even being used except to categorize the unit as one of those to be included in the list.

The lines

							//gDLL->getInterfaceIFace()->popupAddGenericButton(pPopup, szBuffer, GET_PLAYER(pCity->getOwnerINLINE()).getUnitButton(eLoopUnit), iI, WIDGET_TRAIN, iI, pCity->getID(), true, POPUP_LAYOUT_STRETCH, DLL_FONT_LEFT_JUSTIFY );
							//iNumBuilds++;

were suspiciously commented out at some point before I took a look at this last night and this would apparently indicate that the hover help (when you mouseover the particular selection) had been moved to a later point - muting the necessity of the built string in the first place.

So I look further down to find out where our szBuffer is being utilized later and I find, after what looks to be an employing of the szBuffer we built there to help 'sort' the list (perhaps by combat class then? - which would not really make any multiple combat class inclusion helpful since the unit can only be placed once in the list) the following:

				if (pCity->canTrain(eLoopUnit))
				{
					int iTurns = pCity->getProductionTurnsLeft(eLoopUnit, 0);
					szBuffer.Format(L"%s (%d)", GC.getUnitInfo(eLoopUnit).getDescription(), iTurns);
					gDLL->getInterfaceIFace()->popupAddGenericButton(pPopup, szBuffer, GET_PLAYER(pCity->getOwnerINLINE()).getUnitButton(eLoopUnit), iOriginalIndex, WIDGET_TRAIN, iOriginalIndex, pCity->getID(), true, POPUP_LAYOUT_STRETCH, DLL_FONT_LEFT_JUSTIFY );
					iNumBuilds++;
				}

Which apparently re-formats the szBuffer entirely (overwriting the original) and sending up a hover pop display that differs completely from the original one that apparently was in use in VanillaBtS.

On the hover help you get when you're in the game, I can't see any reference to the unit combat classes in the hover text (except where the normal unit help creates them as a part of the hover) so it had already been removed, presumably, by this process.

So it appears to me that it would be less than helpful to try to include all the unit's combat classes in this as its merely a sorting mechanism.

But I could be reading some of the code between these posted examples incorrectly as I'm not 100% clear on what all the lines there accomplish.

 

[Dancing Hoskuld]

Updates

- removed a duplicate I found in the unit spawn xml
- removed the combat class Whale (put on the SVN before it was ready)

edit update

- Extra spawn rate for animals in Ngorongoro crater. This means that there will be many more animals near this natural wonder. I am not sure I have the balance right yet.

 

[Hydromancerx]

Updates
- Tweaked Inventor's Workshop.
- Fixed Redundancies for Benin Warbow, Urban Crossbowman, Storyteller, Bombard Elephant, Flail Elephant, Carack Merchant, Sloop of War and Dromon.
- Fixed redundancies for Mahout Lancer, Howdah and Elephant Gunner.

 

[strategyonly]

2nd set of redundancies for Heroes fix

 

[Koshling]

Let me ask you this though... is that built string:

szBuffer.Format(L"%d %s (%d) - [%d]", GC.getUnitCombatInfo(eUnitCombat).getDescription(), GC.getUnitInfo(eLoopUnit).getDescription(), iTurns, iI);

even used anywhere aside from to be what is saved as the unit is listed? The # of turns to completion will show up the same on multiple combat classes as it will on just one because it draws on data where the multiple combat classes have already been fully factored in.

If I could add a new line and display a breakdown for each and how the build modifiers are affecting the unit in each case individually it'd be worthwhile but without being able to use .append in that file (for whatever reason that is) it seems to me pointless that this string compile is even being used except to categorize the unit as one of those to be included in the list.

The lines

							//gDLL->getInterfaceIFace()->popupAddGenericButton(pPopup, szBuffer, GET_PLAYER(pCity->getOwnerINLINE()).getUnitButton(eLoopUnit), iI, WIDGET_TRAIN, iI, pCity->getID(), true, POPUP_LAYOUT_STRETCH, DLL_FONT_LEFT_JUSTIFY );
							//iNumBuilds++;

were suspiciously commented out at some point before I took a look at this last night and this would apparently indicate that the hover help (when you mouseover the particular selection) had been moved to a later point - muting the necessity of the built string in the first place.

So I look further down to find out where our szBuffer is being utilized later and I find, after what looks to be an employing of the szBuffer we built there to help 'sort' the list (perhaps by combat class then? - which would not really make any multiple combat class inclusion helpful since the unit can only be placed once in the list) the following:

				if (pCity->canTrain(eLoopUnit))
				{
					int iTurns = pCity->getProductionTurnsLeft(eLoopUnit, 0);
					szBuffer.Format(L"%s (%d)", GC.getUnitInfo(eLoopUnit).getDescription(), iTurns);
					gDLL->getInterfaceIFace()->popupAddGenericButton(pPopup, szBuffer, GET_PLAYER(pCity->getOwnerINLINE()).getUnitButton(eLoopUnit), iOriginalIndex, WIDGET_TRAIN, iOriginalIndex, pCity->getID(), true, POPUP_LAYOUT_STRETCH, DLL_FONT_LEFT_JUSTIFY );
					iNumBuilds++;
				}

Which apparently re-formats the szBuffer entirely (overwriting the original) and sending up a hover pop display that differs completely from the original one that apparently was in use in VanillaBtS.

On the hover help you get when you're in the game, I can't see any reference to the unit combat classes in the hover text (except where the normal unit help creates them as a part of the hover) so it had already been removed, presumably, by this process.

So it appears to me that it would be less than helpful to try to include all the unit's combat classes in this as its merely a sorting mechanism.

But I could be reading some of the code between these posted examples incorrectly as I'm not 100% clear on what all the lines there accomplish.

Looked at this a bit more - basically you broke it way back in rev 4535 as it turns out.

The first string is NOT an end-user formatted string - it is JUST used to establish a sort order and pass information to the next stage of processing (the loop below) which produces the user-visible string. The KEY aspects of the first string are:

1) It sorts into the desired order - so the original version started with the combat type as a number in order to sort by combat type. In rev 4535 you changed the string generation to be self-inconsistent(first insert is %d but you are now passing a string!), which breaks this requirement. Given that the combat mod means units can have MULTPLE combat types I don't know what the correct answer is - you need to step back and ask WHY this routine is trying to sort things by combat type in the first place (from a UI requirements perspective), and work out what the most useful sort is in light of the combat mod giving unit multiple types (and then arrange to generate a string that sort in that order)

2) It as the original unitclass id embedded in the string in '[]'

The second loop parses the unitclass id out of the (now correctly ordered) string and uses that (and only that - the rest of the first string is discarded, having served its purpose) to produce the actual string seen by the user

The 'suspiciously commented out' lines are just what use to be there (LONG ago) before Athoress added the sorting. They are still there, but they are now in the second loop which goes over the same elements but in a different order (as defined by the first loop)

 

[AIAndy]

Update
Added promotions to both sync checksum and the OOS logger.

 

[Thunderbrd]

1) It sorts into the desired order - so the original version started with the combat type as a number in order to sort by combat type. In rev 4535 you changed the string generation to be self-inconsistent(first insert is %d but you are now passing a string!), which breaks this requirement. Given that the combat mod means units can have MULTPLE combat types I don't know what the correct answer is - you need to step back and ask WHY this routine is trying to sort things by combat type in the first place (from a UI requirements perspective), and work out what the most useful sort is in light of the combat mod giving unit multiple types (and then arrange to generate a string that sort in that order)

Ok, well, at least I'm not reading it wrong now since pretty much everything you said about what its doing was what I had determined/guessed when I looked at it on Friday.

Did I change that from %s to %d??? Let me guess... iI was located in the first position?

That would still suit our current needs to have it the original way then. The primary CC was not ripped out in anticipation of this sort of potential thing coming up... a need for the unit to have a primary grouping definition where multiple grouping would not be appropriate was forseen as a possibility I could end up encountering. And here's a perfect example. For the list, how its sorted is not terribly important, but if we want to keep it sorted by CC, it might as well be by its 'primary' CC.

I can move the iI to the front and it should sort itself out nicely from there. Would you agree?

 

[Koshling]

Ok, well, at least I'm not reading it wrong now since pretty much everything you said about what its doing was what I had determined/guessed when I looked at it on Friday.

Did I change that from %s to %d??? Let me guess... iI was located in the first position?

That would still suit our current needs to have it the original way then. The primary CC was not ripped out in anticipation of this sort of potential thing coming up... a need for the unit to have a primary grouping definition where multiple grouping would not be appropriate was forseen as a possibility I could end up encountering. And here's a perfect example. For the list, how its sorted is not terribly important, but if we want to keep it sorted by CC, it might as well be by its 'primary' CC.

I can move the iI to the front and it should sort itself out nicely from there. Would you agree?

No. Putting iI at the front would sort it by (entirely user-meaningless) unit type id. You changed the first param from the combatclass id to the combatclass description in the rev I mentioned. What is the UI purpose of the sort-by-combat class? Without knowing that it's hard to say what it SHOULD be with units now having multiple combat classes.

 

[Thunderbrd]

The purpose is nothing more than a sense of preference I'd say. It keeps your missionaries and executives and such and such bound together in the list rather than allowed to be a scattered mess. Those should all be Primary CCs so it would still be appropriate.

Ah... I see about the description bit... I was probably fixing a number of other errors I had just come to understand how to fix and it got erroneously sucked up into that review. Oops!

 

[Koshling]

The purpose is nothing more than a sense of preference I'd say. It keeps your missionaries and executives and such and such bound together in the list rather than allowed to be a scattered mess. Those should all be Primary CCs so it would still be appropriate.

Ah... I see about the description bit... I was probably fixing a number of other errors I had just come to understand how to fix and it got erroneously sucked up into that review. Oops!

Ok - can I leave you to fix it?

 

[Thunderbrd]

Sure

 

[Thunderbrd]

Updates

• Changed FreePromoTypes tag to enable a condition statement regarding the unit that would receive the promotion.
  
  
• Updated Riding School to utilize this tag only for units with the riding combat class
  
  
• Minor repair to the unit sorting on the city production popup
  
  
• Included AI for the FreePromoTypes tag and repaired minor bugs in the AI structure noticed in FreePromotion_2 and FreePromotion_3 along the way.
  
  
• Repaired minor text bug in Assets/Modules/Hydro/Science/Science_CIV4GameText
  Note: on this, I wasn't sure if the 'dissallowed white space' the error stated was at fault was stemming from an ampersand '&' or from the '[3]' that was present (probably the latter if I'd be willing to bet) but one of the two was no bueno.

 

[Thunderbrd]

Updates

Adding these to the Thunderbrd.FPK at:

• ART/Buttons/Promotions/CommanderArcticPromotion
• ART/Buttons/Promotions/CommanderDesertPromotion
• ART/Buttons/ScreenInterface/DemolishCityButton
• ART/Buttons/ScreenInterface/SellBuildingButton

 

[Thunderbrd]

Updates

• Included CommanderCommand1Promotion through CommanderCommand10Promotion (ex directory for xml call reference: ART/Buttons/Promotions/CommanderCommand1Promotion)
  
  
• Included CommanderGeneralStaff1Promotion through CommanderGeneralStaff10Promotion (ex directory for xml call reference: ART/Buttons/Promotions/CommanderGeneralStaff1Promotion)

 

[Dancing Hoskuld]

Update

- Some more Spanish Translations by Kroji (just 7 files a lot more to come.)

 

[Koshling]

@modders - FYI - I'm moving the calculation of the XP needed for next level out of Python and into the DLL. This is expedient as part of multi-core work (turns out you cannot safely call Python on any but the main thread even if you serialize access to it), but will also perform better anyway. The DLL calculation matches what the Python currently says, but be aware that changing what the Python says in 'getExperienceNeeded' will no longer have any effect.

It will be possible to change the code to re-enable Python if we need to, but it's a bit of a pain and right now I'm just being pragmatic as I cannot see us really wanting to change the calculation anyway.

 

[ls612]

Updates:

-Tweaked the Anti-Tank infantry
-Converted OutcomeMission costs to use the Expression system.

@AIAndy: You might want to check and see if I did the second one entirely correctly, I did a sanity check and it seems to work fine, but it wouldn't hurt to make sure.

 

[AIAndy]

Updates:

-Tweaked the Anti-Tank infantry
-Converted OutcomeMission costs to use the Expression system.

@AIAndy: You might want to check and see if I did the second one entirely correctly, I did a sanity check and it seems to work fine, but it wouldn't hurt to make sure.

Two mistakes:
1) You need to delete the expression in the destructor (otherwise you can leak it under some circumstances).

2) The stream reading code is missing a
pStream->Read(&bPresent);
right before the

	if (bPresent)
	{
		m_iCost = IntExpr::readExpression(pStream);
	}

This one actually breaks the cache reading.

 

[ls612]

Two mistakes:
1) You need to delete the expression in the destructor (otherwise you can leak it under some circumstances).

2) The stream reading code is missing a
pStream->Read(&bPresent);
right before the

	if (bPresent)
	{
		m_iCost = IntExpr::readExpression(pStream);
	}

This one actually breaks the cache reading.

1. I don't see such code for the other expressions in CvOutcomeMissions, is that OK or should I add SAFE_DELETE_ARRAY calls for those too?