Bug Fix Discussions

[ogj20]

I've been having a look at profiling Atilla, trying to work out why it's running so slowly. One problem appears to be that the CivEconomy object is constantly being asked to provide economic aggregates, e.g. army supplies needed and estimated tax income per turn, and works them out by a longwinded iteration through all the subeconomies, even when they won't have changed since the last time it was asked. These numbers were being requested from a process that runs once for each inhabited square, so the whole thing was was scaling appalingly.

A similar problem was going on when working out manpower requirements for new units, which are obviously identical so long as the unit is still built out of the same elements, but required a creation of a hashmap, and an iteration through it every time the unit archetype manpower was requested, which was munching quite a long time, just to return an essentially constant float.
Atilla about 30%.

I think this problem of always iteratively computing relatively constant aggregates from subclasses is probably present all throughout the code, given the extensive use of multilayered hierarchies, and a common approach to minimising the processing involved needs to be adopted if we're going to be able to run whole world type games without scaling nightmares.

[Mark_Everson]

Thanks Owen. Agree with everything you say. We'll need to think about design approaches to avoid this sort of issues in the future. But of course we don't want to do too much optimization at this point either. I think you've struck a good balance in finding a few things that can give significant increases. And we know know where to look so we can use better designs when refactoring.

[LDiCesare]

Nice job, Owen . I think technology may have the same problems creeping in it, as every tech value is reevaluated every turn. If you see tech stuff appearing in a profiling, we can add a turn number data in all tech parameter objects and compare it to the current turn in order to know whether the computation must be done again. I may try it if I find fights (or fight simulations) start to grow slow.

[boomer70]

Hi,
I just joined the team and Mark asked me to look at F152 as a starting point to getting to know the code.

I have looked at F152 and have a "fix" of sorts. The
code in MapInputMode was just implemented in a wierd
way. I have changed the code to basically work the
way I think the FR wanted. You press an arrow key and
you get a move order for one square in that direction.

There is an issue with this however. The arrow key
logic does not check to see if the path you are
generating is a "valid" one. So if you arrow key a
move into a coast the draw routine throws a null
pointer. The mouse move code seems to disallow moves
that would be invalid even in the black areas. This
is probably not a feature we really want since it
allows you to find all the coasts simply by trying to
move a unit as far as you can in that direction until
the move is rejected. Presto a coast square. I can
do one of two things. I have added the same check to the arrow code. I can look at fixing the bug to delay
the disallowing until the area in question is at least
not black.

One more thing I should add, I had to change the
keybinding to use CTRL+ARROW because the ARROW keys are being eaten in some component before we even see them. I am trying to track down where exactly but for now this works.

I am also going to look at F168 to see if there is anything we can do with that unless anyone has a more pressing need.

-Aaron

[Mark_Everson]

Thanks Aaron, glad to have you on board!

Having a decent way to organize TF all in one step, rather than unit-by-unit will be great. There is already a mechanism to graphically show selected individual units in the TF box code. When a unit is selected its power bar gets thicker. But probably we want a stronger indicator than that. Please let us know about any gui issues that come up so we can kibbitz

Great to see you making progress already!

-Mark

[boomer70]

OK I am putting this in the bug fix thread since it is now a design/work-around type question.

The problem with the port bug is that the port code checks to see if it is valid to have a port where it is defined which needs the terrain data which has not yet been loaded from the scenario file.

We could do a number of things to fix this problem. We could change the file format to have terrain data defined much before it is (I am sure there will be other things depending on it down the road).
We could change the reader to read the whole file and build a DOM model before it creates objects. This would let us have the file be in any order but still process the objects in the order we need them. Probably want to change to a real XML parser if we were going to do this. Not sure why we are using a hand-rolled one anyway.
Or we could defer the check on the port until after the terrain data is read, sort of like a post process.

Anyone have a preference??

-Aaron

[LDiCesare]

One thing I am sure of is: No DOM. DOM is evil. It is big, slow, memory-heavy and mostly useless. The handmade xml parser is much smaller than xerxes, and makes distribution of the game as a downloadable zip possible. If we take a standard parser, it will be too big.

If the problem stems from the fact that terrain is not yet read, we can either put it before in the file (but the various threads will cause problem if the port definition comes in a different file from the current one - I am unsure there is any point in reading each xml file in its own thread).
The other way is to check port validity later in the game. That is, the setPort method just sets the port, and the getPort methods checks that port terrain is water. If it is not, it resets port to null.
What do you think?

[boomer70]

Originally posted by LDiCesare
One thing I am sure of is: No DOM. DOM is evil. It is big, slow, memory-heavy and mostly useless. The handmade xml parser is much smaller than xerxes, and makes distribution of the game as a downloadable zip possible. If we take a standard parser, it will be too big.

Not that I care one way or the other but the 1.4 series of JDKs have an XML parser and DOM model built in so the download thing doesn't have to be an issue. It is likely to be somewhat slower but I would hate to see us hand roll a DOM model implementation. As long as we don't need it I am OK with the parser as it stands.

Originally posted by LDiCesare
The other way is to check port validity later in the game. That is, the setPort method just sets the port, and the getPort methods checks that port terrain is water. If it is not, it resets port to null.
What do you think?

I think delaying checking is probably a good way to go.
The only concern is not getting some feedback when you put a port in the wrong spot but we are planning on having a scenario editor at some point so the editor could disallow the creation of the port then.

[LDiCesare]

Come to think of it, the best error managemetn we would need doesn't require a DOM tree but just a stack. That is, you try to resolve a given item, fail, but will retry when its enclosing tag is closed. If it is still an error, you can retry when the next tag is closed.
That stack of error is a bit like a DOM tree except it doesn't have to hold all data, just the data that needs late resolution or is outright buggy.
It would take some coding, but I don't think too much as we would just keep the objects around.
As long as we can make without it, we should. If we have to go to a DOM-like structure (I really hope not), we will first have to plug the current reflection parsing stuff in it. I know it is easy with sax as the current approach stems from that, but don't with DOM...
Anyhow, delaying the port tag seems the simplest solution.

[Mark_Everson]

Originally posted by boomer70
The only concern is not getting some feedback when you put a port in the wrong spot but we are planning on having a scenario editor at some point so the editor could disallow the creation of the port then.

If we use the dynamic checking approach you could have the getPort() method put out the error when it finds an inappropriate port. The only issue is that the scenario designer won't get that feedback until someone tries to build a ship.

BTW, I don't like the limitation of only one port per province. What happens if you have an existing province with a port and conquer an enemies' port in an adjacent square? I think multiple ports should be allowed, and for now getPort should just return one of them. We should also label ports somehow so the player knows where they are, but that's a bit less urgent.

[LDiCesare]

Just letting know I have coded the postpone stuff. I can't send anything easily right now because my main computer's modem seems out of order and I have lots of work commitments (and also rugby commitments plus time to play NetHack and soon(?) GalCiv ...)around now. I will be able to send my code modifications (hopefully) the first or second week of april (including a plugging of the new ai infrastructure -which is more of a refactoring of the existing code to use plans and threads, so it's not clever right now, but it should be full of bugs we will have to playtest).

[Mark_Everson]

Hi Laurent! Sorry to hear about all the work committments. I've just gone through a similar period, but the near future looks better.

Originally posted by LDiCesare
Just letting know I have coded the postpone stuff.

The mods for the AI etc sound good. What exactly is the postpone stuff? Looking back on the thread I couldn't figure exactly what you meant. I am still without an IDE on my main computer. However I have gotten some updates from Aaron and IIRC others too. I will probably send out a merged version of the code in a week or so, assuming more comes in.

Cya,

Mark

[LDiCesare]

Mark, postponing was postponing the terrain test for ports. The biggest problem I face with the ai is that it may eat lots of memory. I am not sure, as it may have been due to the fact that there were many units at that time in the game, and I need a game that lasts many turns to reproduce the problem. I tried with the old ai, but it didn't happen, but at one time I couldn't open the econ windows maybe because of resources shortage?
Anyway I will be investigating that in my spare time. I must check the number of threads created and duplicate (simulated) units to make sure that they are reclaimed after use. So slow progress, but progress nonetheless.

[Mark_Everson]

Hi All:

I have just sent out the merged code with Laurent, Aaron, and Owen's efforts. If you think you should have gotten it and didn't, please let me know.

[vovan]

Thanks, Mark. I will merge what you sent out with what I have done so far, and send you the changes, some time this week-end hopefully.