From: Stefan F. <ste...@we...> - 2014-02-18 11:19:57
|
Martin & Alexander & Erik: I did a complete rewrite of the PlayerManager to include 1880 requirements that allows a change of player order. One issue I have to solve is the following: In some cases of Rails players are identified by index and not by name. However index is both a property of the Player (thus static) and PlayerManager (thus for 1880 dynamic). I propose to use the player Names as identifiers for players instead of the index. Actually they are used as the official id inside the Rails players objects in 2.0 already. May intention is to drop index altogether and rely only on name. I have still to identify which old existing code refers to what (player order or player index identity) and then replace that accordingly. All methods in PlayerManager that deal with the index are set to deprecated to indicate that. However they will be removed at the end of this process. Fortunately the PossibleAction class already writes both index and name to the save files, so we should be able to keep the existing save files without conversion and writing support code for this. Erik: Do you have any other idea or comment, what we should consider here? Please review my changes in rails_2_develop_1880. The code compiles, however does not run so far, until the index issue is solved. Stefan Some further details on the rewrite: This is a good example how I best-practice structure of elements in 1880 looks like: * If several states belong to a common structure create a Model (here PlayerOrderModel) and have all of them contained inside. This is the only place where I use some "magic", which I usually do not like, as it might surprise some. Models get automatically subscribed as models to states where they are parents to. So you do need to add the Model to the state using state.addModel(model). Usually it is the case for states that are defined as fields inside of models and you avoid to have state.addModel(this) for all states. Using this mechanism the model is always updated if any of the contained states are updated and all observers of the model get updated as well. However you are free to bind the Model to any other state (or other model). And yes: I use a topological sort to figure out which model has to be updated if the model graph gets more complex. It detects and reports cycles to the log-file ;-) Currently the structure of Rails models is not that complex... * If the model is closely tied to a Rails class make the Model a static nested class of the Rails class. This strongly shows that this model should only be created from inside that class. If the constructor is made private, that will be the case. * Put most of the controlling and interacting code with the outside world inside the (main) Rail class. The model should mainly used to store the states and allow interaction with the states. And it is the outside than shown to the UI client. This is still be done here. * And there are a lot of nice features in the Google Guava library to write concise and fluent code. Take a look if you think you are missing some nice feature from more modern languages. |
From: Erik V. <eri...@xs...> - 2014-02-18 12:33:43
|
Stefan, The main reason to have an index was to identify the "next player", where applicable, being (currentPlayerIndex + 1) mod numberOfPlayers. I suppose you'll need to have (references to) the player names in some (possibly circular) linked list to keep this behaviour without an explicit index. Erik > -----Original Message----- > From: Stefan Frey [mailto:ste...@we...] > Sent: Tuesday, February 18, 2014 12:20 PM > To: Development list for Rails: an 18xx game > Subject: [Rails-devel] PlayerManager rewrite for 1880 > > Martin & Alexander & Erik: > I did a complete rewrite of the PlayerManager to include 1880 requirements > that allows a change of player order. > > One issue I have to solve is the following: > In some cases of Rails players are identified by index and not by name. > However index is both a property of the Player (thus static) and > PlayerManager (thus for 1880 dynamic). > > I propose to use the player Names as identifiers for players instead of the > index. Actually they are used as the official id inside the Rails players objects > in 2.0 already. > > May intention is to drop index altogether and rely only on name. > > I have still to identify which old existing code refers to what (player order or > player index identity) and then replace that accordingly. > > All methods in PlayerManager that deal with the index are set to deprecated > to indicate that. However they will be removed at the end of this process. > > Fortunately the PossibleAction class already writes both index and name to > the save files, so we should be able to keep the existing save files without > conversion and writing support code for this. > > Erik: Do you have any other idea or comment, what we should consider > here? > > Please review my changes in rails_2_develop_1880. The code compiles, > however does not run so far, until the index issue is solved. > > Stefan > > > Some further details on the rewrite: > > This is a good example how I best-practice structure of elements in 1880 > looks like: > > * If several states belong to a common structure create a Model (here > PlayerOrderModel) and have all of them contained inside. > > This is the only place where I use some "magic", which I usually do not like, as > it might surprise some. Models get automatically subscribed as models to > states where they are parents to. > > So you do need to add the Model to the state using state.addModel(model). > Usually it is the case for states that are defined as fields inside of models and > you avoid to have state.addModel(this) for all states. > > Using this mechanism the model is always updated if any of the contained > states are updated and all observers of the model get updated as well. > However you are free to bind the Model to any other state (or other model). > > And yes: I use a topological sort to figure out which model has to be updated > if the model graph gets more complex. It detects and reports cycles to the > log-file ;-) Currently the structure of Rails models is not that complex... > > * If the model is closely tied to a Rails class make the Model a static nested > class of the Rails class. > > This strongly shows that this model should only be created from inside that > class. If the constructor is made private, that will be the case. > > * Put most of the controlling and interacting code with the outside world > inside the (main) Rail class. The model should mainly used to store the states > and allow interaction with the states. And it is the outside than shown to the > UI client. > > This is still be done here. > > * And there are a lot of nice features in the Google Guava library to write > concise and fluent code. Take a look if you think you are missing some nice > feature from more modern languages. > > > ---------------------------------------------------------------------------- -- > Managing the Performance of Cloud-Based Applications Take advantage of > what the Cloud has to offer - Avoid Common Pitfalls. > Read the Whitepaper. > http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.cl > ktrk > _______________________________________________ > Rails-devel mailing list > Rai...@li... > https://lists.sourceforge.net/lists/listinfo/rails-devel |
From: Stefan F. <ste...@we...> - 2014-02-18 14:19:09
|
Erik, sure I understand that reason: That was the reason why the PlayerOrderModel includes the method getNextPlayer(). However internally the order is simply stored by an ArrayListState and I use exactly the formula as you. However the order stored inside the model is used for that, not the index numbers of the players. Otherwise index numbers have to be state variables and that would cause havoc to the reload process. This change will make player names the id's for Player objects and the "index" is only derived from the (current) player order, so it is not suitable for identification. Best way to refer in code to a Player is to use the Player object directly. Only for storage or UI purposes it is converted to the id. I have kept the names to players map that already existed to keep the existing getPlayerForName(String name) method. All others that relate to indices will be removed, as they are potentially dangerous. There is a next player method in PlayerManager and I will add playerInOrder(Player after) method that will return a List<Player> that lists all players in order "after" the Player argument. This is a quite often replicated functionality. Thanks for your clarification, Stefan On 02/18/2014 01:33 PM, Erik Vos wrote: > Stefan, > > The main reason to have an index was to identify the "next player", where > applicable, being (currentPlayerIndex + 1) mod numberOfPlayers. > I suppose you'll need to have (references to) the player names in some > (possibly circular) linked list to keep this behaviour without an explicit > index. > > Erik > >> -----Original Message----- >> From: Stefan Frey [mailto:ste...@we...] >> Sent: Tuesday, February 18, 2014 12:20 PM >> To: Development list for Rails: an 18xx game >> Subject: [Rails-devel] PlayerManager rewrite for 1880 >> >> Martin & Alexander & Erik: >> I did a complete rewrite of the PlayerManager to include 1880 requirements >> that allows a change of player order. >> >> One issue I have to solve is the following: >> In some cases of Rails players are identified by index and not by name. >> However index is both a property of the Player (thus static) and >> PlayerManager (thus for 1880 dynamic). >> >> I propose to use the player Names as identifiers for players instead of > the >> index. Actually they are used as the official id inside the Rails players > objects >> in 2.0 already. >> >> May intention is to drop index altogether and rely only on name. >> >> I have still to identify which old existing code refers to what (player > order or >> player index identity) and then replace that accordingly. >> >> All methods in PlayerManager that deal with the index are set to > deprecated >> to indicate that. However they will be removed at the end of this process. >> >> Fortunately the PossibleAction class already writes both index and name to >> the save files, so we should be able to keep the existing save files > without >> conversion and writing support code for this. >> >> Erik: Do you have any other idea or comment, what we should consider >> here? >> >> Please review my changes in rails_2_develop_1880. The code compiles, >> however does not run so far, until the index issue is solved. >> >> Stefan >> >> >> Some further details on the rewrite: >> >> This is a good example how I best-practice structure of elements in 1880 >> looks like: >> >> * If several states belong to a common structure create a Model (here >> PlayerOrderModel) and have all of them contained inside. >> >> This is the only place where I use some "magic", which I usually do not > like, as >> it might surprise some. Models get automatically subscribed as models to >> states where they are parents to. >> >> So you do need to add the Model to the state using state.addModel(model). >> Usually it is the case for states that are defined as fields inside of > models and >> you avoid to have state.addModel(this) for all states. >> >> Using this mechanism the model is always updated if any of the contained >> states are updated and all observers of the model get updated as well. >> However you are free to bind the Model to any other state (or other > model). >> >> And yes: I use a topological sort to figure out which model has to be > updated >> if the model graph gets more complex. It detects and reports cycles to the >> log-file ;-) Currently the structure of Rails models is not that > complex... >> >> * If the model is closely tied to a Rails class make the Model a static > nested >> class of the Rails class. >> >> This strongly shows that this model should only be created from inside > that >> class. If the constructor is made private, that will be the case. >> >> * Put most of the controlling and interacting code with the outside world >> inside the (main) Rail class. The model should mainly used to store the > states >> and allow interaction with the states. And it is the outside than shown to > the >> UI client. >> >> This is still be done here. >> >> * And there are a lot of nice features in the Google Guava library to > write >> concise and fluent code. Take a look if you think you are missing some > nice >> feature from more modern languages. >> >> >> > ---------------------------------------------------------------------------- > -- >> Managing the Performance of Cloud-Based Applications Take advantage of >> what the Cloud has to offer - Avoid Common Pitfalls. >> Read the Whitepaper. >> http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.cl >> ktrk >> _______________________________________________ >> Rails-devel mailing list >> Rai...@li... >> https://lists.sourceforge.net/lists/listinfo/rails-devel > > > ------------------------------------------------------------------------------ > Managing the Performance of Cloud-Based Applications > Take advantage of what the Cloud has to offer - Avoid Common Pitfalls. > Read the Whitepaper. > http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktrk > _______________________________________________ > Rails-devel mailing list > Rai...@li... > https://lists.sourceforge.net/lists/listinfo/rails-devel > |