Bug Fix Discussions

[vovan]

Originally posted by LDiCesare
The xml reflects exactly what is in the code. Thinking about it first is a good idea as it will also provide you with the interfaces (api) of the (public) classes you will manipulate.

Well, I have made some progress in the code, and it is time to continue our XML discussion.

Right now I am working on integrating the new system of diplomatic statuses into the rest of the game. I have pretty much succeeded, and about the only thing left now is the ability to read the stuff from the XML file. I have taken a look at the XML interface, and the ObjectBuilder class, and I think I kind of understand how the process works, but I am still not sure how I should go about putting the stuff together. Though I have not yet attempted to do it, I would like to first have a clear understanding of how the stuff works.

So, I guess I wil describe the data structure first. There is a class called DiplomaticStatuses. It contains all of the diplomatic statuses for the game. Inside, it has a setStatus() method, that takes in two civilizations, and a StatusStrength object. Now, status strength itself is an abstract class. But the acceptable values for the value to pass in are the children of the class, static classes: War, Peace, and Alliance.

Sooo, I suppose the XML should then look like this (on an example of delenda scenario):

<diplomaticstatuses>
  <status>
    <civilization>Rome</civilization>
    <civilization>Carthage</civilization>
    <diplomaticstrength>
      <war />
    </diplomaticstrength>
  </status>
</diplomaticstatuses>

??

Given that I include the xml interface and the appropriate static function in classes DiplomaticStatuses, and all of the subclasses of DiplomaticStrength. Is that how it is supposed to work, or am I missing something?

[LDiCesare]

Originally posted by vovansim

So, I guess I wil describe the data structure first. There is a class called DiplomaticStatuses. It contains all of the diplomatic statuses for the game. Inside, it has a setStatus() method, that takes in two civilizations, and a StatusStrength object. Now, status strength itself is an abstract class. But the acceptable values for the value to pass in are the children of the class, static classes: War, Peace, and Alliance.

Sooo, I suppose the XML should then look like this (on an example of delenda scenario):

<diplomaticstatuses>
  <status>
    <civilization>Rome</civilization>
    <civilization>Carthage</civilization>
    <diplomaticstrength>
      <war />
    </diplomaticstrength>
  </status>
</diplomaticstatuses>

??

Given that I include the xml interface and the appropriate static function in classes DiplomaticStatuses, and all of the subclasses of DiplomaticStrength. Is that how it is supposed to work, or am I missing something?

You need the xml interface only in the topmost objet, diplomaticstatus, and you have to declare it in the list of classes that the parser recognizes (in FileInput.java or ScenarioFiles.java or both, I don't know exactly which one is used - should check it when I have some time).
So you don't need it in DiplomaticStatus subclasses (you can look at ElementArchetype/ElementCost in the military moel for an example: ElementCost is an object inside ElementArchetype but has no getXML() method, everything is done by reflexion).

To get your example working, you need the getXML method declared for diplomaticstatus class, and a public setStatus method, which takes a Status object as an input, and nothing else (again, you can look at ElementArchetype.setCost method).
The Status object needs a setCivilization (or addCivilization) method (both set and add are OK, add would be better here since you add several). Note hte method names needed can be in any case as the parser doesn't worry about setCost or setcost or setcosT (though setCost is better IMO) for instance.
Instead of a
<diplomaticstrength>
  <war />
</diplomaticstrength>
I'd suggest only a <war /> tag, with the corresponding setWar() method (as an example, you have the militia tag in ElementArchetype). That forces you to have several setWar, setPeace... methods. You can bypass that by using
<diplomaticstrength>war</diplomaticstrength>
and having a single setDiplomaticStrength(String) method instead.

That's about how it works: getXML() for root tags, then reflexion only. In order for reflexion to work, you need public no-arg constructors in the classes that will be created by the parser, and either setXXX public methods or (but I don't like that) public data members XXX.

[vovan]

Thanks for the response, Laurent, but now I think you will be able to see why I asked about the XML before coding the diplomacy. You, see, the way you described the process, it is not applicable to diplomatic statuses. There is no, and cannot possibly be an addCivilization method in the DiplomaticStatus class. Also, I think quite obviously, the setStatus method cannot possibly only take in one thing as a parameter, because it needs to know which civilizations the status is between.

Therefore, to make the diplomatic status work properly with your system of XML, I will need to rework the whole system. Oh, well, scratch two-three weeks of work, and start from null.

EDIT: Actually, I think I know the way to circumvent the limitations of the XML parser we have... So, I might not have to rework everything.

Only question is, does the name of the class have to be the same as the tag? So, if for instance I name some class XMLDiplomaticStatus, can I just include an XML interface with getTag() method returning "status" and then use <status> tag within the <diplomaticstatuses> tag? The only reason for that would be to not have a <xmldiplomaticstatus> tag in there, which would probably be confusing for the designers.

[Mark_Everson]

Originally posted by vovansim
Thanks for the response, Laurent, but now I think you will be able to see why I asked about the XML before coding the diplomacy. You, see, the way you described the process, it is not applicable to diplomatic statuses. There is no, and cannot possibly be an addCivilization method in the DiplomaticStatus class. Also, I think quite obviously, the setStatus method cannot possibly only take in one thing as a parameter, because it needs to know which civilizations the status is between.

Therefore, to make the diplomatic status work properly with your system of XML, I will need to rework the whole system. Oh, well, scratch two-three weeks of work, and start from null.

Hold on there Vovan! I don't see anything that you have described that is incompatible with the current XML setup! Please don't give up on all that work yet

In similar approach to the rest of the code I would instead have a DiplomaticStatus class that has a static collection of DiplomaticStatus objects. Each individual status object is entered as you said above, and is then stored to the static collection.

The setCivilization method as you have it will be a bit wierd in that it would be called twice with different arguments. To make it clear you could just have two setters, setCivilization1 and 2. When the store() method writes the DiplomaticStatus object to the collection it can then use the previously set civ1 and civ2 values. (see the xml interface Laurent referred to). Of course if someone puts in DiplomaticStatus Civ1 Romans, Civ2 Huns, and then later another object with Civ1 Huns and Civ2 Romans, it would overwrite the first since your matrix is symmetric.

Anyway, I think this can be made to work with your existing code with only Minor mods. Of course the final decision whether to tweak or scrap is yours, but please allow Laurent and I to make some suggestions before you scrap it all. . .

[vovan]

Well, once again, I feel like an idiot, because I jumped to conclusions without thinking about the problem enough.

Any way, like I said in the edit of the previous post, I know the way to make this work without reworking my existing code. Any of it, in fact - the only thing that had to be done was to add another class specifically for XML... Sorry about the fuss. I guess feeling stupid makes me think faster.

As of now, it seems that the system of diplomatic statuses works well. Now, to implement the functionality for changing it when civs encounter each other, and when the user wants to.

[Framnk]

Exchanging Code..

All-

How do we keep code in synch on the project anyway? For instance, I have something to add to help the app come up maximized, but where do I put it?

TIA

Framnk

[vovan]

Well... That's a good question! Unfortunately, we do not as of now have any version control system (although Mark said if we demand it, he would look into it. ) So, what we do, is just basically e-mail everyone the changes we make to the code, and say in the e-mail which files we changed so that it is easier to keep track of the stuff. Also, if you are going to change something that you think other people might be working on, just e-mail everybody in advance about the changes you are about to make. If somebody is indeed working on the same class, they will contact you and coordinate the efforts.

So far, code collisions have not been a problem, since there were very few of us working at any given time. But with the number of coders increasing, it might be a good idea to actually set up some kind of version control.

[Framnk]

Source Control...

Yes, a source control system would definitely be nice! I know they use CVS for a lot of internet open projects. I use SourceSafe quite a bit at work and I find it's interface to be nice (although with Java you don't get the nice dev. environment integration).

I'm assuming you guys already have a server running somewhere (that is hosting the Clash web site). Barring that, I always have a server running from my home so I'd be happy to set up a CVS or SourceSafe server on it if you guys would like.

Of course, it's totally up to the group. If you guys are happy the way things are going, that's totally fine with me as well. Just thought I'd throw it out there.

Framnk

[vovan]

Well, that's exactly the problem, you see. I don't think we have a server. AFAIK, apolyton hosts the Clash site, and I am not sure Mark (the Admin, not Everson ) would want to load it further with a source control system.

As for the choice of the system, I would personally prefer SourceSafe, as have also had much experience with it, as opposed to CVS, which I have never used.

[Mark_Everson]

Re: Source Control...

Originally posted by Framnk
I'm assuming you guys already have a server running somewhere (that is hosting the Clash web site). Barring that, I always have a server running from my home so I'd be happy to set up a CVS or SourceSafe server on it if you guys would like.

Hi Guys. Like Vovan says, we have no site for a server at the moment. If we keep the number of coders we have, and there is activity from everyone, going to a version control system would indeed be valuable. Frank, your offer is good, and I apreciate it! But for it to be practical it has to be clear that you're in the project for the long term.

I guess my proposal at this point would be to keep up as we are for the next few months. This will allow us to monitor how much code actually gets slung around, and also let Frank figure out if he's likely to be dedicated in the long term. Once we have answers to those we can make a more informed decision. If most others want version control, I'm certainly willing to use it.

[Mark_Everson]

Per a comment of Richard's in the D7 Comments thread.

Programmers, is there anyone who feels competent to figure out why Clash grinds to a halt on Richard's systems? Richard, can you describe the systems?

I am not experienced enough to handle this. Can someone take it on in the fairly near future? We need Richard functional so he can fine-tune tech. Maybe start a dedicated thread on it, since significant discussion may be needed to figure out the problem.

[LDiCesare]

I really don't have time to check it, but here is a suggestion: Why not trying to shut off all the file output and see if that somehow affects the game? I don't know how streams are implemented in java, but if the game has to open and close a file of 100+kbytes every turn, it might cause problems?

[Mark_Everson]

Hi Laurent:

Based on a previous comment from Gary to this suggestion, IIRC he said the files were opened once and then just stayed open. But still its something to check just in case.

[ogj20]

Hi, I'd like to join in with the coding effort, though my Java knowledge is a little limited. I'd like to start by looking at one or two of the bugs to get to know the code, I thought maybe 164 or 163? If those have been done by now then I'd be happy to look at any of the others that still need fixing.

Who do I need to speak to to get the source code to work on?

[Mark_Everson]

Hi ogj20, glad to have you onboard. Just email me and I'll send the code and some new Clash Team member info. I am setting up a wireless network at home today, and so may not be able to respond soon if it takes you a while to email me.

You could take a shot at 164. I think 163 is something best left to Laurent and me. Richard just made some additional comments on 164 in the D7 Download thread. It appears that one can also colonize water.

I think the best short-term fix for 164 is to:

1. ensure colonization cannot take place for a square with farm sites = 0, that will get rid of problems colonizing desert and water. I think its Square.getSites that you want.

2. change the menu so you can't remove the last bit of population. You'd have to leave at least one population in a square when you pick up. A better fix will come after I refactor the econ code.

3. an associated fix is that the population can grow very large with lots of food. That should be capped at a 20% increase or so. Try looking at PopulationData.