rails-devel — List for discussion of Rails development

[Rails-devel] Refactoring & cleanups

[Erik V. <eri...@hc...>]

I have committed a bunch of changes, none of which should have any visible
consequences.

The main things are:
- Got rid of many static methods in GameManager and PhaseManager, all
replaced by instance methods.
- Removed many direct references from the UI to these classes, channeling
these through GameUIManager.
GameUIManager should in the future become the UI equivalent of GameManager
in the game engine,
and these two classes will share much of the game state information.
- To aid this, a new class Defs has a number of definitions of such shared
game parameters.
I suspect this class will be greatly extended in the future. 
This class also contains my first Java 5 enums.

The general purpose of changes like these is to gradually prepare the system

for a future client/server split as needed for an Internet play version,
by concentrating most of the UI/engine communication through a few dedicated
classes, 
in this case GameUIManager and GameManager (PossibleActions is another
example).

Erik Vos

[Rails-devel] Minor changes

[Erik V. <eri...@hc...>]

I have committed a few changes to GameManager and OperatingRound,
the most important one being that the OR number is now compatible with
Undo/Redo.
IIRC, Mike spotted this error a while ago.

Erik

Re: [Rails-devel] Reformatting

[Erik V. <eri...@hc...>]

I have changed the reformatting rules such, that lines will not be broken on
dots.
In case we want to split a long dot chain anyway, we can use the // trick
mentioned below. 

After picking up the latest patches, I have just reformatted and committed
everything.
So please refresh your sources!

Erik.

> -----Original Message-----
> From: Erik Vos [mailto:eri...@hc...] 
> Sent: Sunday 01 June 2008 14:57
> To: 'Development list for Rails: an 18xx game'
> Subject: Reformatting
> 
> I have reformatted all sources, but so far I have only 
> committed GameStatus_18EU and StatusWindow_18EU as (pretty 
> random) examples.
> 
> As I expected, automatic reformatting does no good on 
> wrapping long statements. 
> For some reason, Eclipse considers dots to be the most 
> preferrable line break points, rather than equals signs and 
> commas, as I would expect and prefer. And I see no way to 
> change that. 
> See StatusWindow_18EU: on lines 59-60 I get
>                     options[i] = LocalText.getText("N_Train", 
> trains.get(i)
>                             .getName());
> whereas it seems a lot more logical to me to make it like
>                     options[i] = LocalText.getText("N_Train",
> 				    trains.get(i).getName());
> This can in fact be achieved and retained by manually 
> inserting // at the preferred wrapping points,
> and as an example I have done that in the other committed 
> class, GameStatus_18EU.
> But it is a bit ugly, and it will probably never be completed 
> (as least not by me).
> 
> Any comments or suggestions before I proceed checking in (or 
> maybe undoing) the whole bunch?
> 
> Erik Vos
> 

Re: [Rails-devel] Patchfiles

[Brett L. <wak...@gm...>]

On Wed, 2008-06-04 at 00:20 +0200, Freek Dijkstra wrote:
> Sorry to bother you, but it seems part of my patch went wrong.
> One of the modifications was made on the wrong line.
> 
> http://rails.cvs.sourceforge.net/rails/18xx/rails/ui/swing/hexmap/GUIHex.java?revision=1.16&view=markup
> Now reads:
> 
>    799 		if (getHexModel().getDestinations() != null) {
>    800 			tt.append("<br><b>Destination</b>:");
>    801 			for (PublicCompanyI dest : getHexModel().getDestinations()) {
>    802 			    tt.append("</html>");
>    803                 tt.append(dest.getName());
>    804 			}
>    805 		}
> 
> This is wrong: the tt.append("</html>"); should be outside the for loop.
> (and the remove tt.append(" "); should be restored)
> 
> So it should read:
> 
>    799 	    if (getHexModel().getDestinations() != null) {
>    800 	        tt.append("<br><b>Destination</b>:");
>    801 	        for (PublicCompanyI dest : 
> getHexModel().getDestinations()) {
>    802 	            tt.append (" ");
>    803               tt.append(dest.getName());
>    804 	        }
>    805 	    }
>    806 	    tt.append("</html">);
>    807 	    toolTip = tt.toString();
>    808 	}
>    809
> 
> This was my fault: my diff file was in error; it seems I accidentally 
> sent a diff between two working copies, instead of a diff between a 
> working copy and the repository.
> 
> I like to blame CVS this time (I'm used to svn).
> 
> I just manually verified all other commits (including those from last 
> weeks commit) and all other files are fine.
> 
> Sorry,
> Freek
> 



Fixed checked in.   Thanks for double-checking. :-)


---Brett.

Re: [Rails-devel] Patchfiles

[Freek D. <sf_...@ma...>]

Sorry to bother you, but it seems part of my patch went wrong.
One of the modifications was made on the wrong line.

http://rails.cvs.sourceforge.net/rails/18xx/rails/ui/swing/hexmap/GUIHex.java?revision=1.16&view=markup
Now reads:

   799 		if (getHexModel().getDestinations() != null) {
   800 			tt.append("<br><b>Destination</b>:");
   801 			for (PublicCompanyI dest : getHexModel().getDestinations()) {
   802 			    tt.append("</html>");
   803                 tt.append(dest.getName());
   804 			}
   805 		}

This is wrong: the tt.append("</html>"); should be outside the for loop.
(and the remove tt.append(" "); should be restored)

So it should read:

   799 	    if (getHexModel().getDestinations() != null) {
   800 	        tt.append("<br><b>Destination</b>:");
   801 	        for (PublicCompanyI dest : 
getHexModel().getDestinations()) {
   802 	            tt.append (" ");
   803               tt.append(dest.getName());
   804 	        }
   805 	    }
   806 	    tt.append("</html">);
   807 	    toolTip = tt.toString();
   808 	}
   809

This was my fault: my diff file was in error; it seems I accidentally 
sent a diff between two working copies, instead of a diff between a 
working copy and the repository.

I like to blame CVS this time (I'm used to svn).

I just manually verified all other commits (including those from last 
weeks commit) and all other files are fine.

Sorry,
Freek

Re: [Rails-devel] Patchfiles

[Brett L. <wak...@gm...>]

On Tue, 2008-06-03 at 23:32 +0200, Freek Dijkstra wrote:
> Brett Lentz wrote:
> 
> >> +        if (tokenMode) {
> >> +            // nothing to show
> >> +        } else {
> > 
> > Is there a reason you didn't use "if (!tokenMode)" ?
> 
> No. I copied it, and forgot to check later (I dislike the implicit 
> assumption that there are only two modes, but never felt it was 
> important enough to improve).
> 
> Regards,
> Freek
> 

Ok.  I've committed your patch, and just simplified the statement.


---Brett.

Re: [Rails-devel] Patchfiles

[Freek D. <sf_...@ma...>]

Brett Lentz wrote:

>> +        if (tokenMode) {
>> +            // nothing to show
>> +        } else {
> 
> Is there a reason you didn't use "if (!tokenMode)" ?

No. I copied it, and forgot to check later (I dislike the implicit 
assumption that there are only two modes, but never felt it was 
important enough to improve).

Regards,
Freek

Re: [Rails-devel] Patchfiles

[Brett L. <wak...@gm...>]

On Tue, 2008-06-03 at 00:55 +0200, Freek Dijkstra wrote:
> Erik Vos wrote:
> 
> >> the upgrade value of a city while laying a tile, 
> > 
> > E.g. as a tooltip for the upgrade tiles at the left-hand side of the map?
> > Good idea.
> 
> Attached is a patch.
> 
> Regards,
> Freek Dijkstra

> plain text document attachment (tooltips.diff)
> diff -waur -x Entries -x Entries.log 18xx repos/rails/ui/swing/UpgradesPanel.java 18xx/rails/ui/swing/UpgradesPanel.java
> --- 18xx/rails/ui/swing/UpgradesPanel.java    2008-02-19 21:31:58.000000000 +0100
> +++ 18xx/rails/ui/swing/UpgradesPanel.java    2008-06-03 00:30:02.000000000 +0200
> @@ -280,9 +280,32 @@
>      }
>  
>      public void mouseEntered(MouseEvent e) {
> +        Object source = e.getSource();
> +        if (!(source instanceof JLabel)) return;
> +        
> +        if (tokenMode) {
> +            // nothing to show
> +        } else {

Is there a reason you didn't use "if (!tokenMode)" ?


> +        protected void setToolTip()
> +        {
> +            TileI currentTile = TileManager.get().getTile(internalId);
> +            StringBuffer tt = new StringBuffer("<html>");
> +            tt.append("<b>Tile</b>: ").append(currentTile.getName()); // or getId()

Tooltip text should support localization. We can catch this with a
second patch.


I'll simplify the tokenMode bits, and commit this shortly.

---Brett.

Re: [Rails-devel] Patchfiles

[Freek D. <sf_...@ma...>]

Erik Vos wrote:

>> the upgrade value of a city while laying a tile, 
> 
> E.g. as a tooltip for the upgrade tiles at the left-hand side of the map?
> Good idea.

Attached is a patch.

Regards,
Freek Dijkstra

[Rails-devel] Reformatting

[Erik V. <eri...@hc...>]

I have reformatted all sources, but so far I have only committed
GameStatus_18EU and StatusWindow_18EU as (pretty random) examples.

As I expected, automatic reformatting does no good on wrapping long
statements. 
For some reason, Eclipse considers dots to be the most preferrable line
break points, rather than equals signs and commas, as I would expect and
prefer. And I see no way to change that. 
See StatusWindow_18EU: on lines 59-60 I get
                    options[i] = LocalText.getText("N_Train", trains.get(i)
                            .getName());
whereas it seems a lot more logical to me to make it like
                    options[i] = LocalText.getText("N_Train",
				    trains.get(i).getName());
This can in fact be achieved and retained by manually inserting // at the
preferred wrapping points,
and as an example I have done that in the other committed class,
GameStatus_18EU.
But it is a bit ugly, and it will probably never be completed (as least not
by me).

Any comments or suggestions before I proceed checking in (or maybe undoing)
the whole bunch?

Erik Vos

Re: [Rails-devel] Patchfiles

[Erik V. <eri...@hc...>]

> Somewhere I found a remark that in Eclipse 3.x it is possible 
> to reformat
> all sources in one fell swoop,
> but I haven't yet discovered how to do that. I haven't 
> reformatted anything
> yet.

Found it here: http://www.artima.com/forums/flat.jsp?forum=121&thread=107852
Open Project Explorer view, right-click project, select Format. Not tried
yet.

Changed Rails format on one point: no blank lines between field declarations
(too spacious IMO).
If you're all agree, I'll reformat and commit the whole bunch next Sunday.

Erik.

Re: [Rails-devel] Patchfiles

[Iain C. <ia...@ch...>]

On 5/27/08, Brett Lentz <wak...@gm...> wrote:
>  We should start building out some developer documentation on the
>  website/wiki, starting with a CodingStyle page. Perhaps Iain has time to
>  do this?

Sure. I'll get onto it over the next week or so.

I am considering a wiki page for each implemented game and a separate
page/area for developer documentation.

-- 
Iain

Re: [Rails-devel] Patchfiles

[Brett L. <wak...@gm...>]

On Sat, 2008-05-24 at 20:35 +0200, Freek Dijkstra wrote:
> plain text document attachment (defaultgame.diff)
> diff -waur 18xx repos/my.properties 18xx/my.properties
> --- 18xx/my.properties	2007-07-23 21:59:16.000000000 +0200
> +++ 18xx/my.properties	2008-05-23 00:05:48.000000000 +0200
> @@ -55,8 +55,12 @@
>  ### Default players
>  # Comma-separated list of player names. 
>  # Useful for game testing purposes.
>  #default_players=Alice,Bob,Charlie 
>  
> +### Default game
> +# Name of game selected in the game selection window.
> +# Useful for game testing purposes.
> +#default_game=1830
>  
>  ####################### Log4J properties ##############################
>  # For information on how to customise log4j logging, see for instance
> diff -waur 18xx repos/rails/ui/swing/GameSetupWindow.java 18xx/rails/ui/swing/GameSetupWindow.java
> --- 18xx/rails/ui/swing/GameSetupWindow.java	2008-01-28 00:27:54.000000000 +0100
> +++ 18xx/rails/ui/swing/GameSetupWindow.java	2008-05-24 16:55:37.000000000 +0200
> @@ -176,9 +175,13 @@
>      }
>  
>      private void populateGameList(List<String> gameNames, JComboBox gameNameBox) {
> +		String preferredgame = Config.get("default_game");
>  	for (String gameName : gameNames) {
>  	    String gameText = gameName + " - " + GamesInfo.getNote(gameName);
>  	    gameNameBox.addItem(gameText);
> +			if (preferredgame.equals(gameName)) {
> +				gameNameBox.setSelectedItem(gameText);
> +			}
>  	}
>      }
>  


Patch applied.   

Sorry for the wait on this one. The (American) holiday weekend slowed me
down a bit.


---Brett.

Re: [Rails-devel] Patchfiles

[John A. T. <ja...@ja...>]

On Tue, 27 May 2008, Erik Vos wrote:

> I also had a look at Codestyle, but from a superficial glance I concluded
> that it does only reporting,
> no automatic or on-demand formatting. I doubt if it will add much to what we
> can already do with Eclipse
> to find simple whitespace differences.

I assume you mean Checkstyle?  It doesn't correct errors, but the nice 
thing is it flags them as errors/warnings (as configured), so once the 
codebase is cleaned up it stays that way.  Also, you can configure 
additional code style guidelines such as variable naming conventions (ie, 
all-caps constants, camel-case identifiers, etc).

--
John A. Tamplin					ja...@ja...
770/436-5387 HOME				4116 Manson Ave
                 		 		Smyrna, GA  30082-3723

Re: [Rails-devel] Patchfiles

[Brett L. <wak...@gm...>]

On Tue, 2008-05-27 at 23:53 +0200, Erik Vos wrote:
> > 
> > > Yes.  4 space indents, expand tab characters to spaces is 
> > fine by me.
> > 
> > My current setting is 4 with tabs, but in fact I almost never 
> > reformat, 
> > because the formatter tends to break lines in ways I don't like.
> > I'm pretty good at formatting manually (he boasted), but no doubt
> > tabs and spaces get mixed this way.
> > 
> > But I agree that it's time to sort this out. 
> 
> I have committed an experimental formatting standard for Eclipse named
> Rails-format.xml (in CVS under 18xx).
> It is an export from a new standard named "Rails" that I created out of the
> Eclipse default.
> The main change is, that tabs are not used at all - I found that advice in
> several places.
> For the rest it is... well, the way I like it...  
>  
> Erik.


Sounds good.

We should start building out some developer documentation on the
website/wiki, starting with a CodingStyle page. Perhaps Iain has time to
do this?


 ---Brett.


Only spammers and relatives send HTML mail.
     -- Faux_Pseudo

Re: [Rails-devel] Patchfiles

[Erik V. <eri...@hc...>]

> 
> > Yes.  4 space indents, expand tab characters to spaces is 
> fine by me.
> 
> My current setting is 4 with tabs, but in fact I almost never 
> reformat, 
> because the formatter tends to break lines in ways I don't like.
> I'm pretty good at formatting manually (he boasted), but no doubt
> tabs and spaces get mixed this way.
> 
> But I agree that it's time to sort this out. 

I have committed an experimental formatting standard for Eclipse named
Rails-format.xml (in CVS under 18xx).
It is an export from a new standard named "Rails" that I created out of the
Eclipse default.
The main change is, that tabs are not used at all - I found that advice in
several places.
For the rest it is... well, the way I like it...  

To use it, I guess it must be imported in the same place from where I
created it: 
Project|Properties|Java Code Style|Formatter.

Somewhere I found a remark that in Eclipse 3.x it is possible to reformat
all sources in one fell swoop,
but I haven't yet discovered how to do that. I haven't reformatted anything
yet.

I also had a look at Codestyle, but from a superficial glance I concluded
that it does only reporting,
no automatic or on-demand formatting. I doubt if it will add much to what we
can already do with Eclipse
to find simple whitespace differences.
 
Erik.

Re: [Rails-devel] Patchfiles

[brett l. <wak...@gm...>]

On Mon, May 26, 2008 at 2:24 PM, Erik Vos <eri...@hc...> wrote:
>> On Sun, May 25, 2008 at 1:42 PM, Erik Vos <eri...@hc...> wrote:
>> > Hi Freek,
>> >
>> > Welcome to the club! Your patch looks useful to me, but
>> I'll leave it to
>> > Brett to incorporate it.
>>
>> It looks good to me, but I'm not going to have a chance to merge it
>> until mid-week. Erik, if you want to merge it sooner, feel free.
>
> Here I must confess, that I have never taken the trouble so far to sort out
> how to deal with such patches... :-(
>

Ok. No worries there.  I'll take care of it as soon as I have time.

---Brett.

Re: [Rails-devel] Patchfiles

[Erik V. <eri...@hc...>]

> On Sun, May 25, 2008 at 1:42 PM, Erik Vos <eri...@hc...> wrote:
> > Hi Freek,
> >
> > Welcome to the club! Your patch looks useful to me, but 
> I'll leave it to
> > Brett to incorporate it.
> 
> It looks good to me, but I'm not going to have a chance to merge it
> until mid-week. Erik, if you want to merge it sooner, feel free.

Here I must confess, that I have never taken the trouble so far to sort out
how to deal with such patches... :-(
 
> > Coding rules: I have written most of the code, and am using 
> 4-space indents.
> > Things go wrong when Brett reformats with his (I believe) 
> 3-space indents
> > ... ;-)
> > Indeed we have never settled that and it sometimes looks 
> ugly. I propose 4
> > spaces....
> > (Tabs or spaces: does it matter? Should I care? Not sure 
> how to control it).
> >
> 
> Yes.  4 space indents, expand tab characters to spaces is fine by me.

My current setting is 4 with tabs, but in fact I almost never reformat, 
because the formatter tends to break lines in ways I don't like.
I'm pretty good at formatting manually (he boasted), but no doubt
tabs and spaces get mixed this way.

But I agree that it's time to sort this out. 

Erik.

Re: [Rails-devel] Patchfiles

[Erik V. <eri...@hc...>]

Well, it ssems that not everyone is using Eclipse. 
But I take your point and will have a look at Codestyle.
Erik. 

> -----Original Message-----
> From: rai...@li... 
> [mailto:rai...@li...] On Behalf 
> Of John A. Tamplin
> Sent: Monday 26 May 2008 08:10
> To: Dev...@sc...; 
> li...@sc...; 
> fo...@sc...; 
> "Ra...@sc...":an 18xx game
> Subject: Re: [Rails-devel] Patchfiles
> 
> brett lentz wrote:
> > On Sun, May 25, 2008 at 1:42 PM, Erik Vos 
> <eri...@hc...> wrote:
> >   
> > Yes.  4 space indents, expand tab characters to spaces is 
> fine by me.
> >
> > I must apologize for this, as its entirely my fault.  I work from a
> > couple different machines and haven't been consistent about my
> > formatting settings.
> >
> > In general, please just follow typical Java conventions.
> >   
> If everyone is using Eclipse, I strongly suggest setting 
> formatting and 
> checkstyle rules and everyone following those.  Otherwise, 
> you wind up 
> with tons of whitespace changes in the diffs.
> 
> --
> John A. Tamplin					ja...@ja...
> 770/436-5387 HOME				4116 Manson Ave
>                 		 		Smyrna, GA  30082-3723
> 
> 
> --------------------------------------------------------------
> -----------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2008.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel
> 

Re: [Rails-devel] Patchfiles

[John A. T. <ja...@ja...>]

brett lentz wrote:
> On Sun, May 25, 2008 at 1:42 PM, Erik Vos <eri...@hc...> wrote:
>   
> Yes.  4 space indents, expand tab characters to spaces is fine by me.
>
> I must apologize for this, as its entirely my fault.  I work from a
> couple different machines and haven't been consistent about my
> formatting settings.
>
> In general, please just follow typical Java conventions.
>   
If everyone is using Eclipse, I strongly suggest setting formatting and 
checkstyle rules and everyone following those.  Otherwise, you wind up 
with tons of whitespace changes in the diffs.

--
John A. Tamplin					ja...@ja...
770/436-5387 HOME				4116 Manson Ave
                		 		Smyrna, GA  30082-3723

Re: [Rails-devel] Patchfiles

[brett l. <wak...@gm...>]

On Sun, May 25, 2008 at 3:08 PM, Freek Dijkstra <sf_...@ma...> wrote:
> Erik Vos wrote:
>
>> Coding rules: I have written most of the code, and am using 4-space indents.
>> Things go wrong when Brett reformats with his (I believe) 3-space indents
>> ... ;-)
>> Indeed we have never settled that and it sometimes looks ugly. I propose 4
>> spaces....
>> (Tabs or spaces: does it matter? Should I care? Not sure how to control it).
>
> I don't care, as long as it is consistent.
>
> Let's look at the statistics of *.jave files in the rails directory:
>
> 39194 lines of code
> 30748 lines with some sort of indentation
>
> 2542 lines combining tabs and 2 or more spaces
> (That should exclude lines with extra space to indent /* */ comments)
>
> 18245 lines with tabs only
> 9825  lines with spaces only
>
> 69 lines with 3 spaces
> 4008 line with 4 spaces
> 82 lines with 5 spaces (huh?)
> 53 lines with 6 spaces
> 68 lines with 7 spaces
> 2404 lines with 8 spaces
> 43 lines with 9 spaces
> 1 line with 10 spaces
> 54 lines with 11 spaces
> 1442 lines with 12 spaces
> etc
>
> So it seems most popular is tabs, then 4-spaces, then 3-spaces.
>

Yep. It's pretty grotty.  Feel free to send in a cleanup patch, if you like.

---Brett.

Re: [Rails-devel] Patchfiles

[brett l. <wak...@gm...>]

On Sun, May 25, 2008 at 1:42 PM, Erik Vos <eri...@hc...> wrote:
> Hi Freek,
>
> Welcome to the club! Your patch looks useful to me, but I'll leave it to
> Brett to incorporate it.

It looks good to me, but I'm not going to have a chance to merge it
until mid-week. Erik, if you want to merge it sooner, feel free.

> Coding rules: I have written most of the code, and am using 4-space indents.
> Things go wrong when Brett reformats with his (I believe) 3-space indents
> ... ;-)
> Indeed we have never settled that and it sometimes looks ugly. I propose 4
> spaces....
> (Tabs or spaces: does it matter? Should I care? Not sure how to control it).
>

Yes.  4 space indents, expand tab characters to spaces is fine by me.

I must apologize for this, as its entirely my fault.  I work from a
couple different machines and haven't been consistent about my
formatting settings.

In general, please just follow typical Java conventions.

> Submissions: start with sending patches like these. In some stage Brett may
> give you commit access.
>

My rule of thumb is submit around 4-5 patches, and then I'm usually
seen enough of your work to be comfortable allowing you to commit
directly.

> Erik Vos

---Brett.

Re: [Rails-devel] Patchfiles

[Freek D. <sf_...@ma...>]

Erik Vos wrote:

> Coding rules: I have written most of the code, and am using 4-space indents.
> Things go wrong when Brett reformats with his (I believe) 3-space indents
> ... ;-)
> Indeed we have never settled that and it sometimes looks ugly. I propose 4
> spaces....
> (Tabs or spaces: does it matter? Should I care? Not sure how to control it).

I don't care, as long as it is consistent.

Let's look at the statistics of *.jave files in the rails directory:

39194 lines of code
30748 lines with some sort of indentation

2542 lines combining tabs and 2 or more spaces
(That should exclude lines with extra space to indent /* */ comments)

18245 lines with tabs only
9825  lines with spaces only

69 lines with 3 spaces
4008 line with 4 spaces
82 lines with 5 spaces (huh?)
53 lines with 6 spaces
68 lines with 7 spaces
2404 lines with 8 spaces
43 lines with 9 spaces
1 line with 10 spaces
54 lines with 11 spaces
1442 lines with 12 spaces
etc

So it seems most popular is tabs, then 4-spaces, then 3-spaces.

>> saving of preferences between games, 
> 
> What preferences? I can imagine a UI to manage my.properties might come in
> handy, althoug I wouldn't consider it a priority.

Saving the my.properties based on the last settings. For example, if the
last time I started a game of 18Kaas with John, Joe and Jane as players,
and unlimited D-trains, it would be nice to see those options as the
default the next time the program is run.

Regards,
Freek

Re: [Rails-devel] Patchfiles

[John A. T. <ja...@ja...>]

Erik Vos wrote:
> Coding rules: I have written most of the code, and am using 4-space indents.
> Things go wrong when Brett reformats with his (I believe) 3-space indents
> ... ;-)
> Indeed we have never settled that and it sometimes looks ugly. I propose 4
> spaces....
> (Tabs or spaces: does it matter? Should I care? Not sure how to control it).
>   
FWIW, on Google Web Toolkit we use the Checkstyle plugin for Eclipse, 
and define Eclipse format settings so everyone gets the same result when 
they autoformat.

--
John A. Tamplin					ja...@ja...
770/436-5387 HOME				4116 Manson Ave
                		 		Smyrna, GA  30082-3723

Re: [Rails-devel] Patchfiles

[Erik V. <eri...@hc...>]

Hi Freek,

Welcome to the club! Your patch looks useful to me, but I'll leave it to
Brett to incorporate it.

Coding rules: I have written most of the code, and am using 4-space indents.
Things go wrong when Brett reformats with his (I believe) 3-space indents
... ;-)
Indeed we have never settled that and it sometimes looks ugly. I propose 4
spaces....
(Tabs or spaces: does it matter? Should I care? Not sure how to control it).

Submissions: start with sending patches like these. In some stage Brett may
give you commit access.

For what it's worth, some initial comments about your further suggestions:

> how many trains the bank still has, 

You can see that in the GameStatus window. The subject of duplicating more
info from there into the OR window is coming up regularly. I'm not very much
in favour of that, as the OR window is already overcrowded - at least at my
resolution it often gets higher than my screen, so that I must scroll the
map. In the past we had it split into two windows, and I'm not convinced
that it was a good move to combine these.

But perhaps you actually meant putting the number into the text of the Buy
Train popup selction box. That's OK, but it's not as trivial a change as it
seems.

> the 
> upgrade value 
> of a city while laying a tile, 

E.g. as a tooltip for the upgrade tiles at the left-hand side of the map?
Good idea.

> stricter tile-laying rules 
> -companies can 
> now place unreachable tile-, 

... and tokens, yes. The concept of 'routes' (or just 'reachability') has
not been implemented yet. IMO it's not a 'minor' change.

> saving of preferences between games, 

What preferences? I can imagine a UI to manage my.properties might come in
handy, althoug I wouldn't consider it a priority.

> unlimited tiles -a variant I prefer to play-, 

Currently we only have per-game variants/options, but this one would qualify
as a useful generic option, perhaps to be put in my.properties. 

> perhaps even automatic 
> route calculation)
 
... and revenues, I suppose you mean... Sure we want that, but this is
definitely not a minor update....

Best regards,
Erik Vos