exe replacement

a place to discuss it

Moderators: Deathifier, Sukayo

tichtich
Assault Legionnaire
Posts: 56
Joined: Thu Jan 26, 2006 2:01 am
Location: Bristol, UK

Postby tichtich » Mon Feb 17, 2014 9:05 pm

I just noticed that, in UnitTypes, the 13 resource costs for building a unit are stored as separate fields. It would be more convenient to store them as an array of 13 int's. I'll provide a method for consuming resources that takes a reference to such an array as its argument, so you only need to pass me the one argument.

flowerpot
Assault Legionnaire
Posts: 74
Joined: Mon Dec 17, 2012 8:54 am
Location: Nowhere, trying to conjure up a Symbiot AI
Contact:

Postby flowerpot » Tue Feb 18, 2014 10:45 am

I made a commit to master which you can fetch. This commit has the disappearing own cargo pods fixed. Also:

tichtich wrote:For example it tries to consume resources on planets before consuming any in space, which means you don't often see resources disappearing from pods in space. If I consumed resources in unit list order, people might quite often see resources disappearing from a transport as it's passing over a planet! I'm trying to reproduce EFS's priorities, at least where I know them and they seem significant.


I've added a function in Util, getCargoPods, which returns all the cargo pods of the current faction, sorted so that pods are in planet order and for each planet sorted so that pods appear in resource type order, and for each resource type sorted so that pods appear with planet side pods first and space pods last. To make it smarter it should probably be modified so that it sorts in planet order only when universal warehouse is off. To test it I added a button with two red triangles to the lower left corner on planet and space windows. Pressing it will iterate through the cargo pod list as it was in the beginning of turn. Maybe this is useful, perhaps as a blueprint to improve your own algorithms, during eg. secondary production where you will have to search for pods for each producing city in sequence or for feeding units and cities. I was also thinking of making a Class Cargo which would provide methods for returning cargo sorted in ArrayLists of planets and resource types.

So I'm sometimes searching through the hexes of a planet, instead of through the unit list. At present that means I'm specifically checking for the one less hex in even-numbered columns. But perhaps at some point you would like to provide me an iterator for doing that.


I made an iterator for hexes of a planet, Util.HexIter with a constructor method Util.getHexIter, it will go through the hexes using planetary Hex arrays and automatically takes into account the missing hexes.

Also I noticed that NetBeans made some formatting changes to your harvesting modifications. You can format your code in NetBeans by pressing shift-alt-f. Shame it doesn't autoformat code on save, perhaps there is a switch somewhere for such behaviour. And shame on me for not noticing this when committing your changes.

I just noticed that, in UnitTypes, the 13 resource costs for building a unit are stored as separate fields. It would be more convenient to store them as an array of 13 int's. I'll provide a method for consuming resources that takes a reference to such an array as its argument, so you only need to pass me the one argument.


Now this is inconvenient, required tech is an array so why haven't I made the resources an array. Maybe I should change this into an array.

tichtich
Assault Legionnaire
Posts: 56
Joined: Thu Jan 26, 2006 2:01 am
Location: Bristol, UK

Postby tichtich » Tue Feb 18, 2014 8:21 pm

flowerpot wrote:I made a commit to master which you can fetch.

Thanks. I've fetched it. I haven't merged yet, but I checked it out temporarily, and ran it. Nice new red button. ;)

flowerpot wrote:I've added a function in Util, getCargoPods, which returns all the cargo pods of the current faction, sorted so that pods are in planet order and for each planet sorted so that pods appear in resource type order, and for each resource type sorted so that pods appear with planet side pods first and space pods last.

You could have a separate list for each resource type, as they don't have to be consumed in any particular order. That would save some sorting time (and a little access time). It would also be more convenient to access. Note that I don't think Java will let you have an array of lists, but you can certainly have a list of lists, and probably an arraylist of lists.

flowerpot wrote:To make it smarter it should probably be modified so that it sorts in planet order only when universal warehouse is off.

To reflect what EFS does it should sort by planet in both cases. With UW on, resources are consumed from planet 0 first, and so on. As far as I can see, pseudo-random consumption would make more sense than concentrating consumption on certain planets, but I don't think it much matters. Incidentally, people on planet 0 also get priority when it comes to eating, so famine is concentrated on a few planets, and not evenly spread. Choose a low-numbered planet when you're deciding where to live!

flowerpot wrote:Maybe this is useful, perhaps as a blueprint to improve your own algorithms, during eg. secondary production where you will have to search for pods for each producing city in sequence or for feeding units and cities. I was also thinking of making a Class Cargo which would provide methods for returning cargo sorted in ArrayLists of planets and resource types.

What you're suggesting is what I earlier called a secondary data structure. The trouble with that is that the secondary must be updated as the primary data changes. I see two alternatives:
1. We keep the secondary up to date continually. That means updating it every time someone creates, destroys or moves (to another planet) any cargo pods during their turn. I think that would be my preference. But it means being disciplined and ensuring that all such acts are funnelled through a small set of methods which update the secondary data.
2. We regenerate the secondary at the start of each faction's turn, and only use it for the start-of-turn stuff. I suspect that's what you had in mind. But even that requires a certain amount of updating. Deleting cargo pods from the secondary as they're consumed is no problem. But we would also have to add production to the list immediately, so it can be consumed by other cities, unless we decide not to allow that. In any case, we would need to add production or do a new search before proceeding to any other consumption, like eating. For consuming resources during a turn, e.g. when building, we would have to read and sort the list after each action, which might be quicker than searching the surface of 40 planets (UW on) but slower than searching the surface of just one planet (UW off). Of course we could program more than one method, and use the best one for the circumstances.

I've already spent quite a lot of time on this particular question, so I won't recode until we've made a definite decision. In the meantime I'm doing some other stuff. I've implemented eating, and my next job is to split off most of my work from Game into one or two new classes. I'll probably merge after doing that.

flowerpot wrote:I made an iterator for hexes of a planet, Util.HexIter with a constructor method Util.getHexIter, it will go through the hexes using planetary Hex arrays and automatically takes into account the missing hexes.

Thank you.

flowerpot wrote:Also I noticed that NetBeans made some formatting changes to your harvesting modifications. You can format your code in NetBeans by pressing shift-alt-f. Shame it doesn't autoformat code on save, perhaps there is a switch somewhere for such behaviour. And shame on me for not noticing this when committing your changes.

I'm not sure I want it to change my formatting, but I won't worry about it.

tichtich
Assault Legionnaire
Posts: 56
Joined: Thu Jan 26, 2006 2:01 am
Location: Bristol, UK

Postby tichtich » Wed Feb 19, 2014 2:03 pm

P.S. Sorry to be negative, but I doubt that sorting into an ordered sequence is the best strategy. My original plan--if we decided to use a secondary data structure--would have just had a list of pods for each resource, for each planet (and for each faction if the structure persisted over the longer term). This would be a list of lists of lists (of lists). Then the order within each pod list doesn't matter. You just consume the first pod on the appropriate pod list. And you can add new pods to the end of the list.

Another complication you may not have considered: we can't just take resources from a pod as soon as we find a suitable one. We have to wait and see whether there are enough resources to meet the full requirement, which might include several types. If there's not enough in total, we mustn't consume any. So either I give the caller two methods, one to check there are enough resources and another to consume them (without checking), leaving it up to him to check before consuming. Or else I combine the two into one method.

At present I do the latter, since doing both check and consumption together allows me to be a bit more efficient. During the check I compile a list of the relevant pods I find, so that I don't have to search again if I proceed to consume. This also helps avoid the problem of removing a pod from the list I'm iterating over. (I made that mistake at first, and got some nasty exceptions.)

If we have a persistent secondary data structure that makes it quick to find suitable pods, then there's little or no advantage in the latter system.

flowerpot
Assault Legionnaire
Posts: 74
Joined: Mon Dec 17, 2012 8:54 am
Location: Nowhere, trying to conjure up a Symbiot AI
Contact:

Postby flowerpot » Wed Feb 19, 2014 9:58 pm

tichtich wrote:P.S. Sorry to be negative, but I doubt that sorting into an ordered sequence is the best strategy.

Don't worry about mentioning weak points, I'd rather learn about my mistakes before I do them.

Note that I don't think Java will let you have an array of lists, but you can certainly have a list of lists, and probably an arraylist of lists.
You can actually have an array of lists. You just have to beat java into submission. From Planet.java:

Code: Select all

public List<Unit>[] space_stacks;
space_stacks = (LinkedList<Unit>[]) new LinkedList[14];

But, I regret doing it , I should have used an arraylist instead of an array since that is in java compilers opinion unchecked or unsafe and I was afraid it could potentially be incompatible with serialization (eg. doing loading and saving in one statement.)

My original plan--if we decided to use a secondary data structure--would have just had a list of pods for each resource, for each planet (and for each faction if the structure persisted over the longer term). This would be a list of lists of lists (of lists). Then the order within each pod list doesn't matter. You just consume the first pod on the appropriate pod list. And you can add new pods to the end of the list.
An arraylist of arraylists (of arraylists) of lists would be fastest I think. And you could make that list into a full LinkedList and add pods to the front of the list but pods in space to the end of the list. I actually have an arraylist of arraylists of int[] int Structure.java to record which units are buildable in which cities. This will move to Faction.java when technologies are implemented and each faction will have its own lists of buildable units.

Another complication you may not have considered: we can't just take resources from a pod as soon as we find a suitable one.

I considered this but I must admit that I haven't taken a very detailed look into resource pod consumption and maintenance. I intend to leave that up you :)

So either I give the caller two methods, one to check there are enough resources and another to consume them (without checking), leaving it up to him to check before consuming. Or else I combine the two into one method.

At present I do the latter, since doing both check and consumption together allows me to be a bit more efficient.

A method to check the availability of resources without consuming them is needed to be able to show in red those units which cannot be built because there are not enough resources.

tichtich
Assault Legionnaire
Posts: 56
Joined: Thu Jan 26, 2006 2:01 am
Location: Bristol, UK

Postby tichtich » Thu Feb 20, 2014 11:08 am

Hi. Since last posting, I've merged with your master, split most of my work off from Game, and uploaded a revision to my Github page. It's at https://github.com/tichtich/Phoenix-RSW2.git

Here are some notes on what's in this revision:
- Resource counting and consumption are working, but I haven't yet made any changes in the light of our recent discussion.
- Cities and units consume food, lose health from starving, and regain health (as described in The Truth About EFS).
- Class Resources contains the resource counting and consumption methods intended for general use.
- Class Economy does the start-of-turn economic update.
- Class Faction holds faction-specific data, such as money. There may be some overlap between this and Economy.
- Class EfsIni holds data from EFS.INI. At present it's just initialised to the standard values, not read in from the file. For now it also holds game options that are normally entered through the GUI, such as Universal Warehouse.
- I've left createUnitInHex and deleteUnit in Game. I hope you'll be able to use them. I've also added some tiny utilities, like getUnitHex, which returns the hex a given unit is in.
- I've moved the harvest data table out of Game, and made it private in Economy, as no other class should need it.
- I've put new initialisation in Game.init, as we discussed.
- I've created getters in Game for structures, planets and random.
- I've made a few other minor edits.

Altogether I've made quite a lot of changes to Game. If you have difficulty merging it with any new changes you've made, and you want me to do the merge, just let me know.

Edit: I see you want to put research in Faction too. I hadn't thought of that. It's probably OK. But let me just explain the main reason I made a separate Economy class. (And maybe that wasn't a good name for it.) I wanted to localise the harvest and production DAT tables in the one class that uses them, rather than having them in Game. But I couldn't put them in Faction, because they can't be static (as they need to be saved with the game), and, if non static, each faction would have its own copy of them! The same would probably be true of the research DAT table. But maybe it would have been easier to just put all the DAT tables in Game, even when they're only needed by one class. What do you think?

flowerpot
Assault Legionnaire
Posts: 74
Joined: Mon Dec 17, 2012 8:54 am
Location: Nowhere, trying to conjure up a Symbiot AI
Contact:

Postby flowerpot » Thu Feb 20, 2014 6:49 pm

tichtich wrote:Hi. Since last posting, I've merged with your master, split most of my work off from Game, and uploaded a revision to my Github page. It's at https://github.com/tichtich/Phoenix-RSW2.git


I took a quick look at your revision. Couple of issues:

1. Game.deleteUnit()

use carrier.disembark(unit) instead of unit.carrier.cargo_list.remove(unit);

What are the correct semantics for deleting a unit with cargo. Is the cargo to be destroyed also or should it be disembarked and added to the stack ? In Hyperion eg. naval cruiser is a carrier and could be selected as input unit whit cargo on board.

2. EFS.INI

I'm hesitant at including any material from EFS which is not absolutely necessary (eg. list of faction id:s, list of resource types, data file names etc) since I want to be well protected against copyright claims so EFS.INI should be read in from the file. Edit. I'm in the process of writing code to read in EFS.INI as a java Properties object.

3. Netbeans seems to have problem with data fetched from https://github.com/tichtich/Phoenix-RSW2.git. After I switch to the master branch from your repository I can no longer switch back to other branches or even checkout a revision from the current branch. Netbeans claims I have modified files in working copy (which I don't have) which would result in a merge conflict. I select revert but it fails (well of course the is nothing to revert.) This is odd because git bash has no trouble switching and checking out.

tichtich
Assault Legionnaire
Posts: 56
Joined: Thu Jan 26, 2006 2:01 am
Location: Bristol, UK

Postby tichtich » Thu Feb 20, 2014 9:23 pm

flowerpot wrote:use carrier.disembark(unit) instead of unit.carrier.cargo_list.remove(unit);

OK. I'd seen disembark. I don't know why I didn't use it.

flowerpot wrote:What are the correct semantics for deleting a unit with cargo. Is the cargo to be destroyed also or should it be disembarked and added to the stack ? In Hyperion eg. naval cruiser is a carrier and could be selected as input unit whit cargo on board.

Good question. Most units die when their transport is destroyed, but fighters on a space carrier survive. A simple and reasonable rule would be that units survive if they are capable of living in their current environment, i.e. land units die in space or at sea, but survive on land. But I can't remember if they survive on land in EFS. For that matter, do land passengers on a landed transport participate in land combat? I guess not, but I can't remember. In the unlikely even that a transport dies of non-combat causes, like starvation or plague, it makes sense that the passengers survive if they can. As far as deliberate termination of the transport is concerned, EFS doesn't let you disband a transport with passengers. (I just tried it.) I would follow the same rule for input units, in case the player does it accidentally. He can always unload the passengers and try again.

If the question is whether such decisions should be implemented by deleteUnit or by the caller, I would leave it up to the caller, who knows more about the circumstances, unless there's a simple rule for deleteUnit to follow. But I'm happy to include a boolean input parameter to specify whether passengers are to be deleted too, and then have deleteUnit do the work. Making it a parameter will also remind the calling programmer that he has to make that decision.

flowerpot wrote:I'm hesitant at including any material from EFS which is not absolutely necessary (eg. list of faction id:s, list of resource types, data file names etc) since I want to be well protected against copyright claims so EFS.INI should be read in from the file. Edit. I'm in the process of writing code to read in EFS.INI as a java Properties object.

OK. I'll leave that up to you.

flowerpot wrote:3. Netbeans seems to have problem with data fetched from https://github.com/tichtich/Phoenix-RSW2.git. After I switch to the master branch from your repository I can no longer switch back to other branches or even checkout a revision from the current branch. Netbeans claims I have modified files in working copy (which I don't have) which would result in a merge conflict. I select revert but it fails (well of course the is nothing to revert.) This is odd because git bash has no trouble switching and checking out.

I don't know if it's relevant, but I first tried to push the revision to the address I'd used before, and it was refused on the grounds that it was not fast-forwardable. I assume that was because the old version didn't know about your version that I'd merged with. After being confused for a while, I decided I should upload to a new address. Perhaps it would be simpler if you let me push to your address.

tichtich
Assault Legionnaire
Posts: 56
Joined: Thu Jan 26, 2006 2:01 am
Location: Bristol, UK

Postby tichtich » Thu Feb 20, 2014 9:29 pm

Thinking some more about the subject of resource consumption, I'm strongly leaning towards maintaining a permanent secondary structure for cargo pods. Having such a structure not only saves execution time, it also makes programming easier, because I can program in the way that's simplest and most natural, without worrying about execution efficiency. Of course that has to be weighed against the extra programming effort to handle the structure. But I think it's worth it. I was also concerned that we might overlook places in the existing program that create, move or delete units, all of which require updating the structure. But the IDE makes it quite easy to find such places, and I believe there are very few, namely:
- setUnitCoords (two versions)
- dropDead*, removeDead and resolveGroundBattleFinalize

If you agree, I'll proceed to implement the secondary structure, and modify those methods accordingly. I would add a relocateUnit method to my current createUnitInHex and deleteUnit, and then we should say that all creation, deletion and removal of units after the start of the game must go through these three methods, which will be responsible for keeping the secondary structure up to date. With that in place, it would be easy to add further secondary unit structures if we want them. For example, we might want lists of all units (not just cargo pods) by faction and planet.

At the start of the game, the structure would be generated after other game intialisation, so methods like placeUnits which create and initialise units at the start of the game wouldn't be affected by it. That's why I only have creatUnitInHex and not a more general createUnit, as I don't think there's any way to create a unit in space during the game. (I have some thoughts about unifying start-of-game initialisation with how things are done during the game, but thta can wait.)

* dropDead doesn't remove units from the main unit list, only from stacks, but I'd like to encapsulate all the clean-up associated with deleting a unit into my deleteUnit.

P.S. I'd make this change in a new Git branch, and treat it as experimental at first.

flowerpot
Assault Legionnaire
Posts: 74
Joined: Mon Dec 17, 2012 8:54 am
Location: Nowhere, trying to conjure up a Symbiot AI
Contact:

Postby flowerpot » Fri Feb 21, 2014 7:22 pm

tichtich wrote:
flowerpot wrote:3. Netbeans seems to have problem with data fetched from https://github.com/tichtich/Phoenix-RSW2.git. After I switch to the master branch from your repository I can no longer switch back to other branches or even checkout a revision from the current branch. Netbeans claims I have modified files in working copy (which I don't have) which would result in a merge conflict. I select revert but it fails (well of course the is nothing to revert.) This is odd because git bash has no trouble switching and checking out.

I don't know if it's relevant, but I first tried to push the revision to the address I'd used before, and it was refused on the grounds that it was not fast-forwardable. I assume that was because the old version didn't know about your version that I'd merged with. After being confused for a while, I decided I should upload to a new address. Perhaps it would be simpler if you let me push to your address.


Well, if you had pushed to joulupunikki/Phoenix the repository would be in a state where my netbeans could not operate on it. And I would probably have had to re-initialize the repository again. Note that I don't mean this is your fault, I suspect netbeans git integration may be at fault. Also I should have been clearer about this earlier but I think that you are better of pushing/fetching/pulling only between repositories that are forks/clones of each other. It looks like I'm gonna have to do another file copy merge. And ask you to fork Phoenix on github afterwards and clone from that fork and use the clone for work and the fork for pushing. And hope that this works cause if it doesn't then I don't know what to do with netbeans git integration. :( Have you done a lot of work since you merged your new modifications ?

tichtich
Assault Legionnaire
Posts: 56
Joined: Thu Jan 26, 2006 2:01 am
Location: Bristol, UK

Postby tichtich » Sat Feb 22, 2014 2:56 pm

Hi. I didn't receive email notifications from the forum about your last two comments. So don't be surprised if I'm slow replying in the future. I've checked my spam folders (local and mailbox) but didn't find them there, though I did find an email from Github!

flowerpot wrote:Well, if you had pushed to joulupunikki/Phoenix the repository would be in a state where my netbeans could not operate on it. And I would probably have had to re-initialize the repository again. Note that I don't mean this is your fault, I suspect netbeans git integration may be at fault.

I wouldn't be surprised if I did something wrong. Perhaps anyway I should start using GitBash from now on.

flowerpot wrote:Also I should have been clearer about this earlier but I think that you are better of pushing/fetching/pulling only between repositories that are forks/clones of each other. It looks like I'm gonna have to do another file copy merge. And ask you to fork Phoenix on github afterwards and clone from that fork and use the clone for work and the fork for pushing.

OK, but what's the advantage of me pushing to a fork over pushing to a newly created empty repo each time? Is it just to avoid creating multiple repos, and having to delete unwanted ones? Perhaps creating a new repo each time would be safer for now, until we know we've got our problems sorted out, even if it is a bit more work.

flowerpot wrote:Have you done a lot of work since you merged your new modifications ?

Some, but I don't understand why that matters. Surely you don't need to merge with my latest working version. If you're afraid my repo may be corrupted, I can provide the src folder as it was after that merge. I could presumably get it from the merged repo, but I also have a backup I made at the time. I'm still making separate backups as I don't trust myself not to make a mistake with Git and lose some work!

BTW I was just about to post the following update when I saw your comment...

I've coded readers for PROD.DAT and RES.DAT, but haven't pushed my latest version yet. Having read RES.DAT, I now use the resource names from that file when displaying cargo pods. That meant that Util.drawUnitDetails needed access to the res table, for which it needed access to game, which it didn't have. So I added game to its parameters, and modified the calls from these methods to pass it: Util.drawStackDisplay, UnitInfoWindow.drawUnits, and UnitInfoWindow.drawDraggedUnit, and CombatWindow.renderCombatWindow.

tichtich
Assault Legionnaire
Posts: 56
Joined: Thu Jan 26, 2006 2:01 am
Location: Bristol, UK

Postby tichtich » Sat Feb 22, 2014 7:09 pm

flowerpot wrote:Have you done a lot of work since you merged your new modifications ?

I realised after my last comment that you were probably asking whether I'd done a lot of work between merging and pushing. Strangely, I thought I remembered that I'd done my splitting off of my work from Game before merging, and had done little or nothing between merging and pushing. But now I realise that was quite wrong, and that I made that split between merging and pushing, and made some other changes besides. Fortunately most of that was creating new files and deleting a large chunk from game. Those shouldn't be too difficult to recreate manually. But I did make a fair number of other changes to Game. If you like I'll give you a list.

flowerpot
Assault Legionnaire
Posts: 74
Joined: Mon Dec 17, 2012 8:54 am
Location: Nowhere, trying to conjure up a Symbiot AI
Contact:

Postby flowerpot » Sat Feb 22, 2014 9:35 pm

tichtich wrote:
flowerpot wrote:Well, if you had pushed to joulupunikki/Phoenix the repository would be in a state where my netbeans could not operate on it. And I would probably have had to re-initialize the repository again. Note that I don't mean this is your fault, I suspect netbeans git integration may be at fault.

I wouldn't be surprised if I did something wrong. Perhaps anyway I should start using GitBash from now on.


I'm sure no git or netbeans commands should leave the repository in a state that makes netbeans go bonkers. You would have to manhandle the repository with eg. a hex editor. Well, there is the possibility that every now and then a critical bit flips producing bizarre results. :?


flowerpot wrote:Also I should have been clearer about this earlier but I think that you are better of pushing/fetching/pulling only between repositories that are forks/clones of each other. It looks like I'm gonna have to do another file copy merge. And ask you to fork Phoenix on github afterwards and clone from that fork and use the clone for work and the fork for pushing.

OK, but what's the advantage of me pushing to a fork over pushing to a newly created empty repo each time? Is it just to avoid creating multiple repos, and having to delete unwanted ones? Perhaps creating a new repo each time would be safer for now, until we know we've got our problems sorted out, even if it is a bit more work.


Well, actually, if you clone and fetch from joulupunikki/Phoenix then technically I think it doesn't matter if its empty repo or fork. Same data will be there. But the idea is to get this git/netbeans stuff working so you don't have to start with a new clone every time.

I have merged your second modifications and pushed to Phoenix. From now on I suggest you do all development work on separate work branches and push those work branches to github for me to see before we merge them with master.

Here's a list of things that came to mind/I did when committed.

1.Should feeding be only simulated for house factions ? Other factions, especially rebels, seem to have far more troops and cities than food and suffer from famine everywhere. I think in EFS only houses are simulated fully with eg. consume food and this is reflected in eg. not providing rebels with enough farms and they aren't gaining any more. In Phoenix all factions have so far been simulated equally but none of the datafiles in original EFS or the mods reflect this.

2.If unit or city health == 0 after health loss from famine unit or city should be destroyed ?

3. I modified Game.deleteUnit to remove unit from unmoved_units and to delete cargo. I think we could have a wrapper function which does optional cargo unloading before calling deleteUnit.

4. Added EfsIni implements Serializable

5. Harvest.java in dat can be deleted ?

6. EfsIni gets EFS.INI data as a java Properties object.


I've coded readers for PROD.DAT and RES.DAT, but haven't pushed my latest version yet. Having read RES.DAT, I now use the resource names from that file when displaying cargo pods. That meant that Util.drawUnitDetails needed access to the res table, for which it needed access to game, which it didn't have. So I added game to its parameters, and modified the calls from these methods to pass it: Util.drawStackDisplay, UnitInfoWindow.drawUnits, and UnitInfoWindow.drawDraggedUnit, and CombatWindow.renderCombatWindow.


Well, that should be ok.

tichtich
Assault Legionnaire
Posts: 56
Joined: Thu Jan 26, 2006 2:01 am
Location: Bristol, UK

Postby tichtich » Sun Feb 23, 2014 6:37 pm

flowerpot wrote:I have merged your second modifications and pushed to Phoenix. From now on I suggest you do all development work on separate work branches and push those work branches to github for me to see before we merge them with master.

OK. I've just made a fresh start, deleting all my repos at Github and my Phoenix project (after saving the src folder). Then I forked your repo, cloned the fork, made a new branch (rsw1) off the master, checking out that branch at the same time, copied my saved src folder over the project's src folder, committed everything, and pushed branch rsw1 to my fork without merging. I hope I've understood you correctly, and you didn't want me to merge, or edit my changes into your files.

Unfortunately, I didn't try compiling until after pushing, and now I've found there's a small problem. You've renamed a file (in folder gui) from Resources.java to Resource.java. When I copied my source over yours, I re-added the old Resources.java without deleting the new Resource.java. The new file was inconsistent with the rest of my source, and the compiler objected. No big deal, but I thought I'd better mention it, as you may be surprised I uploaded a version that doesn't compile!

Sorry, when I called my class "Resources" I didn't notice that you already had a class by that name in another folder, or I would have picked a different name, though it does seem natural to keep that word for referring to something that's called "resources" within the game itself. I felt justified in renaming an existing variable in Game.java from "resources" to "game_resources", since that variable was being used to store a reference to class GameResources, and, as it was a private variable, changing it wouldn't affect any other classes. But I wouldn't have gone so far as to name a class "Resources" if I'd noticed you already had a class of that name.

To help you merge, I think the only files we've both edited are Game and Util.

In Game, I made the following changes:
- Moved the reading in of data files up to the start of the initialisation. I thought that seemed wise, since they aren't going to change, but they might be used by some later initialisation.
- Added "turn" as a parameter to UpdateEconomy. That wasn't strictly necessary, but I thought it made things clearer.
- Added a getter for res_types.

In Util, I made the changes I already mentioned to drawUnitDetails and DrawStackDisplay (to show the resource names read from RES.DAT). And I moved my "cleanLine" method into Util, as it's used by all 3 of my readers.

flowerpot wrote:1.Should feeding be only simulated for house factions ? Other factions, especially rebels, seem to have far more troops and cities than food and suffer from famine everywhere. I think in EFS only houses are simulated fully with eg. consume food and this is reflected in eg. not providing rebels with enough farms and they aren't gaining any more. In Phoenix all factions have so far been simulated equally but none of the datafiles in original EFS or the mods reflect this.

Yes, I've been deliberately ignoring any differences between houses and other factions for the time being. (And perhaps AI houses follow some different rules from human houses.) I agree it seems pretty clear that the rebels don't eat. Apart from that, we have some decisions to make about the rules, and I for one would happily put that off for a while, as I have enough to think about already!

Incidentally, at the moment Game declares two variables: "turn" (which is the number of the current faction) and "current_faction" (which isn't used). Let's agree to continue calling it "turn" and delete "current_faction".

There are other places where we want to refer locally to a faction which isn't necessarily the current one. You've used the name "owner", since you're referring to a faction in the context of a unit (or maybe city) that it owns. I've followed that convention, e.g. with regard to resources. Earlier I'd used the local variable "faction", but I'm trying to avoid that now, as we have a class called Faction, and it would probably be best to try to keep the name "faction" for referring to objects of that class. Game now has an array faction[i] of such objects, where the index is a faction number. If we ever need a local faction number variable in a context where "owner" isn't appropriate (because we're not referring to ownership of anything), perhaps we should call it "faction_num", "faction_index", or for short "f_idx" (by analogy with p_idx).

Strictly speaking, perhaps we should be declaring such faction variables as an enum type instead of int. But I doubt it's worth changing. We're not using enum types much, and I for one am in no hurry to do so.

flowerpot wrote:2.If unit or city health == 0 after health loss from famine unit or city should be destroyed ?

Definitely. I should have thought of that, especially as I recently coded a deleteUnit method!

flowerpot wrote:3. I modified Game.deleteUnit to remove unit from unmoved_units and to delete cargo. I think we could have a wrapper function which does optional cargo unloading before calling deleteUnit.

Agreed, except that I think the wrapper should be called "deleteUnit"! That one would be the public method to be called for any disposal of units. The lower-level method (currently called deleteUnit) would be a private helper method only called by the wrapper.

Alternatively, we could get rid of the boolean parameter and have two public methods intended for general use: deleteUnitExcCargo and deleteUnitIncCargo.

My thinking is that I want there to be no doubt about which methods are approved for creation and disposal of units, so that we can be sure that all creation and disposal is being funnelled through just those methods. "Delete" is the word I've chosen as indicating an approved method for unit disposal, but I'm open to using a different word if you think another one is clearer, maybe "destroy". I just didn't want it to be anything that suggested it was restricted to loss in combat.

Ideally we would have such unit-related operations in the Unit class. Then only those intended for general use would be public and accessible. But Unit doesn't have access to all the game data that it would need to fulfill that role. Up till now you've restricted Unit to doing what it can do without game data. That means, for example, that some of the work of disembarking a unit is left to the caller, and not performed by Unit.disembark. That in turn leads to some code duplication, and a bit more so now that we're adding a third call to disembark. But, given the difficulties of providing game data to Unit, it's probably a cost worth accepting.

Since I didn't fully appreciate that you'd adopted this policy, I've been trying to force Unit to do some stuff with game data, namely set move_type, type_data and camo for new units. But I guess I should stick to your current policy of avoiding any game data in Unit. So I'm going to take that code out of Unit, and move it to createUnitInHex. You already use setMoveType() and setUnitTypeData() for setting such data at the start of the game. Continuing that policy, you just need to add setCamo(). Better, I'll create a single method "initialiseUnitGameData(unit)" that intialises move_type, type_data and camo for one unit. createUnitInHex can call that for new units, and you can initialise units at the start of the game with just one loop that calls it for each unit. If it later turns out we need to initialise any more unit data, we only have to add the new code in one place.

I'd also like to add a common initialisation method to Unit, for doing any initialisation that can be shared between both constructors. For example, I recently added a "turns_starving" field to Unit, which needs to be initialised to 0. It would be best to avoid duplicating that code in each constructor. Come to think of it, that's a case where the initialisation can probably be included in the field's declaration. But I quite like to make it clearer by putting it in the constructor or method.

flowerpot wrote:5. Harvest.java in dat can be deleted ?

??? That's my reader for the harvest DAT files: FARM.DAT, etc.

flowerpot wrote:6. EfsIni gets EFS.INI data as a java Properties object.

OK. (I don't know anything about java Properties yet.)

tichtich
Assault Legionnaire
Posts: 56
Joined: Thu Jan 26, 2006 2:01 am
Location: Bristol, UK

Postby tichtich » Sun Feb 23, 2014 6:54 pm

On the subject of your GameResources class, I'm not clear what this is for, and whether I should be putting anything in it myself. At the moment it contains only the UnitSpot table, which is only used by HexProc, and unlikely to be used by any other class. Perhaps it would make more sense to put that table in HexProc, like I've put the Harvest and Prod tables in Economy, as that's the only class that will use them. On the other hand, if you want to keep such tables in GameResources, perhaps I should move mine there.


Return to “Fixing it ourself”

Who is online

Users browsing this forum: Google [Bot] and 9 guests