C2C SVN Changelog

Just pushed to SVN (3606):
  • Fixed potential crash in archery bombard
  • Fixed inconsistent reporting of remaining turns on a partially completed improvement builds

Note - I think combat is still not right, but it's not as far off as it was before. I'm looking into it...

I think combat is ok after all. I had under estimated how good a strategic position the AI had, and the hill I was being attacked on and couldn't understand why I was losing so much turns out to be desert (with a mine built on it which made it inobvious graphically), so I had a defense penalty as well as per turn damage!
 
Updates

- Art for new resources (Henna, Indigo and Guinea Pig) - resources not yet available
- Mangrove terrain feature - wont appear on maps yet need to edit the C2C post map script python to do that.
 
Just pushed to SVN (3604):
  • Fixed memory corruption when loading or saving with new combat mod

@TB (et al) - I'm also a bit concerned about the memory footprint the combat mod changes imply. Just looking at units, you've added approximately 16K bytes per unit. Large games can easily have 10000 units, so even just for the unit stuff that's an extra 160M or so on the footprint of the game. You need to look at not initializing arrays that are not used (check out most of the CvInfo class arrays which are left unallocated if all members have the default value)
I understand the second part of that (see below). But I'm wondering if you're also implying the sheer volume of tags is a concern. I might like to point out that all of those tags are not necessary to express on any given units - none are required. Thus if the unit needs definition there, it should be included but it doesn't need to include the unused tags like many of our xml entries tend to do. Along the same lines of thinking, it could be useful for space, although not handy for modding so much, if all empty tags that aren't listed as required (most) were removed from entries. This is not just an issue on units either. Only here, where we have SO many units/techs/promos/civics and on and on do we start seeing enough cause to consider that it may be worthwhile to streamline by eliminating unused taglines.

Then again, you might not being pointing at that at all...

It would be better to just store it in a different format, similar to what TB actually did in CvInfo. Storing sparsely populated arrays in that manner is a waste of memory.
Ok... I KNEW this was an issue as I was programming it. But my understanding of the vectors is not agile enough yet to have converted it to application for those. So I programmed it how I understood to do so but did so knowing it would need to be changed. I'd simply request that one of you establish on just one of those arrays the correct method to employ complete with commentary to help me 'get it'. I knew enough to know it'd need to be improved for the sake of processing, and to have an idea of the method it would need to employ, but not enough to actually DO it and expect it to be done right.

What threw me is that in the info files I was able to establish a procedure that worked, but utilizing the same thinking into the Unit file presented some new challenges. Rather than take extra weeks to sort out the new procedure and struggle to understand your efforts to help through written explanation, I figured being able to see it done right would teach me far faster.

Heck... I'm just happy I understood what you guys were getting at with these comments here! lol Helped that I knew I'd hear this kind of feedback on those arrays.

But vectors should probably also be employed on the totals functions (where its combining information from unit promotions, leader influences, base unit information etc to come up with the final definitions on given units.) This is another area I wanted to improve with vectors once I'd learned to employ them in the infos but was not quite sure about how to best go about it, something else I would ask for an example of and then I can go back in and go to town rewriting based on the example.

Thanks in advance if either of you decide to help me by converting one of each for the sake of example.

@GiuseppeIII: It probably won't break the game per se at this point but there still could be lurking instability issues. If you do update, just let us know if you experience anything that doesn't seem right (as usual).
 
Ok... I KNEW this was an issue as I was programming it. But my understanding of the vectors is not agile enough yet to have converted it to application for those. So I programmed it how I understood to do so but did so knowing it would need to be changed. I'd simply request that one of you establish on just one of those arrays the correct method to employ complete with commentary to help me 'get it'. I knew enough to know it'd need to be improved for the sake of processing, and to have an idea of the method it would need to employ, but not enough to actually DO it and expect it to be done right.
Vectors are arrays that resize themselves to the size you need at some point. You could do the same with normal arrays, but then you have to deal with managing the memory yourself.
More important is what you actually store. In your new arrays in CvUnit you store a piece of information for every single promotion in every unit. Most of those entries will always be the default value because the unit will never get the promotion or because it is not an affliction or ... . So you have written a lot of zeros into memory instead of only storing information about the promotions that are in use.

To change that instead of storing a 0 for every promotion, you store pairs of promotion id and value and leave out all the promotions for which the value is currently 0.
Now the choice of container depends on how many entries you expect on average. In other words for how many promotions the value is not 0 on a unit. If that will be very short, then you use a vector of pairs and eliminate all entries that are back down to 0 (that is how the properties are currently stored in CvProperties). If it might get larger, then consider using a map or hash map as those data structures have a faster access to the data by promotion id at the cost of a bit of memory.
 
AIAndy said:
but then you have to deal with managing the memory yourself.
This is the part I wasn't sure on how to handle properly in CvUnit as opposed to the methods used in CvInfos. Where to put the deconstruction? Also, in CvUnit.h there are no declarations of vectors. Should they go here nevertheless? Or is there a reason for that not to take place as opposed to the way its handled in CvInfos?

I'm thinking ALL of my arrays would probably be better off as vectors.

The second method you noted there seems more appropriate for combining, say, subcombats from unitinfo and from promos on the unit and other possible sources, as each unit will need its own definition here that will rarely list off zeros. However, I'm not even sure what exactly you're referring to by 'map or hash map'. (we should probably go back to my noob coding questions thread for more discussion on this ;))

Mind you, again, if I can see an example, I can ask a few easier questions to answer, and follow it from there to repeat the method. After a while, doing this makes the mechanism make sense... After repeated application, I eventually hit an 'aha' moment and seem to fully grasp it at that point.
 
Is it safe to update the SVN now without it destroying my game

I update the SVN daily (at least), as it goes faster that way rather than trying to d/l a whole lot at once (especially with the combination of my flakey internet and sourceforge being a flakey server at times). Since the updates will override any older files there is no reason not to update when possible. Just don't update your WORKING copy unless you are prepared for potential problems.
 
Hi guys,

I keep getting an annoying volcano event every few turns. Even though I was using the world editor to remove the volcanos, they keep recurring at the same spot; one next to my capital and the other on a city tile (!). So I think that the world editor is just a cosmetic thing and the game thinks that the volcanos are still there. I was wondering if I could tweak the xml code for the events to reset the volcano events. I really like the concept but the volcano exploding in a city tile is really annoying.

Currently running revision 3604...

EDIT: And of course, my programming skills being limited to a bit of VBA dabbling, I need a XML for Dummies guide... :D Thanks!
 
Something is wrong with the combat promotions. It is gamebreaking.

Combat I and II work as planned. but with Combat III you have less bonus than with II (don't remember exact amount), same with IV. At Combat V you have a "bonus" of -70% and at Combat VI the "bonus" is -125%. Also, you get 10% less healing in neutral lands instead of a positive amount. I tried to edit the units in world builder, but when I started removing promos the unit got even worse, like having -360% Strength and stuff.

This needs to be fixed as it totally ruins the game!

Otherwise a great many improvements since I last played a month ago. I read the thread about the new Combat System and I am thrilled about it! (Maybe this is causing the problems, by the way? Just a thought...)

Stuff like this belongs in the bugs/crashes thread, thx.
 
Updates

- Improved concept page for Subdued Animals.
- Attempted fix for Lawyer. Still not working or available in game.D'oh, I see what I did wrong:blush:
 
Pushing a fix to the two bugs mentioned earlier today:
1) The Combat Line promotion effects were compounding erroneously: FIXED
(Koshling: please take a look at the notes I left in setHasPromotion and let me know what your intentions were to make those changes or if it was a mistake in a merge or whatnot. You may have been trying to fix another error situation and if so I need to know what that problem is to resolve.)
2) The free promotions were running a filter check they should not have had to run and coming up afoul of a false tech evaluation. (my oops there - I forgot my own rules on free promo setup would already make a filter check when necessary.)
FIXED
 
Hello everybody

I found a few small things(Mostly cosmetic fixes) while browsing the code and created a patch.
 

Attachments

  • 1.7z
    1.7z
    5.1 KB · Views: 41
Pushing a fix to the two bugs mentioned earlier today:
1) The Combat Line promotion effects were compounding erroneously: FIXED
(Koshling: please take a look at the notes I left in setHasPromotion and let me know what your intentions were to make those changes or if it was a mistake in a merge or whatnot. You may have been trying to fix another error situation and if so I need to know what that problem is to resolve.)
2) The free promotions were running a filter check they should not have had to run and coming up afoul of a false tech evaluation. (my oops there - I forgot my own rules on free promo setup would already make a filter check when necessary.)
FIXED

Maybe I misunderstood the code, but it was deliberate since it wasn't doing what the comment (which seemed to make sense) said it should do. The comment says:
Code:
//TB Combat Mods Begin (set up a loop that adds the values of all promotions at the line priority and under without counting those promotions under the promo in question as being possessed
I took this to mean 'the effects of all members of the promotion line up to the one we're now being told to set are applied, but m_pabHasPromotion[eIndex] is only set for the largest one possessed in said line' (that latter bit reflecting the 'without counting...' part of the comment). The code as it was originally (and is now with your latest change) not only processed in the effects of all the earlier promotions in the line, but also set m_pabHasPromotion[eLoopPromotion] for them. Then after that loop it goes through and removes them again in this loop at the end of the method:
Code:
		for (iI = 0; iI < GC.getNumPromotionInfos(); iI++)
		{
			eHasPromotion = ((PromotionTypes)iI);
			if (isHasPromotion(eHasPromotion))
			{
				if (isOverriden(eHasPromotion))
				{
					setHasPromotion(eHasPromotion, false);
				}
			}
		}
That is both very wasteful (adding them in only to immediately take them out again), and was a significant part of the performance problems, and seemed to leave the unit without the effect of the earlier promotions in the line (since it processes them all in in the first loop, then takes them all out again (apart from the last one) in the second loop).

I think you're going to have to add significantly more detailed comments to that routine explaining what its meant to do. Unless I have misread something it definitely doesn't look right as it is now for the above reasons.
 
Back
Top Bottom