Re: [Rails-devel] Some (multi-)undo related bugs

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

Erik:
other  implementation details follow here as they arose from to the topic in 
the subject:

I have added two new state classes: ArrayListState and HashMapState, that 
create ModelObjects, which use the already defined Move classes for changes of 
ArrayLists and HashMaps. It basically wraps the ModelObject/State around 
standard ArrayLists and HashMaps and uses the standard method names to 
manipulate them. In principle it would be possible to fully implement the 
Map/List interfaces, but I was too lazy to write all required methods for 
that.

My reason for adding those is that at least for the approach above protects 
against accidentally using the non-move methods on list or maps, which are in 
fact stateful and thus require the moves instead. And I still prefer using the 
standard syntax stateList.add(object) instead of new AddToList("NameOfList", 
stateList, object). And it allows to create more complicated operations using 
the atomic moves (for example addAll or initFromMap). It is possible to 
retrieve the wrapped list/map as an unmodifiable view.

This allowed fixing the opertingCompanies bug in 1856 and the tilesPerColour 
problem in 18EU.

Currently they do not inherit from neither ModelObject/State as the current 
usage has not required them to do so. But it is possible to do so, but I would 
prefer to inherit from ModelObject directly. 

If you do not mind I would like to add an according HashSetState class and  
replace BooleanState, IntegerState and StringState with more specialized 
versions that inherit from ModelObject instead of State and have according 
simple Moves (BooleanChange, IntegerChange, StringChange) and it would allow 
to introduce an easier syntax, for example for boolean: booleanVar.set(true) 
or booleanVar.set(false) and booleanVar.is() or booleanVar.isNot()

This would also allow to deprecate (and remove later) the State class,as there 
is already a replacement available with GenericState. All those changes make
the StateI interface unnecessary. It could still be used as a mere marker 
interface or have the getName() function at most.
Stefan

On Monday, August 16, 2010 10:48:07 pm Erik Vos wrote:
> Stefan,
> 
> I'll come back to your remarks later this week. I do have thoughts on these
> matters, but no time right now.
> 
> Erik.
> 
> -----Original Message-----
> From: Stefan Frey [mailto:ste...@we...]
> Sent: Monday 16 August 2010 21:46
> To: 'Development list for Rails: an 18xx game'
> Subject: [Rails-devel] Some (multi-)undo related bugs
> 
> A rather longish discussion of implementation details to follow. Mainly
> information for Erik.
> Stefan
> 
> A side-effect of the new game history is that it allows to jump around
> between
> very different game states, switching round types and phases on the fly.
> This acts as a advanced test on the undo mechanism and overall it works
> very
> 
> well. However there are still some bugs to discover, which might also act
> as
> 
> examples to avoid confusing the undo/redo mechanism:
> 
> * The toolTip on hexes was not updated correctly after undo/redos (still
> showing the tile information from the previous state), as the update was
> controlled only by the tile/token lay UI classes and not the game backend.
> I shortly considered a ToolTipModel approach, but I realized that is much
> easier to update the toolTip at the time it is requested by the user.
> I would still like to move the code to create the toolTip string from the
> UI
> 
> GUIHex class to the game MapHex class to reduce the amount of processing
> and
> 
> calls to the game classes from the UI class.
> Or is there a reason to avoid that, Erik?
> It seems to me that now all UI elements update correctly after undo (I
> recently added a PresidentModel to update the president column).
> 
> * A more subtle problem arose from a 18EU test: After going back from a
> major
> turns to a minor ones during the green phase, the minor was suddenly
> allowed
> 
> to lay a green tile.
> This occured as the tileLayPerColour state change triggers a call to the
> operatingCompany  variable inside the OperatingRound instance. This
> variable
> 
> itself however is not a state variable itself, but gets only initialized
> from
> the state variable operatingCompanyObject at the begin of the companies'
> action generation. Thus during the undo Rails assume that the
> tileLayPerColour
> request is still for the major company, as the operatingRound is not
> updated.
> 
> I admit that I have introduced the operatingCompanyObject myself and did
> not
> 
> think about this problem at that time: The lesson to learn here is to
> strongly
> avoid working with non-state variables, which simply contain a reference to
> a
> state variable. This is an wide open door for bugs & troubles a long time
> later.
> Either use the state variable directly or encapsulate it to a method call,
> if
> the state mechanism seems to unwieldy.
> 
> ---------------------------------------------------------------------------
> - --
> This SF.net email is sponsored by
> 
> Make an app they can't live without
> Enter the BlackBerry Developer Challenge
> http://p.sf.net/sfu/RIM-dev2dev
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel
> 
> 
> ---------------------------------------------------------------------------
> --- This SF.net email is sponsored by
> 
> Make an app they can't live without
> Enter the BlackBerry Developer Challenge
> http://p.sf.net/sfu/RIM-dev2dev
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel