I have posted the class HexGridNumberingY, and the test classHexGridNumberingYTest, under the branch pgeerkens/…/mapgrid. These are a workable, though only partially tested, implementation of a unifed algorithm for rectangular and oblique coordinates on a hexgrid. It is a complete replacement for HexGridNumbering, extending RegularGridNumbering at the moment. I am now going to think about how to setup interfaces to support better integration, first by returning to some of Joel’s earlier posts on this thread.
Pieter
Here are initial thoughts on a class structure for the rewrite. I have concentrated mostly on hex grids, my area of proficiency, so any comments in particualr on how this affects non-hexagonal tilings, and zoned and regional grids, would be most welcome.
I have adopted Tile and Tiling as replacements for Grid to avoid name conflicts.
ITiling
- the basic interface shared by all tilings of the playing area.
AbstractTiling extends AbstractConfigurable implements ITiling
- abstract implementation of common functionality in ITiling.
ITileNumbering
- the basic interface shared by all numberings of a tiled playing area.
AbstractTileNumbering extends AbstractTiling implements ITileNumbering
- abstract implementation of common functionality in ITileNumbering.
AbstractHexTileNumbering extends AbstractTileNumbering implements IHexTileNumbering
- abstract implementation of functionality specific to hex tilings, but implementation independent.
HexTileNumbering extends AbstractHexTileNumbering
- full implementation of the class
Pieter
Thus spake pgeerkens:
Here are initial thoughts on a class structure for the rewrite. I have
concentrated mostly on hex grids, my area of proficiency, so any
comments in particualr on how this affects non-hexagonal tilings, and
zoned and regional grids, would be most welcome.
I have adopted __Tile __and Tiling as replacements for Grid to
avoid name conflicts.
It’s not necesary to avoid Grid in names, as I’d like to put new code
under org.vassalengine hierarchy insted of under VASSAL. There won’t
be any conflicts that way.
Also, there will be lots of classes in 3.2 with Tile in their names
due to the image tiling project, so sticking with Grid is preferable.
ITiling
- the basic interface shared by all tilings of the playing area.
AbstractTiling extends AbstractConfigurable implements ITiling
- abstract implementation of common functionality in ITiling.
ITileNumbering
- the basic interface shared by all numberings of a tiled playing
area.
AbstractTileNumbering extends AbstractTiling implements ITileNumbering
- abstract implementation of common functionality in ITileNumbering.
AbstractHexTileNumbering extends AbstractTileNumbering implements
IHexTileNumbering
- abstract implementation of functionality specific to hex tilings,
but implementation independent.
HexTileNumbering extends AbstractHexTileNumbering
- full implementation of the class
-
Don’t preface interfaces with “I”. That makes code harder to read. Most
typenames we’re dealing with should be interfaces anyway, except for
where new objects are being created. (The fact that it’s not this way
presently is something I hope to correct.
-
What common functionality do you expect will be in
AbstractHexTileNumbering? What other subclasses might it have besides
HexTileNumbering?
-
Regarding model-view separation: These classes look entirely on the model
side to me. There will need to be a view interface corresponding to ITiling
so that we can attach views to an ITiling.
–
J.
OK for most. I am still experimenting with how to decorate a HexGrid from within itès numbering, so that the construction of AffineTransforms dependent only on grid properties can be constructed on each change of Grid Properties. It turns out that almost the entire process of converting points to grid indices can be driven by a couple of affine transforsm ( You knew that, but now you know that I do too.). However it is a shame to construct those transforms on every call to get a grid location.
I like to preface interfaces with “I” because that way I don’t consume good names so quickly.
Thus spake pgeerkens:
OK for most. I am still experimenting with how to decorate a HexGrid
from within itès numbering, so that the construction of
AffineTransforms dependent only on grid properties can be constructed on
each change of Grid Properties. It turns out that almost the entire
process of converting points to grid indices can be driven by a couple
of affine transforsm ( You knew that, but now you know that I do too.).
However it is a shame to construct those transforms on every call to get
a grid location.
You should store those matrices as class members.
I like to preface interfaces with “I” because that way I don’t consume
good names so quickly.
This is one point on which I will insist: Labeling interfaces this way
harms readability and is inconsistent with the rest of our codebase.
If you need help coming up with good names, ask me, I’ll be glad to think
of some.
–
J.
Thus spake pgeerkens:
1)in progress.
- no need to insist - I was merely stating a preference rather than a
refusal to cooperate. I may continue to use the convention for work in
progress, while I refine naming choices, but will comply with team
standards for all ready-for-prime-time code.
Great. Much appreciated.
I’m a bit concerned by how monolithic the classes you’re writing are—
that’s going to make it very hard to write unit tests.
–
J.
The big, clunky and monolithic is more a function of where I started from, than on where I will end up. As I am in new territory here on a few counts, I am progressing slowly and carefully to avoif losing my way.
Any and all comments, particularly on the philosophy of using inclosed and sub-classes is appreciated.
Another interim check-in coming shortly.
Thus spake pgeerkens:
The big, clunky and monolithic is more a function of where I started
from, than on where I will end up. As I am in new territory here on a
few counts, I am progressing slowly and carefully to avoif losing my
way.
Any and all comments, particularly on the philosophy of using inclosed
and sub-classes is appreciated.
I’m a bit time-constrained at the moment, which is unfortunate, because
I’d like to be more involved in the design process than I’m able to be.
There are three important things which I see here:
-
Inversion of control will help with writing smaller classes, and make
what you write more testable.
-
Unit tests. I’ll try to show you an example of what I’m looking for
here soon.
-
I’ve referred many times to things which this code will need to
accomodate or interact with, but haven’t shown you what those things
look like. In particular, the whole properties system is slated to
change in 3.3, for which some of the utility code is written but I
haven’t had time to document it yet, and at the same time the model-
view separation project will take all of the GUI code out of these
classes and put it behind some view interfaces in its own hiearchy.
This is frustrating for me, because it means that I can’t provide
more precise guidance without stopping the tiling work for 3.2, but
I also don’t want to cause you to waste your time writing a lot of
code which will need to be changed not far down the line; I’m finding
it hard to strike a balance between the two.
–
J.
Thus spake pgeerkens:
- I will google search “inversion of control” to be sure that I
understand what you mean. First impression is use of callback functions,
but I will verify.
No, that’s not quite what I mean.
Basically you have these small classes which conform to interfaces, and you
pass instances of them in to the larger classes at construction time. This
kind of design lets you build things in layers. You can verify that the
classes in the innermost layer work independently of anything else, and then
test subsequent layers using stubs and mocks.
An example of this is the code in VASSAL.tools.signals on my
uckleman-working branch, both under src and test.
- No rush. I have some background here, and have some thoughts myself
once I get my classes down a bit in size.
Once you’ve defined an API, writing tests before you write the application
code can be quite helpful.
- My take for the moment is to break classs down in to a pair: an
abstract super-class with the GUI and properties handling, extended by a
concrete class with the real guts. This is most likely not your ultimate
mechanism, but it will separate functionality and make the port easier.
Ok. I can guarantee you that the GUI code and properties handling will
ultimately not reside in the same class, as that’s the goal of the
model-view project, but what you’re doing shouldn’t make it any harder to
pull apart later.
–
J.
Not done yet, but looking much cleaner. Still thinking about how to best apply IoC.
First try at using IoC seems to work reasonalby, and decouples code nicely IMHO. Any comments or naming suggestions you might have are welcome. I broke the calculation of MaxOblique et al somehow adding the PropertyCHangeListeners, so I am tracking that down now.
Joel,
This is ready for review. I would really appreciate your comments on the design as well as the implementation.
Hi Pieter,
Are you still working on HexGridNumberingX. I have checked out your latest code and have been trying to apply it to a map for a new module I am working on that has an Oblique grid combination I have not seen before. It’s essentially a sideways hex grid with a regular grid numbering (offset hexes) that has been rotated 60 degrees anticlockwise.
I have managed to build a single-case version of HexGridNumbering for it, but I was interested to see if I could get HexGridNumberingX to do the job. I am having trouble getting the settings right for this map. Would you like to have a look? The map is available from
home.exetel.com.au/swampwallaby/ … scen-2.jpg
I know the printed grid itself is twisted (painful!).
Thanks,
Brent.
That’s the wierdest numbering I can imagine. Let me see what I can do.
Pieter
I have figured out why the grid looks so weird, and what I need to do to implement it. Now I just have to figure out why Eclipse cannot oopen my Java VM.
Hi Brent,
Whatever Eclipse’s problem was, upgrading to the latest release solved it.
This numbering scheme has the two co-ordinat axes at 30 degrees, which previous UI’s had not considered. I believe the best solution is to define a UI class which allow direct input of the affine-transformation, thus allowing any numbering scheme to be handled (in extremis). I have this in progress now.
Pieter
I have added a new class HexGridNumberingIrregular which takes as parameters direct input of the m00, m01, m10, m11 indices of the AffineTransform from grid-canonical hex indices to world hex indices. (The m20 and m21 matrix indices are already visible as the starting horizontal and vertical indices.) To obtain the grid numbering printed on the map the required parameter settings are thus:
M00 = -1.0 m01 = 1.0
M10 = 0.5 m11 = 0.5
Starting horizontal = 72
Starting vertical = 145
Once I generalized the underlying algorithm, and then tidied up the code, this new class is almost identical to HexGridNumberingX - I will ponder on abstracting this commonality into a separate class, but in the meantime I have checked in the code.
Joel:
If you read this, I am still unsure how I go about merging this code into the trunk. Can you point me at some guidelines for this?
Thank you.
Thus spake pgeerkens:
I have added a new class HexGridNumberingIrregular which takes as
parameters direct input of the m00, m01, m10, m11 indices of the
AffineTransform from grid-canonical hex indices to world hex indices.
(The m20 and m21 matrix indices are already visible as the starting
horizontal and vertical indices.) To obtain the grid numbering printed
on the map the required parameter settings are thus:
M00 = -1.0 m01 = 1.0
M10 = 0.5 m11 = 0.5
Starting horizontal = 72
Starting vertical = 145
Once I generalized the underlying algorithm, and then tidied up the
code, this new class is almost identical to HexGridNumberingX - I will
ponder on abstracting this commonality into a separate class, but in the
meantime I have checked in the code.
Joel:
If you read this, I am still unsure how I go about merging this code
into the trunk. Can you point me at some guidelines for this?
I’ve been doing the merging to trunk myself, as a way of doing code
review. Can you give me a list of commits I need to merge?
–
J.
