1)in progress.
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.
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 rush. I have some background here, and have some thoughts myself once I get my classes down a bit in size.
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.
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 = 145Once 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.
Note to self:
Keep a list of commits for Joel.
Unfortunately no; I didn’t think of that. Is there a way to ask Subscript for all commits by me? I can confirm that I was inactive from about mid-January until last week, due to being overly busy on a new contract.
Thus spake pgeerkens:
Note to self:
Keep a list of commits for Joel.
This is only necessary while we’re still using Subversion, btw.
Once we switch over to Git, the thing to do would be to put all
work for one thing on its own branch, and just have me merge the
branch (or, tell me which commits to cherry-pick).
Unfortunately no; I didn’t think of that. Is there a way to ask
Subscript for all commits by me? I can confirm that I was inactive from
about mid-January until last week, due to being overly busy on a new
contract.
Did you make all of these commits on one branch? If so, I an find them
that way.
–
J.
EverythinG is on VASSAL-src/branches/pgeerkens. There are a couple of small independent changes not related to HexGridNumbering (such as the extension to SendToLocation), but those will be obvious from context (if not, just ask and I will assist)
Thus spake pgeerkens:
EverythinG is on VASSAL-src/branches/pgeerkens. There are a couple of
small independent changes not related to HexGridNumbering (such as the
extension to SendToLocation), but those will be obvious from context (if
not, just ask and I will assist)
Thanks. I’ll start in on them soon.
–
J.
I was looking through my sub-tree, and identifed two small enhancements unrelated to HexGridNumbering:
#7624: Implemented an AutoExec GlobalKeyCommand in AutoExecGKC and GameModule
#7625: Implemented SendToNamedLocation (instead of just grid coordinates) in SendToLocation.
The baseline should be v3_1_15 in both cases.
