I recently had to fix Replace.java after PlaceMarker.java had a new property added. The problem was that Replace.java added a property to the end of the type string and when a property was added to PlaceMarker, Replace.mySetType was no longer correct. It occurs to me that subclassing Decorators or Configurables is a bitch because of the way they are configured at module load time.
The solution is relatively straightforward. Descendant classes should include parent properties as a single string-encoded property in the buildfile with a delimeter like # or something. That way, if the parent adds a property, then the descendent doesn’t get totally messed up (if that descendent also defines its own properties). Old-style configuring have to be maintained.
I don’t mind tackling this, but the question is whether it’s too late for 3.1. I’ve got a bit of free time before New Year’s.
Right now we’re waiting on Rodney to look at Brent’s fixes to the server.
We’re not going to release beta7 (or 3.1.0) without those.
When you say that we’d have to maintain old-style configuring, what would
that amount to, exactly?
(Also, if you’re looking for something to work on, we need to hash out how
to start with the JOGL stuff. What I think needs doing first is to separate
out all of the model stuff in our classes from the view, in order that we
can swap one view out for another without disturbing the model. Do you agree?
If so, then we should start figuring out an interface for each class’ view.)
If you have the parent configuration as a property, the mySetType would look for “parentID;” after its own “childID;”. If it’s not there, it would fall back to the old-style processing which means cutting and pasting the superID on the current argument (like what it’s doing now). If it is there it would call super.mySetType on the next token instead (including “parentID;”). super.mySetType would eat up all the parent tokens and when the call returns, the child would eat up the remaining tokens defined in the subclass. The downside is that all the semicolons or whatever are escaped in the parent tokens, but on the upside, if the parent is redefined, it won’t break the children.
The way it works now is annoying.
Did I say I was looking for stuff to work on?
I do agree. We’re going to have to keep the current graphics context for those people where JOGL is just not going to work. Last I checked, it was a bit of a slog getting it to work on the few systems that were tested so we definitely need a fallback.
I think I understand what you’re talking about. Do you mean we need a view class for every current class that draws something? And then that view class has to be attached to the current class at creation time based on user preferences?
Definitely the way to go. Sounds like a butt-load of work just to refactor everything so that it works exactly the same… Do you want to share a branch?
Also, is there something on top of the model and view that coordinates them, or does the model just get a view attached to it? Is the model pulled out of the current classes, or do they stay the same except for the drawing part (and we call the current classes the models)?
I did a comp. sci. course once, but it was in Lisp… All I remember are closing parentheses.
All we have to do is paint directly onto the texture using Java2D calls. We may not have to pull out views from existing classes as the graphics calls can essentially be the same. The only purpose for pulling out view classes is if we wanted to do something quite different in the JOGL view.
M.
2008/12/27 Michael Kiefte <mkiefte@dal.ca (mkiefte@dal.ca)>
I want to pull out views anyway, as doing so will make it easier to write
tests for the model. We have enough code now that having unit tests could
help us a lot with eliminating bugs and preventing new bugs.
Also, having the drawing code completely separate from the data handling
code will help a lot with code clarity.
Hmm. Ok. I’d like it if Brent would put a word in here, as I’m uncertain
that I understand all of the consequence of this.
Yes. Views are like listeners for the model. The model shouldn’t know anything
about how (or whether) it’s being represented, it just says “Hey! I changed!”
and the views figure out what (if anything) to do with that information.
Another advantage of this is that we can safely call methods in the model
off the EDT and isolate all of the Swing calls to the views only, which
should cut threading bugs and make the GUI more responsive.
Yes. But even before that we should talk about what the interfaces should
look like.
I think that the current classes should be the model, with the view stuff
pulled out into separate classes—this is because the methods which are
view methods are fewer than the methods which are model methods, at least
in most classes.
Take BasicPiece as an example. We’d create an interface called, say,
BasicPieceListener, the purpose of which would be to listen for property
changes in the BasicPiece. I don’t know if we’d want to to use all of the
PropertyChangeListener/Support classes for this, or whether we’d want to
build our own. E.g.,
public interface BasicPieceListener {
public void locationChanged(Point old_loc, Point new_loc);
public void pieceNameChanged(String old_name, String new_name);
public void mapChanged(Map old_map, Map new_map);
…
}
(As an aside, this sort of thing could be useful elsewhere, not just for
building views.)
Then, a BasicPieceView would be a superinterface of BasicPieceListener,
along with a few extra methods:
Then, we have an AbstractViewFactory which generates views for each class:
public abstract class AbstractViewFactory {
public BasicPieceView createBasicPieceView();
public MarkerView createMarkerView();
public FreeRotatorView createFreeRotatorView();
…
}
and for each kind of rendering framework (e.g., classic Java2D, JOGL) we
have a subclass of AbstractViewFactory which produces objects implementing
the appropriate view classes:
public class JOGLViewFactory extends AbstractViewFactory {
…
}
What the objects are which implement the various view interfaces actually
are depends entirely on what the appropriate view factory returns. This
way, all of the rendering implementation is decoupled from data handling.
Some view implementations might be so simple as to have no state at all,
in which case we could use the same view for ever instance of the
corresponding model. (BasicPieceView might be like this.) I think for
BasicPiece, we’d move draw() into the view. I’m not sure what else should
be moved. I think maybe boundingBox() should be, as what the actual bounds
are depend on how the piece is drawn, and maybe also keyEvent().
That’s all I know aout Lisp, in fact, lots of parentheses.
I don’t have to do anything. It works right now. However, PlaceMarker.java has a field that only Replace.java ever uses. I find that annoying.
My first reaction was that this was convoluted and might discourage future developers. But I can’t think of a better way to do this as I’m thinking ahead. My son just asked me why you couldn’t tilt the board and my response was that the pieces weren’t actually standing up, but that they were flat on the map. He was disappointed…
So there’s two ways to go. I can subclass TextureRenderer so that it breaks up the image buffer into manageable textures and create a graphics context that updates the individual textures (I’m probably going to do that anyway as it makes life easier down the road). Or we can do it your way. Your way is longer, but probably more sensible. I think my way has to be done anyway and it’s relatively easy to do (all it does is put everything in the demo in a single class and makes it more dynamic).
But all the AI people were big into it so it instantly made you cool.
I was thinking just the opposite. What we have now is convoluted, and
discourages me. The changes I’m proposing make each class responsible
for less than it is now, and that should make it easier for future
developers to find their way around.
I wasn’t offering this as an alternative to what you’ve done in the demo.
What I described is a structure into which to put the drawing code, but is
not the drawing code itself. What you’ve done in the demo would go into a
class which instantiates BoardView (or possibly MapView, or possibly both)
according to what I’ve described so far. It’s just that I don’t see any
reasonable place to put what you’ve done in the demo without doing this
separation first.
That brings up a question I have about the demo: How will you handle (1)
drawing pieces, and (2) drawing whole maps, not just single boards?
I was referring more to the fact that to create a new Configurable, you have to make several classes.
But otherwise, you’re right.
I would also suggest that a default superclass of the factory should just draw into a GraphicsContext without knowing where it came from. That’s doable and I’m going to be working next.
Right now, my goal is to subclass TextureRenderer to handle very large images by breaking them up into smaller textures. Everything will just be drawn to a graphics context without knowing where it comes from.
However, my secondary goal, is to draw pieces as individual textures. That will come when there are separate views for Java2D and JOGL. The pieces can then have a thickness and be placed “on” the board and actually stacked. Boards should just be drawn to the TextureRenderer graphics context without knowing what it’s being drawn to as there’s nothing really special about a board.
Some time in the next day or two I’ll set up a test server with Brent’s changes so we can test it live. If it checks out, I’ll upgrade the server and we can proceed.
Actually, I think it’s reasonable to just say that sub-classes shouldn’t call super.mySetType() at all. Those methods are usually boilerplate, so copy-pasting them into the subclass isn’t too objectionable. Then you get the benefit of using the inherited fields and logic without being vulnerable to changes in the encoding of the type information.
If you do change the format of the type string, I recommend introducing an alternate prefix for the trait, and leaving the old implementation around for use with the old prefix. You can see where this was done in the Embellishment class, where the current prefix is ‘emb2;’ but the old prefix of ‘emb;’ is still recognized for backward compatibility.
I also believe that separating the Model from View is essential to improving the look of the engine going forward.
I would argue, however, that the “Model” should be the dynamic image of the piece (i.e. the way it should be drawn in the absence of any context) and the “View” should be how those piece images are combined with map images to render a playing surface.
For example, one View implementation would rotate the map 180 degrees but leave the pieces right-side up. Another View implementation could show the map in a perspective view with the pieces rising perpendicular to the map surface (i.e. on their “edges”).
Having to define a Listener and a View class for every single trait is way too heavyweight. It will significantly raise the barrier to developing new game piece traits. Take another example: Labeler. Would you really define listener events for font, font size, vertical position, horizontal position, vertical justification, horizontal justification, etc.? And then have a view class that takes all of those properties and implements draw() just as Labeler does now? Doesn’t make sense. But maybe I’m misunderstanding what you’re suggesting.
We want the ability to change how a piece looks, but we should pick the right granularity. Just being able to orient the map independently from the roughly-rectangular-icon-that-is-a-game-piece would be a huge advance. That can be done with something close to Michael’s TextureRenderer suggestion.
There is no way that a piece should be drawn in the absence of any context.
This is just mixing the data with the on-screen representation again, which
is what I’m arguing against. Doing it this way gets us no closer to having
the graphics code separated from the data handling code than we are now, and
probably makes it harder to do that in the future.
I don’t believe that. This will make it no harder to develop new traits, and
possibly easier. One way or the other, you have to write the code which would
be in these methods. This is only a question of where it lives. Right now we
have a lot of classes which are a confusing tangle of drawing code and data
handling code, and for a new developer having those two bodies of code live
in separate classes should help them answer the question of “What am I looking
at?” more rapidly than they can now.
Why not? We already do this, just inside Labeler.setProperty() A View could
in principle respond to any change in a Labeler, and it’s not for the Labeler
to decide that—the Labeler shouldn’t know anything about how its Views
render it.
I’m not saying that we have to have a seperate method for each property—
we could do this with PropertyChangeListeners instead. I haven’t written
a prototype with either one, so I don’t know which involves more boilerplate
code.
Rendering the map with a different orientation from the tokens is othogonal
to the TextureRenderer. Whatever we do there to bring that about won’t be
much different from what it would take to do it in the current rendering code.
It’s just some tests and an affine transform.
Well, because those properties don’t actually represent the state of the piece. The only piece of state in a Labeler trait is the value of the text label. Similarly, the only piece of state in a Layer/Embellishment is the index of the active level. In fact, the majority of game-piece configuration properties don’t specify the state; they specify how to draw the state.
So when a user configures a Text Label trait in the editor, they’d really be configuring a TextLabelView class. That’s fine, but you’d have to also have editors for the properties of all the other different Views in the system. Could get overwhelming to the module designer. What other views are we imagining? About the only thing I can think of is simplifying the small zoom factors by dropping text labels, maybe.
If we were to split the GamePiece classes, we’d probably end up with the current classes assuming the View role and create new XXXState classes. For example:
interface GamePiece {
…
GamePieceState getGamePieceState(); // get the state as an object
…
}
interface GamePieceState {
Point getPosition();
Map getMap();
Stack getParent();
String asString(); // Equivalent of today’s GamePiece.getState()
}
They are part of the model, for the following reason: All of the data in
a view should be transient—it should be possible to write a save or log
without ever touching anything in a view. If you want to make a distinction
between the part of the model which is state and the part of the model which
is largely static, that’s fine, but you wouldn’t be able to restore a view
from only the state information if you put all of the configuration
information into the view.
When I say that the model should contain no graphics code, I mean that it
should contain no concrete graphics code. Clearly trait like Text Label has
an intended semantics, by which you’d expect some text to be produced in a
specified font and size at a specified location, and so a lot of stuff in
the Labeler class would be graphics code in the abstract—but my point is
that none of it should produce or manipulate actual graphics. The font, size,
and location of text is still data, not rendering.
Consider, for comparison, what’s stored in a document for a word processor.
The situation there is similar to ours. What’s stored are the text and its
properties, but not the actual rendering of the text—the word processor
is responsible for generating that. A different word processor which can
read the same file format might generate a somewhat different rendering of
the same document.
Much of what I’m saying here is echoed in the Gang of Four book, Design
Paterns, specifically in the case study in Chapter 2 where they describe
the structure of a document editor. You can see a brief summary of it here:
In particular, look at the summaries of the sections “Supporting Multiple
Look-And-Feel Standards” and “Supporting Multiple Window Systems”. The
situation which I believe we’re in is the one described in the latter
section. Anyway, if you have access to this book, I recommend it highly.
There are clean solutions here to many design problems we’ve had (and have),
and it’s well written.
There are many, many ways we could use this. A piece inventory, e.g., could
be built as a view, as could other sorts of displays and tables. A view could
be some kind of log, or a web page. And part of the point of this is to make
it easy to create views we haven’t thought of yet. Having views be listeners
means less churn in the model classes when we do graphics work, and should
also give us a more reliable way to make sure that visuals are updated when
they should be. Additionally, we could use listeners for some things which
aren’t views—e.g., triggers could be implemented as listeners.
I see several architectural problems here:
How do you attach multiple views to the same state this way?
How can we switch out one view for another? (That’s what needs to happen
with the JOGL code.)
How does a state tell its views to update themselves?
As is, no existing module would be able to take advantage of new views. A
Map would get loaded instead of a GLMap, becuase Map is what’s in the
buildFile.
If the views are classes instead of interfaces, then you lose the ability
to implement multiple views together in the same class. This could make a
difference (a) in memory usage (allowing us to combine view implementations
which have no internal state) and (b) efficiency (in the case where the
drawing of different views is not independent).
And a nomenclature problem:
The distinction between noumena (things themselves) and phenomena (things as
they appear) is a long-standing one in philosophy. These are, in fact, what
you’d call Model and View, respectively, in software engineering. Labeler,
BasicPiece, FreeRotator, etc. are names which strike me as more appropriate
for noumena than for phenomena. If BasicPiece is not actually the piece, but
a view of it, then we will have a misleadingly-named ontology. That will
be confusing (especially for new developers).
We should be able to design this in a way which is type-safe.
Perhaps you’re taking a narrower definition of “view” than I am, but different views clearly need different information to render a piece. Take the example of a piece with 4 different Text Label traits. One view acts like today’s Labeler and draws the label values at each corner of the piece. Another view might be a table view in which the label values goes into differently-named columns. One view needs the module designer to specify positions, another requires the designer to provide column names. Both sets of properties have to be specified independently at module-edit time. You’re right that you don’t have to write them in the saved game, provided you have another way of solving the module-version-compatibility problem.
Classic book. Obviously had a big impact on VASSAL when I first read it about 10 years ago now. Didn’t even bother to rename the Command and Decorator patterns in the code!
Well, this effort will fail if we start writing code without a clear use in mind. Otherwise, we’ll surely find that we chose the wrong abstractions when we go to apply it. That’s a fairly common pitfall in software development.
This is not hard. Each view component has a collection of GamePieceView references. Each GamePieceView is a stack of Decorator views (LabelView, LayerView, etc.), each of which has a reference to a state (LabelState, LayerState, etc.) What I don’t yet know is whether the different GamePieceView implementations have enough in common that it even makes sense to have them implement a common interface. For example, an inventory-table view would not want to have a draw() method, like the current Map-oriented views do. Instead, it would probably need a getColumnValue(String) method or something.
In fact, you could implement a new view perfectly fine as I described above without refactoring the GamePiece classes at all, just give each Decorator View a reference to the Decorator object as currently defined. That’s essentially what the Game Piece Inventory component does now. It gets all the information it needs from the current set of GamePiece classes and composes an alternative view just fine. Bottom line is that there will be very little common code between the existing GamePiece classes and the classes that implement a different view, and I doubt that the different views will share much code either. There’s nothing wrong with that, but it means that this debate we’re having over interface design is relatively low-impact.
I agree with Rodney on this one. Replace.mySetType() should never have called super.mySetType(). I don’t think there will ever be enough traits extending other traits that need any other solution than encoding mySetType() from scratch.
