rails-devel

[Rails-devel] Phase mgmt step 4, and 18TN Civil War (cee06b)

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

I have implemented:

1. Phase management step 4: <Phase> can now have an <Action> child with
(required) name and (optional) value attributes.
Each action causes gameManager.processPhaseAction (name, value) to be called
when that phase is activated.  
GameManager in turn calls processPhaseAction(name, value) in the current
round.

2. This new feature is used to implement 18TN Civil War (except the revenue
calculation).
The new code is in OperatingRound_18TN (updated) and PublicCompany_18TN
(new).
The latter method has a civilWar BooleanState that is set and unset
according to the 18TN rules.  This already works. 

3. To fully implement the 18TN rules, the CivilWar setting also requires
publicCompany.hasRoute() to return true. 
This is a new stub that always returns true (and is not used anywhere else
yet).
I hope we will once be able to detect route existence in the game engine.

Stefan, I trust you will now be able to finish 18TN completely by
implementing the effect of the Civil War on the revenue calculation.

To check the company CivilWar status you could call its isCivilWar() method
directly , but from an architectural POV (client/server separation) it would
be preferable, if at all possible, to create an Observer (ViewObject) per
company to register at the ModelObject (=BooleanState) via the company
getCivilWar() method.  

Erik.

Re: [Rails-devel] Phase mgmt step 4, and 18TN Civil War (cee06b)

[Stefan F. <ste...@we...>]

See my comments below.

As always the train not to run will chosen such that the revenue is optimized.

If the president wants another choice he can adjust the revenue. 

Interestingly 18TN is one the games where one stockholder can point out a 
better route and enforces the president to use that. Seems that the rules are 
slightly contradicting here, however I would use the general approach here 
that the more specific rule (civil war with president choice) would have 
priority here.

On Tuesday, August 09, 2011 11:18:30 pm Erik Vos wrote:
> I have implemented:
> 
> 1. Phase management step 4: <Phase> can now have an <Action> child with
> (required) name and (optional) value attributes.
> Each action causes gameManager.processPhaseAction (name, value) to be
> called when that phase is activated.
> GameManager in turn calls processPhaseAction(name, value) in the current
> round.

I do not need to remind you that I am not happy that the Round mechanisms get 
more complicated again. Is there really no solution possible that avoids 
putting phase mechanisms inside the rounds/gamemanager classes again?

> 
> 2. This new feature is used to implement 18TN Civil War (except the revenue
> calculation).
> The new code is in OperatingRound_18TN (updated) and PublicCompany_18TN
> (new).
> The latter method has a civilWar BooleanState that is set and unset
> according to the 18TN rules.  This already works.

Again OperatingRound ;-)
So my solution will be that PublicCompany_18TN will implement a 
RevenueStaticModifier and will remove the shorter train 

I assume that you have implemented the rule that a company with only one train 
(thus zero revenue) has its token unmoved instead of going left.

> 
> 3. To fully implement the 18TN rules, the CivilWar setting also requires
> publicCompany.hasRoute() to return true.
> This is a new stub that always returns true (and is not used anywhere else
> yet).
> I hope we will once be able to detect route existence in the game engine.

Existence of a route checking is easy to implement and is on my todo-list.
I will add that asap.

> 
> Stefan, I trust you will now be able to finish 18TN completely by
> implementing the effect of the Civil War on the revenue calculation.
> 
> To check the company CivilWar status you could call its isCivilWar() method
> directly , but from an architectural POV (client/server separation) it
> would be preferable, if at all possible, to create an Observer
> (ViewObject) per company to register at the ModelObject (=BooleanState)
> via the company getCivilWar() method.

No the push mechanism I use here is the RevenueStaticModifier (see above).

> 
> Erik.
> 
> 
> ---------------------------------------------------------------------------
> --- uberSVN's rich system and user administration capabilities and model
> configuration take the hassle out of deploying and managing Subversion and
> the tools developers use with it. Learn more about uberSVN and get a free
> download at:  http://p.sf.net/sfu/wandisco-dev2dev
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel

Re: [Rails-devel] Phase mgmt step 4, and 18TN Civil War (cee06b)

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

Some answers below.

> -----Original Message-----
> From: Stefan Frey [mailto:ste...@we...]


> > 1. Phase management step 4: <Phase> can now have an <Action> child
> > with
> > (required) name and (optional) value attributes.
> > Each action causes gameManager.processPhaseAction (name, value) to be
> > called when that phase is activated.
> > GameManager in turn calls processPhaseAction(name, value) in the
> > current round.
> 
> I do not need to remind you that I am not happy that the Round mechanisms
> get more complicated again. Is there really no solution possible that
avoids
> putting phase mechanisms inside the rounds/gamemanager classes again?

GameManager is the central action broker, and action execution is controlled
in the various Round classes. 
That's the current architecture, and I have no plan to change that,
certainly not without a clear vision on what could be a better architecture
and a road map to get there.

Another reason is that I did not want to make this new feature specific for
any particular type of action.
I could have restricted its reach to company properties by putting this new
method into CompanyManager (and subclass it for 18TN), but then this new
phase-action mechanism would no longer be generic, which is what I want.

And thirdly: if we would start scattering around all kinds of action paths
over all kinds of different managers, that would IMO rather complicate than
simplify the structure of Rails.

> > 2. This new feature is used to implement 18TN Civil War (except the
> > revenue calculation).
> > The new code is in OperatingRound_18TN (updated) and
> > PublicCompany_18TN (new).
> > The latter method has a civilWar BooleanState that is set and unset
> > according to the 18TN rules.  This already works.
> 
> Again OperatingRound ;-)

Inevitable, as I'm overriding an existing OR method to implement this
specific 18TN feature.  Besides, OperatingRound_18TN already existed for
other purposes.

> So my solution will be that PublicCompany_18TN will implement a
> RevenueStaticModifier and will remove the shorter train
> 
> I assume that you have implemented the rule that a company with only one
> train (thus zero revenue) has its token unmoved instead of going left.

Yes.  It works.
 
> >
> > 3. To fully implement the 18TN rules, the CivilWar setting also
> > requires
> > publicCompany.hasRoute() to return true.
> > This is a new stub that always returns true (and is not used anywhere
> > else yet).
> > I hope we will once be able to detect route existence in the game
engine.
> 
> Existence of a route checking is easy to implement and is on my todo-list.
> I will add that asap.
> 
> >
> > Stefan, I trust you will now be able to finish 18TN completely by
> > implementing the effect of the Civil War on the revenue calculation.
> >
> > To check the company CivilWar status you could call its isCivilWar()
> > method directly , but from an architectural POV (client/server
> > separation) it would be preferable, if at all possible, to create an
> > Observer
> > (ViewObject) per company to register at the ModelObject
> > (=BooleanState) via the company getCivilWar() method.
> 
> No the push mechanism I use here is the RevenueStaticModifier (see
> above).

Fine.

Erik.

[Rails-devel] Current implementation, branch Rails 2.0?

[Stefan F. <ste...@we...>]

On Wednesday, August 10, 2011 12:31:42 pm Erik Vos wrote:
> GameManager is the central action broker, and action execution is
> controlled in the various Round classes.
> That's the current architecture, and I have no plan to change that,
> certainly not without a clear vision on what could be a better architecture
> and a road map to get there.
> 
> Another reason is that I did not want to make this new feature specific for
> any particular type of action.
> I could have restricted its reach to company properties by putting this new
> method into CompanyManager (and subclass it for 18TN), but then this new
> phase-action mechanism would no longer be generic, which is what I want.
> 
> And thirdly: if we would start scattering around all kinds of action paths
> over all kinds of different managers, that would IMO rather complicate than
> simplify the structure of Rails.

Erik,
I do not agree here with your design:

In my understanding of Rails the term action is something different: All of 
them are user's actions and extend  the abstract class PossibleAction and 
belong to the rails.game.action package.

This new kind of "action" is in my eyes a named trigger which is activated by 
the train purchase. There is no need to pass the trigger through GameManager 
and Round classes with new methods and then call from the OperatingRound the 
method inside PublicCompany_TN.

My preferred solution would have been the possibility for PublicCompany_TN to 
register themselves with the PhaseManager and then get updated directly.

And only by convention actions are passed from GameManager to the Round 
(sub-)classes , but they design neither encourages nor enforces this:
Take for example the correction actions, they are processed in individual 
manager classes, and there is one for each action. If I have taken your 
approach all code inside the correction package would have been added to the 
GameManager too.

And that is my proposal or roadmap for those classes: Try to get GameManager 
focused on what is required for the top level class of the game (thus mainly 
storing the other component managers and interacting between them). Clean up 
the Rounds to include only the mechanisms to proceed from activity/step to the 
next and to switch between players/companies. All action processing should be 
done outside in individual components/classes.

For you it might seem easier to have everything in one place/file, but  I 
would prefer to refactor it somehow, before I am able to implement required 
adjustments for other games. 

Similar as it is difficult for you to write revenue code, it is difficult for 
others to change the Round or GameManager classes. But if nearly everything 
important is inside those classes, no one else will be able to help you 
implement the parts of new games that require changing those classes.

But if you split those classes into smaller pieces it makes it easier for 
outsiders (including myself) to understand the insides and it is less risky to 
change it without causing side effects.

See below for some stats if you do not believe my words.

My intention is not to blame you or to a blunt critique: It is my sole 
intention to help with coding. But I realize that I am mainly adding code 
around the core (revenue calculation, configuration, correction, states etc.) 
as I avoid touching the Round/GameManager  components.

And adding another algorithm for revenue gets a little boring if we are still 
waiting for 1825 and 1835 to work properly/fully.

However as I seem it hardly possible to refactor against a moving target (as 
you are stilling adding new functions to the core classes - and rightly do so, 
as long as there is no alternative), I suggest to create a new branch inside 
the git repo for a potential Rails 2.0. 

This branch would allow to refactor main parts without any fear to break the 
existing code/games/save files etc. It would also allow cleaning up from 
legacy code and components and a better support for unit testing.

In the main trunk new games/features could still be added to Rails 1.x, until 
Rails 2.0 is stable/mature enough.

Stefan


Some numbers on the size of classes in Rails.

The package Rails.game has 40960 instructions.

From those are the 5 largest
OperatingRound 7062 
PublicCompany 3817
GameManager 3623
StockRound 3200
MapHex 2962

And here you still have to consider that for PublicCompany and MapHex most of 
the code is in fact initialization and configuration.

Compare this to the top 6 largest packages (after rails.game)
rails.ui.swing 27949
rails.algorithms 12198
rails.game.action 5616
rails.ui.swing.hexmap 4792
rails.game.specifc._1856 4538
(from those the largest are again Round subclasses with a
sum of over 4000 instructions).
rails.game.specific._18EU 4136
(from those the largest are again Round subclasses with a 
sum of over 3000 instructions). 

Taken altogether it is not too far fetched that nearly 50% of all Rails.game 
code is part of the Round classes and subclasses. And the largest of it 
exceeds all other packages of rails except two.

And if you think about so important packages like move, model and state:
rails.game.move 1776
rails.game.model 863
rails.game.state 798

And those three packages consist of 10+ classes each.

Re: [Rails-devel] Current implementation, branch Rails 2.0?

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

On Thu, Aug 11, 2011 at 10:09 AM, Stefan Frey <ste...@we...> wrote:
> On Wednesday, August 10, 2011 12:31:42 pm Erik Vos wrote:
> However as I seem it hardly possible to refactor against a moving target (as
> you are stilling adding new functions to the core classes - and rightly do so,
> as long as there is no alternative), I suggest to create a new branch inside
> the git repo for a potential Rails 2.0.
>
> This branch would allow to refactor main parts without any fear to break the
> existing code/games/save files etc. It would also allow cleaning up from
> legacy code and components and a better support for unit testing.
>
> In the main trunk new games/features could still be added to Rails 1.x, until
> Rails 2.0 is stable/mature enough.
>
> Stefan


I'm open to this. It sounds like public topic branches may be
necessary for publishing larger chunks of work prior to being merged
into master. It may also help us discuss more extensive changes, if we
can pull down a published branch to review the changes. Sharing the
code is often easier than trying to describe the idea.

Pushing a new branch is easy:  'git push origin remotebranch', which
is shorthand for 'git push origin localbranch:remotebranch'
Deleting branches is also easy:  'git push origin :remotebranch',
which means "push NULL to remotebranch"


I also agree with the assertion that some of our classes have probably
become "catch-all" buckets for functionality. I've been having similar
problems un-knotting the XML parsing code from the *Manager
initialization code. It's a bit of a mess.


---Brett.

Re: [Rails-devel] Current implementation, branch Rails 2.0?

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

See below.

> -----Original Message-----
> From: Stefan Frey [mailto:ste...@we...]

> In my understanding of Rails the term action is something different: All
of
> them are user's actions and extend  the abstract class PossibleAction and
> belong to the rails.game.action package.
> 
> This new kind of "action" is in my eyes a named trigger which is activated
by
> the train purchase. 

By the phase change, to be precise.  Indeed the term "action" is a bit
confusing.

> There is no need to pass the trigger through
> GameManager and Round classes with new methods and then call from the
> OperatingRound the method inside PublicCompany_TN.
> 
> My preferred solution would have been the possibility for
> PublicCompany_TN to register themselves with the PhaseManager and then
> get updated directly.

That's indeed another possible approach.  I'll consider it.
It's probably more fashionable. 

> And only by convention actions are passed from GameManager to the Round
> (sub-)classes , but they design neither encourages nor enforces this:
> Take for example the correction actions, they are processed in individual
> manager classes, and there is one for each action. 

I'm not sure if that doesn't amount to multiplying entities beyond
necessity.

> If I have taken your
> approach all code inside the correction package would have been added to
> the GameManager too.
> And that is my proposal or roadmap for those classes: Try to get
> GameManager focused on what is required for the top level class of the
> game (thus mainly storing the other component managers and interacting
> between them). Clean up the Rounds to include only the mechanisms to
> proceed from activity/step to the next and to switch between
> players/companies. All action processing should be done outside in
individual
> components/classes.
> 
> For you it might seem easier to have everything in one place/file, but  I
would
> prefer to refactor it somehow, before I am able to implement required
> adjustments for other games.
> 
> Similar as it is difficult for you to write revenue code, it is difficult
for others to
> change the Round or GameManager classes. But if nearly everything
> important is inside those classes, no one else will be able to help you
> implement the parts of new games that require changing those classes.
> 
> But if you split those classes into smaller pieces it makes it easier for
> outsiders (including myself) to understand the insides and it is less
risky to
> change it without causing side effects.

As I proposed before, I can reorganize OperatingRound to group the methods
by function (OR step), each with some descriptive header.
That's all I can do on short term. It might make OR easier to read, and
perhaps it could be a starter for later refactoring.

> See below for some stats if you do not believe my words.

All fine, but so what?  Is size bad per se?  

> However as I seem it hardly possible to refactor against a moving target
(as
> you are stilling adding new functions to the core classes - and rightly do
so, as
> long as there is no alternative), I suggest to create a new branch inside
the git
> repo for a potential Rails 2.0.

Large-scale refactoring of Rails should certainly be done in a separate
branch.

Erik.

Re: [Rails-devel] Current implementation, branch Rails 2.0?

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

On Thu, Aug 11, 2011 at 1:02 PM, Erik Vos <eri...@xs...> wrote:
>> See below for some stats if you do not believe my words.
>
> All fine, but so what?  Is size bad per se?

Class size is a leading indicator of excessive complexity, conflicting
goals, unclear design, or just generally difficult to understand code.
It's not a hard and fast rule, but it's usually an indicator of
"smelly code". [1] [2]

In general, a class should have a single, clearly defined
responsibility. I think the Hex, MapHex, GUIHex classes do a pretty
decent job of staying within their defined sets of responsibilities.
It's not perfect, but I think it illustrates the point. Also, I'm not
saying that there is a set line count limit for all classes. For
example, much of the GUI classes are going to be somewhat lengthy just
by virtue of how AWT/Swing is designed.

We're just getting to the point in the lifecycle of the project where
some of our old design decisions need to be revisited. There's a few
of those old decisions that, if we're going to be able to continue
moving the project forward, are going to need to be re-decided in
light of where we're at now.

---Brett.

[1] http://www.codinghorror.com/blog/2006/05/code-smells.html
[2] http://www.ibm.com/developerworks/java/library/j-cq06306/index.html

Re: [Rails-devel] Current implementation, branch Rails 2.0?

[Stefan F. <ste...@we...>]

> All fine, but so what?  Is size bad per se?
> 

Erik,
No certainly not. All this is not about good or bad. In one sense the size 
shows the thoughts and efforts for all details you have put into those 
classes.

It is the other way around: As I do not have the time and the memory to 
understand all thoughts and conditions you considered, that created that code, 
I would prefer to digest all that in little portions. 

Organizing code into classes allows to review only the relevant classes that 
you need know. It is like organizing stuff into little boxes where you put 
labels on the boxes what is inside. This makes it much easier to find things 
and to understand the big picture: Maybe like in a big department store: It is 
much easier to get an overview by studying the directory of the store instead 
of searching every shelf.

An object orientated languages allows storing pieces of code in classes and if 
I want rewrite/change/add something to a class, I have to make sure that after 
my changes everything works as before. If the class is small, it is easier to 
check if I do not introduce side effects and I only have to test the part 
where the class is responsible for: So take the example for the 
OperatingRound. If I change anything inside I have to test all functionality 
which is managed by the OperatingRound. Thus I change something related to 
token lay, it can potentially have side effects on tile laying, revenue 
payouts, train buying, loans etc. 

If the token lay code is inside in a separate class it is impossible to effect 
the other functionality of OperatingRound. 

I had the issue more than a year ago: I started adding the NoMap 
functionalities which in principle should only change Tile and Token Lay. 
However due to side effects of my (at that time bad understanding) of the step 
mechanism inside the OperatingRound I broke the 1856 loan mechanism.
And I was not even aware of that code as this code was part of a subclass of 
OperatingRound.

The same issue about the size of classes effects writing unit tests: For 
refactoring the best is to have the class covered fully by unit tests. If you 
have two small classes instead of one large you can start refactoring earlier 
than you had to write all tests first.

And the other good thing is, that with smaller classes it is less likely that 
two people work at the same time on the same class, thus it makes merging and 
potential merge conflicts less likely.

For my daily job I have to deal with large programs in non-object orientated 
languages and even there I prefer to split problems into smaller, independent 
pieces. Especially if you come back to those programs several years later, it 
makes it much easier to enhance or fix something. 

OK enough text, but it is not about fashionable or good/bad, it is simply
about making coding easier and so increase the fun part of it.

Stefan

´

Re: [Rails-devel] Current implementation, branch Rails 2.0?

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

> -----Original Message-----
> From: Erik Vos [mailto:eri...@xs...]
> As I proposed before, I can reorganize OperatingRound to group the
> methods by function (OR step), each with some descriptive header.
> That's all I can do on short term. It might make OR easier to read, and
> perhaps it could be a starter for later refactoring.

Done.

Re: [Rails-devel] Current implementation, branch Rails 2.0?

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

Hi Stefan,

I agree with 100% of what you are saying, but there is one implicit
presumption: that we can avoid side effects.  And that isn't generally true.

Take your alternative proposal to handle phase actions (or triggers - I
don't like that word either, but that aside).  
Now the Phase tries to reach the action taker by calling a stub, that call
is passed on and must in some Round subclass be implemented with actual
code.
You propose a register-and-call-back approach, which is fine, if only the
action taker can access the registration point. Getting access is the
central problem; read on.

There is quite some history before I arrived at the current architecture. 
Originally I tried to follow the let-each-object-mind-its-own-business
approach (following the original OO paradigm), which led to a situation
where many different objects (portfolios, public companies etc.) were
executing various parts of particular action types.
This worked in simple cases, but as more features got implemented, more
side-effects cropped up, where code in various objects needed to call
methods in different, sometimes remote objects.  So the question arose how
an object can get access to another object to which it doesn't have a direct
reference.

Initially this was often done via static methods calls.  But I had to
abandon that approach as I realised that it would inhibit foreseen future
developments.

Mainly in the last two years I found myself following two paths:
1. Move code out of the objects into the Round classes, which have easier
access to other objects via the GameManager.  You will not believe it, but
in my perception these moves have much simplified and clarified the affected
code parts.
2. Pass down references to the various managers (including GameManager),
usually via the ubiquitous finishConfiguration methods, which are
specifically intended to create any required relationships after all XML has
been parsed.

There transitions aren't complete, but now that we have most OR code in the
OR class, it has become much easier to add new or override existing
behaviour.  And about every object is now reachable from there.

The situation has become much more complex with the advent of
rounds-within-rounds in 1856, 1835 and 18EU.  In 1835, Prussian formation
rounds even run inside company turns.  It took a while before I had arrived
at a workable approach to make this happen.

Now I'm facing your proposal to fragment the OR class into an untold number
of separate classes.  Your example of the correction classes is instructive,
and I regret that I have not looked at and learned from your code in an
earlier stage. But correction is a very simple case, out of the game as it
were, with just one item being affected without side effects.  Fine.  But
I'm far from convinced that it will look equally nice if we are going to
apply this concept to real game actions, with all required side effects, and
that it really will simplify rather than even more complicate the code.

You certainly have set my thoughts in motion, and I'm open to be convinced
by more realistic examples.  But I hope you'll understand that I have no
great desire to set myself to start reworking these central concepts once
again.

Erik.

> -----Original Message-----
> From: Stefan Frey [mailto:ste...@we...]
> Sent: Thursday, August 11, 2011 10:38 PM
> To: Development list for Rails: an 18xx game
> Subject: Re: [Rails-devel] Current implementation, branch Rails 2.0?
> 
> > All fine, but so what?  Is size bad per se?
> >
> 
> Erik,
> No certainly not. All this is not about good or bad. In one sense the size
> shows the thoughts and efforts for all details you have put into those
classes.
> 
> It is the other way around: As I do not have the time and the memory to
> understand all thoughts and conditions you considered, that created that
> code, I would prefer to digest all that in little portions.
> 
> Organizing code into classes allows to review only the relevant classes
that
> you need know. It is like organizing stuff into little boxes where you put
> labels on the boxes what is inside. This makes it much easier to find
things
> and to understand the big picture: Maybe like in a big department store:
It is
> much easier to get an overview by studying the directory of the store
instead
> of searching every shelf.
> 
> An object orientated languages allows storing pieces of code in classes
and if
> I want rewrite/change/add something to a class, I have to make sure that
> after my changes everything works as before. If the class is small, it is
easier
> to check if I do not introduce side effects and I only have to test the
part
> where the class is responsible for: So take the example for the
> OperatingRound. If I change anything inside I have to test all
functionality
> which is managed by the OperatingRound. Thus I change something related
> to token lay, it can potentially have side effects on tile laying, revenue
> payouts, train buying, loans etc.
> 
> If the token lay code is inside in a separate class it is impossible to
effect the
> other functionality of OperatingRound.
> 
> I had the issue more than a year ago: I started adding the NoMap
> functionalities which in principle should only change Tile and Token Lay.
> However due to side effects of my (at that time bad understanding) of the
> step mechanism inside the OperatingRound I broke the 1856 loan
> mechanism.
> And I was not even aware of that code as this code was part of a subclass
of
> OperatingRound.
> 
> The same issue about the size of classes effects writing unit tests: For
> refactoring the best is to have the class covered fully by unit tests. If
you
> have two small classes instead of one large you can start refactoring
earlier
> than you had to write all tests first.
> 
> And the other good thing is, that with smaller classes it is less likely
that two
> people work at the same time on the same class, thus it makes merging and
> potential merge conflicts less likely.
> 
> For my daily job I have to deal with large programs in non-object
orientated
> languages and even there I prefer to split problems into smaller,
> independent pieces. Especially if you come back to those programs several
> years later, it makes it much easier to enhance or fix something.
> 
> OK enough text, but it is not about fashionable or good/bad, it is simply
> about making coding easier and so increase the fun part of it.
> 
> Stefan
> 
> ´
> 
>
----------------------------------------------------------------------------
--
> Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user
> administration capabilities and model configuration. Take the hassle out
of
> deploying and managing Subversion and the tools developers use with it.
> http://p.sf.net/sfu/wandisco-dev2dev
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel

Re: [Rails-devel] Current implementation, branch Rails 2.0?

[Stefan F. <ste...@we...>]

Erik:
just a comment on your thoughts and a short summary of my current plans:

I understand that the issue how various classes can interact with each other 
has to be solved and needs consideration. 

However the solution to put everything very close together is similar to solve 
such problems by making all variables global.

I am currently sketching/designing a proposal/possibility to break up the 
round classes and still allowing classes to interact in a disciplined way:

My solution will be to store such information in an abstraction called 
context, which allows to store objects that implement a contextItem interface. 
That includes for example information about the current phase, the active 
player, the operating company, the available colors etc.

I will split the round classes into two separate parts: First a replacement 
for the round mechanism, that defines the progression between players (for 
share rounds) or companies (for operating rounds).

And then the sub-phases inside a round will all get separate classes called 
activities. So there will be a LayTileActivity, a LayTokenActivity, a 
BuyTrainActivity etc. for the OperatingRound. They provide and execute actions 
similar to the Correction managers.

All of this should be configurable using a Round.xml file, which will allow to 
define not only the activities used, but also the sequence of those. For this 
all activities will get a slot that allows to assign ActivationStrategies to 
decouple the activity  (especially for those activities that only occur after 
specific events etc.)

Finally the RoundManager asks the current active Round which Activities are 
there, checks if they are active currently by calling their 
ActivationStrategies, allows the Round classes to add additional information 
to the current context which is then passed to the active Activities. Based on 
the context the activities create the actions. The RoundManager collects the 
actions and forwards them to the UI. Given the selected action the 
RoundManager then returns it to the according Activity class object.

I will hope to come up with a prototype implementation of the new framework 
soon, as working code is usually more convincing than lots of words.

Stefan


On Friday, August 12, 2011 05:21:25 pm Erik Vos wrote:
> Hi Stefan,
> 
> I agree with 100% of what you are saying, but there is one implicit
> presumption: that we can avoid side effects.  And that isn't generally
> true.
> 
> Take your alternative proposal to handle phase actions (or triggers - I
> don't like that word either, but that aside).
> Now the Phase tries to reach the action taker by calling a stub, that call
> is passed on and must in some Round subclass be implemented with actual
> code.
> You propose a register-and-call-back approach, which is fine, if only the
> action taker can access the registration point. Getting access is the
> central problem; read on.
> 
> There is quite some history before I arrived at the current architecture.
> Originally I tried to follow the let-each-object-mind-its-own-business
> approach (following the original OO paradigm), which led to a situation
> where many different objects (portfolios, public companies etc.) were
> executing various parts of particular action types.
> This worked in simple cases, but as more features got implemented, more
> side-effects cropped up, where code in various objects needed to call
> methods in different, sometimes remote objects.  So the question arose how
> an object can get access to another object to which it doesn't have a
> direct reference.
> 
> Initially this was often done via static methods calls.  But I had to
> abandon that approach as I realised that it would inhibit foreseen future
> developments.
> 
> Mainly in the last two years I found myself following two paths:
> 1. Move code out of the objects into the Round classes, which have easier
> access to other objects via the GameManager.  You will not believe it, but
> in my perception these moves have much simplified and clarified the
> affected code parts.
> 2. Pass down references to the various managers (including GameManager),
> usually via the ubiquitous finishConfiguration methods, which are
> specifically intended to create any required relationships after all XML
> has been parsed.
> 
> There transitions aren't complete, but now that we have most OR code in the
> OR class, it has become much easier to add new or override existing
> behaviour.  And about every object is now reachable from there.
> 
> The situation has become much more complex with the advent of
> rounds-within-rounds in 1856, 1835 and 18EU.  In 1835, Prussian formation
> rounds even run inside company turns.  It took a while before I had arrived
> at a workable approach to make this happen.
> 
> Now I'm facing your proposal to fragment the OR class into an untold number
> of separate classes.  Your example of the correction classes is
> instructive, and I regret that I have not looked at and learned from your
> code in an earlier stage. But correction is a very simple case, out of the
> game as it were, with just one item being affected without side effects. 
> Fine.  But I'm far from convinced that it will look equally nice if we are
> going to apply this concept to real game actions, with all required side
> effects, and that it really will simplify rather than even more complicate
> the code.
> 
> You certainly have set my thoughts in motion, and I'm open to be convinced
> by more realistic examples.  But I hope you'll understand that I have no
> great desire to set myself to start reworking these central concepts once
> again.
> 
> Erik.
> 
> > -----Original Message-----
> > From: Stefan Frey [mailto:ste...@we...]
> > Sent: Thursday, August 11, 2011 10:38 PM
> > To: Development list for Rails: an 18xx game
> > Subject: Re: [Rails-devel] Current implementation, branch Rails 2.0?
> > 
> > > All fine, but so what?  Is size bad per se?
> > 
> > Erik,
> > No certainly not. All this is not about good or bad. In one sense the
> > size shows the thoughts and efforts for all details you have put into
> > those
> 
> classes.
> 
> > It is the other way around: As I do not have the time and the memory to
> > understand all thoughts and conditions you considered, that created that
> > code, I would prefer to digest all that in little portions.
> > 
> > Organizing code into classes allows to review only the relevant classes
> 
> that
> 
> > you need know. It is like organizing stuff into little boxes where you
> > put labels on the boxes what is inside. This makes it much easier to
> > find
> 
> things
> 
> > and to understand the big picture: Maybe like in a big department store:
> It is
> 
> > much easier to get an overview by studying the directory of the store
> 
> instead
> 
> > of searching every shelf.
> > 
> > An object orientated languages allows storing pieces of code in classes
> 
> and if
> 
> > I want rewrite/change/add something to a class, I have to make sure that
> > after my changes everything works as before. If the class is small, it is
> 
> easier
> 
> > to check if I do not introduce side effects and I only have to test the
> 
> part
> 
> > where the class is responsible for: So take the example for the
> > OperatingRound. If I change anything inside I have to test all
> 
> functionality
> 
> > which is managed by the OperatingRound. Thus I change something related
> > to token lay, it can potentially have side effects on tile laying,
> > revenue payouts, train buying, loans etc.
> > 
> > If the token lay code is inside in a separate class it is impossible to
> 
> effect the
> 
> > other functionality of OperatingRound.
> > 
> > I had the issue more than a year ago: I started adding the NoMap
> > functionalities which in principle should only change Tile and Token Lay.
> > However due to side effects of my (at that time bad understanding) of the
> > step mechanism inside the OperatingRound I broke the 1856 loan
> > mechanism.
> > And I was not even aware of that code as this code was part of a subclass
> 
> of
> 
> > OperatingRound.
> > 
> > The same issue about the size of classes effects writing unit tests: For
> > refactoring the best is to have the class covered fully by unit tests. If
> 
> you
> 
> > have two small classes instead of one large you can start refactoring
> 
> earlier
> 
> > than you had to write all tests first.
> > 
> > And the other good thing is, that with smaller classes it is less likely
> 
> that two
> 
> > people work at the same time on the same class, thus it makes merging and
> > potential merge conflicts less likely.
> > 
> > For my daily job I have to deal with large programs in non-object
> 
> orientated
> 
> > languages and even there I prefer to split problems into smaller,
> > independent pieces. Especially if you come back to those programs several
> > years later, it makes it much easier to enhance or fix something.
> > 
> > OK enough text, but it is not about fashionable or good/bad, it is simply
> > about making coding easier and so increase the fun part of it.
> > 
> > Stefan
> > 
> > ´
> 
> ---------------------------------------------------------------------------
> - --
> 
> > Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user
> > administration capabilities and model configuration. Take the hassle out
> 
> of
> 
> > deploying and managing Subversion and the tools developers use with it.
> > http://p.sf.net/sfu/wandisco-dev2dev
> > _______________________________________________
> > Rails-devel mailing list
> > Rai...@li...
> > https://lists.sourceforge.net/lists/listinfo/rails-devel
> 
> ---------------------------------------------------------------------------
> --- FREE DOWNLOAD - uberSVN with Social Coding for Subversion.
> Subversion made easy with a complete admin console. Easy
> to use, easy to manage, easy to install, easy to extend.
> Get a Free download of the new open ALM Subversion platform now.
> http://p.sf.net/sfu/wandisco-dev2dev
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel