Page 1 of 1

committed rev 10016.

PostPosted: 24 Jun 2011, 15:26
by Braids
run svn update (or run screaming...)


from http://code.google.com/p/cardforge/source/detail?r=10016:

Log message
* java 5 compatibility: eliminated all compiler errors and warnings for java 5;
this necessitated use of java's reflection mechanism in a few places where we
were using java 6 calls. so, the code will still take advantage of java 6
features if they are available. this may have been a blockheaded move, as other
developers are starting to think java 6 is the way to go. fortunately, all uses
of reflection are easily found with searches for getMethod and getField.
* methods instead of static fields: refactored direct references to static
fields of forge.AllZone, forge.ComputerUtil_Block2, and forge.Phase to use
method calls instead. i made the static member fields themselves private. this
changed many lines of code, but will help with future AI improvements. also, the
changes are simple. for example, instead of AllZone.HumanPlayer, use
AllZone.getHumanPlayer().
* removing unneeded code: removed SimultaneousEntry and SimultaneousEntryCounter
fields and associated methods from forge.PlayerZone_ComesIntoPlay. these fields
were only being written to and never read from.

Affected files ...
This revision affected a large number of files. Only a subset of 50 changed paths are being shown. To see all changed paths, use the svn log command-line. ...

Re: committed rev 10016.

PostPosted: 24 Jun 2011, 15:40
by Hellfish
Holy heck, that's a bigass commit. Better Java 5 compatibility AND a good step towards minmax AI? Can't find any faults with that. :)

Fake EDIT: Two missed instances of AllZone.ManaPool though, nothing major. :)

Re: committed rev 10016.

PostPosted: 24 Jun 2011, 15:52
by Braids
Hellfish wrote:Holy heck, that's a bigass commit. Better Java 5 compatibility AND a good step towards minmax AI? Can't find any faults with that. :) ... Fake EDIT: Two missed instances of AllZone.ManaPool though, nothing major. :)
thanks. i'm definitely feeling my Cabal Minion aspects more today.

sorry i missed those. i did run a test, but i think that was before i ran svn update myself. kudos to you for not running screaming. :wink:

if you're using java 5, you'll get benign stack traces on stdout in certain situations. i didn't really know how to reproduce them. i doubt users will notice, unless they run forge from CLI.

Re: committed rev 10016.

PostPosted: 24 Jun 2011, 16:08
by Chris H.
Maintaining java 5 compatibility for awhile longer is nice but I would not want anyone to devote too much of their free time to it. Having someone on the dev team who is capable of finding and then quickly fixing java 5 compatibility issues should help.

It does not surprise me to see that there were still a few spots with old dead code that had not yet been removed. Most of it should have been taken care of before. Sloth and Slapshot cleared out a lot of old trash as part of the AbilityFactory project.

Re: committed rev 10016.

PostPosted: 24 Jun 2011, 16:19
by Rob Cashwalker
I think "bigass commits" (thanks Hellfish) should be avoided if possible. It's easier to roll back individual changes if necessary.

Re: committed rev 10016.

PostPosted: 24 Jun 2011, 17:00
by Braids
Rob Cashwalker wrote:I think "bigass commits" (thanks Hellfish) should be avoided if possible. It's easier to roll back individual changes if necessary.
i tend to agree, but when it comes to refactoring, one must balance the quantity of changes while maintaining the ability to compile the code.

into how many and what size pieces should i have used? i could have committed the change to each class separately. i could have committed the change to each field separately. it made sense to me that since i would be changing a large number of files anyway, to do it in one shot.

would you rather roll back one large commit or track down 51 smaller ones that touch many of the same files in different places? (yes, that's approximately how many static fields i changed.)

i could have committed the java 5 edits separately. that would have made more sense.

Re: committed rev 10016.

PostPosted: 24 Jun 2011, 20:35
by Rob Cashwalker
No need to roll back now.. just something to consider in the future. In this case, I would've done each bullet point from above separately... but I would really have worked on them separately... there could have been weeks in-between my edits....

Draft mode took me over 2 months, because I never spent more than an hour or so here and there....

Re: committed rev 10016.

PostPosted: 24 Jun 2011, 22:03
by Braids
ok, so if the commit is complex enough to require bullet points, it's probably too big. i can see the wisdom in that. i'll try to remember.

i still owe you all a lazy cardfactory... that was part of the bargain. :wink: i am working on it along with my first commit to the braids-minimax branch. **grumble** merging in svn has been problematic. the trunk is safe. i don't want anyone to worry.

plus i downloaded RapidSVN. it seems to be handling it much better than the command line. in fact, it just finished a merge! gotta run.

Re: committed rev 10016.

PostPosted: 25 Jun 2011, 20:13
by Rob Cashwalker
Most of us just use Eclipse with Subclipse for the SVN.