exe replacement

a place to discuss it

Moderators: Deathifier, Sukayo

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 14, 2014 4:18 pm

tichtich wrote:Ah, I didn't think of that. But now I'm surprised the compiler doesn't object to Game's "new HexProc(this)", since it can see perfectly well that this is a case of passing a self-reference in a constructor.

Well, I was reminded of passing this when I noticed that Netbeans underlined and put a light bulb next a row which passed this in a constructor. But now I don't see those light bulbs anymore. :?
I think the problem is only with passing a self-reference from a constructor, not to one, so I guess you could still get rid of Battleinit, put the initialisation in Battle's constructor instead, and call "new Battle" from Game.init. Nevertheless, the fact that we can't initialise in Game's constructor (and have to get Gui involved in calling init) is a minus for this system IMO.


Well, having said what I said, I have looked at various sources around the net and some say that passing this in constructor is ok if you are cognizant of the fact that your object is not yet constructed and you don't for example call any non-static functions using that reference.

Yes, I had in mind a static variable, but there's no need to add an extra variable. Just add "static" to the existing declaration of game. I just tried it, and it seemed to work.

Still, it doesn't smell right. We would have sub-objects of Game going over its head to Gui to get access to its data! I'm sure purists wouldn't like it.


On one hand, this would be convenient, but then as you say it does sound backward.

I've gone back to thinking about why HexProc and Battle are separate classes in the first place.
<clip>
I think what we really want is to put some methods in a separate file, but not in a separate class. Java doesn't provide that option, so we're trying to find a way of splitting into classes with a minimum of overhead.

If we do stick with your current system, there are two variations. You've used a mixture of both.
1. Pass only game to the constructor, which stores it. The methods can then access specific fields via getters, e.g. game.getUnitTypes().
2. Pass specific fields as parameters to the constructor, and store them, e.g. with "this type_data = type_data". They can then can be accessed directly.
<clip>

Sorry I don't have a strong recommendation for how to proceed. I can't make up my own mind.


Passing fields as parameters could get out of hand quickly. I would vote we pass game only.

- If we don't pass game at all, then presumably we avoid the problem of passing a self-reference from a constructor. (Or is there a problem with passing references to one's own fields from a constructor?)


The problem with passing this is just the possibly unintialized members, if a field was not initialized it would have a possibly undefined value.

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

Postby tichtich » Fri Feb 14, 2014 4:50 pm

flowerpot wrote:Well, having said what I said, I have looked at various sources around the net and some say that passing this in constructor is ok if you are cognizant of the fact that your object is not yet constructed and you don't for example call any non-static functions using that reference.

From what I've read there seem to be some issues with multi-threading, but I didn't pay much attention. I should think that, if we're doing nothing but assigning the game reference to a variable, there shouldn't be any problem.

EDIT: Of course the called constructor might be doing some other stuff as well, including accessing game data. So I guess we do have to be careful.

flowerpot wrote:Passing fields as parameters could get out of hand quickly. I would vote we pass game only.

I take it you mean pass the game once and save it (as we've been discussing), not pass it in every method call. OK, let's go with that. And we'll pass it from constructor to constructor, as you've done with HexProc. OK?

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

Postby tichtich » Fri Feb 14, 2014 7:28 pm

Thinking about this some more, one of the things that bothered me about using init was that it seemed to involve dividing Game's initialization up between init and Game's constructor. But why don't we transfer all Game's initialization to init, along with its parameters? Game's constructor would then become an empty no-arg constructor. In principle we wouldn't even need to declare it, though we might as well do so for the sake of clarity.

Also, I don't think we would need additional init's for HexProc and Battle, because those don't want to pass on their own "this" while initializing. Only classes which want to do that (which might include Faction) would need further init's, as far as I can see.

I've tried this out, and it seems to work. Based on some further browsing, it seems to be an accepted pattern. So my vote is now for this.

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

Postby tichtich » Sat Feb 15, 2014 6:48 am

I've finally managed to create a repository, merge it with your version 0.8.3, and push it to my own page at git: https://github.com/tichtich

From what you've said, you may not have done a merge yet, so I'll tell you a little about how it went. After a couple of failed attempts, I created a new repository from my version, fetched your 0.8.3 and then merged them. That worked, but I was forced to manually check each file on which the two versions differed. There seemed to be no files that we'd both changed, so it was just a matter of picking the file that had changed from 0.8.0. I should think git is capable of doing that automatically by itself. I suspect the problem was that I didn't seem to be comparing two branches with a common root, so the merge was a straight comparison between two versions, rather than comparing two with an earlier common source. I hope to find a better way of doing it next time. I did everything with NetBeans, not from GitBash.

As far as I can tell the merge was successful. At least based on a quick look, both my harvesting and your space button are working in the merged version.

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 15, 2014 3:31 pm

Edit aargh I overwrote my own post.
edit Removed duplicate.
Last edited by flowerpot on Sat Feb 15, 2014 8:22 pm, edited 3 times in total.

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 15, 2014 6:31 pm

tichtich wrote:
flowerpot wrote:Well, having said what I said, I have looked at various sources around the net and some say that passing this in constructor is ok if you are cognizant of the fact that your object is not yet constructed and you don't for example call any non-static functions using that reference.

From what I've read there seem to be some issues with multi-threading, but I didn't pay much attention. I should think that, if we're doing nothing but assigning the game reference to a variable, there shouldn't be any problem.

EDIT: Of course the called constructor might be doing some other stuff as well, including accessing game data. So I guess we do have to be careful.

I take it you mean pass the game once and save it (as we've been discussing), not pass it in every method call. OK, let's go with that. And we'll pass it from constructor to constructor, as you've done with HexProc. OK?


Now that you say it I feel that the Gods of Computing will frown upon us for including heretic code and Phoenix will become jinxed :) maybe we should pass game on every function, or at least we check that constructors don't access the passed game reference.

Thinking about this some more, one of the things that bothered me about using init was that it seemed to involve dividing Game's initialization up between init and Game's constructor. But why don't we transfer all Game's initialization to init, along with its parameters? Game's constructor would then become an empty no-arg constructor. In principle we wouldn't even need to declare it, though we might as well do so for the sake of clarity.

Also, I don't think we would need additional init's for HexProc and Battle, because those don't want to pass on their own "this" while initializing. Only classes which want to do that (which might include Faction) would need further init's, as far as I can see.

I've tried this out, and it seems to work. Based on some further browsing, it seems to be an accepted pattern. So my vote is now for this.


Somehow I lost the thread here. What exactly do you vote for ?

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 15, 2014 8:07 pm

tichtich wrote:I've finally managed to create a repository, merge it with your version 0.8.3, and push it to my own page at git: https://github.com/tichtich

From what you've said, you may not have done a merge yet, so I'll tell you a little about how it went. After a couple of failed attempts, I created a new repository from my version, fetched your 0.8.3 and then merged them. That worked, but I was forced to manually check each file on which the two versions differed. There seemed to be no files that we'd both changed, so it was just a matter of picking the file that had changed from 0.8.0. I should think git is capable of doing that automatically by itself. I suspect the problem was that I didn't seem to be comparing two branches with a common root, so the merge was a straight comparison between two versions, rather than comparing two with an earlier common source. I hope to find a better way of doing it next time. I did everything with NetBeans, not from GitBash.

As far as I can tell the merge was successful. At least based on a quick look, both my harvesting and your space button are working in the merged version.



Well the merge of the files appears to have gone more of less okay but something went quite not the right way. After pushing the merged files to github git blame there now reports tichtich as the author of most of the source files. Funny since I checked diffs with git and it reported only 5 files modified and 1 file added, apart from the source distribution compile files. The files did not show any change in diff but the authorship of the files seems to have changed. Did you say you merged two repositories with no common source ? I suspect the proper thing would have been to copy your sources onto a checked out revision of the Phoenix repository and then commit those files.

Edit. Why should I complain ? I have your repository. I can check out those files myself and copy them onto a checked out snapshot of Phoenix and then commit.

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

Postby tichtich » Sat Feb 15, 2014 10:03 pm

flowerpot wrote:Something went quite not the right way. On github git blame now reports tichtich as the author of most of the source files. Funny since I checked diffs with git and it reported only 5 files modified and 1 file added, apart from the source distribution compile files. Did you say you merged two repositories with no common source ?

What I think happened was this. I created a new repository from your 0.8.0 source files. But I didn't realize I needed to commit those. I assumed that initializing from a source would automatically commit it. I then copied my latest set of source files (including unchanged ones) over the top of yours. I committed that version. Then I fetched your 0.8.3 repository from github, so I could see both that one and mine in the repository browser, and I merged those two.

flowerpot wrote:I suspect the proper thing would have been to copy your sources onto a checked out revision of the Phoenix repository and then commit those files.


I tried something like that but ran into problems. I must have done something wrong. I've read chapters 2 and 3 of the manual, but I'm still a bit confused. It doesn't help that the manual uses command lines, and I'm trying to use NB.

Anyway, I just tried cloning again, after first deleting everything and trying to start from a bank slate. (Of course I've backed up my source files.) But it seems you've replaced your 0.8.3 repository at github with my merged version. So now I can't clone a version that's purely yours.

What do you want to do now? Can you revert to 0.8.3? Or have I irreparably infected your system with my merged version? Sorry!

flowerpot wrote:Somehow I lost the thread here. What exactly do you vote for ?.

I vote for the following system. If a class wants to pass "this" as an argument (to methods or constructors) during its own initialization, it must do all its initialization in a method (not its constructor) and have an empty no-arg constructor. Classes whose constructors only receive such a call (and don't make one) don't have to follow this rule. It looks like HexProc and Battle don't need to follow it.

So everything currently in Game's constructor would be moved to Game.init, along with its parameters. In Gui we would have:

Code: Select all

            game = new Game();
            game.init(args[1], 14);

We could get rid of Battle.battleinit, moving its contents into Battle's constructor, along with its parameters. And Game.init would call:

Code: Select all

        new Battle(random, damage, target, terr_cost, this, planets);


BTW here are some notes about what's in my merged version:
-- getHexesWithinRadiusOf still uses the parallel queues method. You can easily change it later if you want it to return the r's.
-- Harvests near map edge are correctly uprated proportionally for off-map hexes (excluding specials).
-- I haven't added p_idx to Hex (as I proposed). I've decided it's not yet worth it. I'll wait to see whether there are more cases where it would be useful.
-- createUnitInHex now calls spotProc to update spotted[] and I've check that that works.

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

Postby tichtich » Sat Feb 15, 2014 10:10 pm

flowerpot wrote:Edit. Why should I complain ? I have your repository. I can check out those files myself and copy them onto a checked out snapshot of Phoenix and then commit.

I suspect that means that now you'll be blamed for my work. But that's OK with me.

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 15, 2014 10:25 pm

What do you want to do now? Can you revert to 0.8.3? Or have I irreparably infected your system with my merged version? Sorry!


Don't worry ! Rest assured that if I didn't have multiple up to date backups on this computer and on flashdrives I would be a fool :)

edit
I suspect that means that now you'll be blamed for my work. But that's OK with me.


No, the authorship data should be exactly correct once I manage to replace the repository.

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 15, 2014 11:30 pm

Repository replaced. You can now delete your forked Phoenix and fork the replaced repository again. All authorship info, including yours, is up to date. Well we would have avoided this little diversion altogether if I had provided instructions on contributing in the first place. And I should have myself done that copy modifications onto checked out snapshot trick instead of merging. Sorry if I made you think you have caused permanent damage. I should have been clearer that no damage was done.

There is currently a bug in Phoenix which prevents factions from seeing their own newly created units when they are alone in a stack. It will be a one line fix. I also plan to include your expanded stack function


Code: Select all

public Stack getExpandedStack(Stack stack) {    // Returns a temporary stack with cargo listed separately
    Stack ret_val = new Stack();
    for (Unit unit : stack) {
        ret_val.add(unit);
        for (Unit cargo : unit.cargo_list) {
            ret_val.add(cargo);
        }
    }
    return ret_val;
}   



Then you can iterate through a stack with...

Code: Select all

for (Unit unit : getExpandedStack(stack)) {
    ...
}


in the Util class soonest. And I have nearly finished basic unit building, only handling of input units is still missing.

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

Postby tichtich » Sun Feb 16, 2014 2:34 am

I've downloaded your latest revision. Thanks for clearing things up. Git has rather a lot of unfamiliar terminology to get to grips with, and several things are referred to by two different names, like "staging area" and "index". But I think I basically understand it now.

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 » Sun Feb 16, 2014 7:06 pm

tichtich wrote:
flowerpot wrote:Something went quite not the right way. On github git blame now reports tichtich as the author of most of the source files. Funny since I checked diffs with git and it reported only 5 files modified and 1 file added, apart from the source distribution compile files. Did you say you merged two repositories with no common source ?

What I think happened was this. I created a new repository from your 0.8.0 source files. But I didn't realize I needed to commit those. I assumed that initializing from a source would automatically commit it. I then copied my latest set of source files (including unchanged ones) over the top of yours. I committed that version.


I just read this with thought and yes I think that is where the authorship data was changed.

Then I fetched your 0.8.3 repository from github, so I could see both that one and mine in the repository browser, and I merged those two.


So you managed to merge two repositories ? I didn't know that was possible. When talking about merging it usually means merging two branches inside a single repository. Here's what I did just now. I have the new Phoenix repo, call it PHOENIX, and the repo with altered authorship data, call it WORK. In WORK I have the newest unit building code on a branch called unit_building. I want to import that into PHOENIX. I have the two repositories side by side in a directory, call it DIR, PHOENIX is named Phoenix and WORK is named Source and I have the following script in a file called copy.sh in DIR:

Code: Select all

#!/bin/bash

PHOENIX=Phoenix/src/
SOURCE_REPO=Source/src/
DIRS=$(ls $SOURCE_REPO)

for dir in $DIRS
do
  cp $SOURCE_REPO$dir/*.java $PHOENIX$dir
done


So I open WORK in NetBeans and check out the newest commit in unit_building branch. I close WORK in NetBeans. I then open PHOENIX in netbeans and checkout the newest unit_building branch (which is behind WORKs' unit_building branch.) I close PHOENIX in netbeans.

I then execute the script copy.sh in git bash with

Code: Select all

bash copy.sh
. It copies the source files checked out in WORK onto the source files checked out in PHOENIX. I then open PHOENIX in netbeans and commit the copied files. I now have the latest unit building code included in my new Phoenix repository. When I have finished unit building I will merge unit_building branch into master branch.

If you still have code you want to include in Phoenix in a repository you made from those v0.8.0 sources then you could do the above, just replace unit_building branch in Phoenix with v0.8.0 tag.

I tried something like that but ran into problems. I must have done something wrong. I've read chapters 2 and 3 of the manual, but I'm still a bit confused. It doesn't help that the manual uses command lines, and I'm trying to use NB.


When I started a year ago I had only galaxyreader and gui and I had them in different repositories. And galaxyreader was a library used by gui. I also didn't use temporary branches but instead developed on the master branch. It takes time to get used to things. I use netbeans for commit, checkout and jumping between branches and git bash for managing branches, tagging and pushing and fetching. I haven't done that many merges, I have yet to see a hairy merge conflict.

flowerpot wrote:Somehow I lost the thread here. What exactly do you vote for ?.

I vote for the following system. If a class wants to pass "this" as an argument (to methods or constructors) during its own initialization, it must do all its initialization in a method (not its constructor) and have an empty no-arg constructor. Classes whose constructors only receive such a call (and don't make one) don't have to follow this rule. It looks like HexProc and Battle don't need to follow it.

So everything currently in Game's constructor would be moved to Game.init, along with its parameters. In Gui we would have:

Code: Select all

            game = new Game();
            game.init(args[1], 14);

We could get rid of Battle.battleinit, moving its contents into Battle's constructor, along with its parameters. And Game.init would call:

Code: Select all

        new Battle(random, damage, target, terr_cost, this, planets);


Okay, I think this sounds reasonable. I will finish unit building and then move all Game initialization code into Game.init. Of course this does not absolve us from the duty of initializing resources before using them but at least it should make uninitialized references into proper null pointers instead of random addresses.

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

Postby tichtich » Sun Feb 16, 2014 11:06 pm

flowerpot wrote:So you managed to merge two repositories ? I didn't know that was possible.

I was still very confused when I wrote that. I assumed that "fetching" your repository just brought the repository to me, so I then had two repositories. I now understand it better. I believe a fetch brings the contents of a remote repository into one's own repository, but doesn't do a merge. (On the other hand, I think a "pull" automatically does a merge.) I think perhaps the fetch gave me two separate trees within my repository, because I had started my depository independently of yours (instead of cloning). I believe I do basically now understand the principles of operations within a repository. But I'm not so confident about introducing material from one repository into another.

Sorry, my brain switches itself off every time I try to read the next part of your post. ;) I've just drunk a strong cider, which may not help. I don't want to start learning the command line language if I can help it. I'm hoping to do everything in NetBeans. Next time you upload a repository, I'll fetch it, look carefully at what I then have, and hopefully this time I'll understand the situation, consulting the manual as necessary before proceeding. I must admit that last time I was just trying out options I didn't understand, and hoping for the best.

At the moment I'm happily working from the repository I cloned from you yesterday. I'm adding resource consumption, including eating food, with universal warehouse on or off. It's turned out to be trickier than I thought. Just searching through the unit list (for cargo pods or units to be fed) means that they get used up or fed in a pseudo-random order. But I've found that EFS has certain priorities. 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. 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'm planning soon to try a stress test: filling up the galaxy with units, to see whether my searching works at an acceptable speed.

flowerpot wrote:Okay, I think this sounds reasonable. I will finish unit building and then move all Game initialization code into Game.init. Of course this does not absolve us from the duty of initializing resources before using them but at least it should make uninitialized references into proper null pointers instead of random addresses.


OK. Before we next merge I'll probably split most of my Game stuff off into one or more separate classes, and I'll use that system where necessary.

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

Postby tichtich » Mon Feb 17, 2014 6:40 pm

I've now written a fillGalaxy method for filling the game with units, so I can do some speed tests with many units under a variety of conditions. It lets you choose how many planets you want to put units on, how many stacks on each planet, and how many units per stack. The following tests were done with 10 units per stack, 100 stacks per planet, on each of 20 planets. That's 20,000 units total, or 4,000 per house. (These are in addition to the units in the standard galaxy at the start of the game.)

I tested my consumeResources method, which consumes one type of resource at a time, based on the worst case scenario, where there's none of the resource available, so you end up searching the whole list in question. This is pessimistic, since normally most searches will be successful. If a resource is plentiful and well-distributed, you're likely to find it early in the search. Still, sticking to this scenario made for more consistent tests. The figures below are average ms for one search. Bear in mind my PC is relatively slow, a 6-year-old basic laptop, with Intel 2.0 Ghz T7250 Core2 Duo processor.

Universal Warehouse OFF...
Searching through unit list: 2.087
Searching surface of one planet: 0.349/0.154

Universal Warehouse ON...
Searching through unit list: 2.001
Searching surface of all planets: 12.109

The split figure (0.349/0.154) is for searching a planet filled by fillGalaxy, and then a typical house home world at the start of the game, which has far fewer units.

This isn't too bad, but I'm a bit concerned the later game may be rather slow with UW ON. I suppose it's not worth worrying too much about that now. I may do some more tests after I've implemented secondary resource production.

P.S. When searching a planet's surface, I'm using getExpandedStack to look for cargo, and this slows things down. If I search stacks unexpanded (and mostly there's nothing to expand since my automatically generated stacks don't contain cargo) it halves the search time.


Return to “Fixing it ourself”

Who is online

Users browsing this forum: No registered users and 28 guests