Thus spake Flaney:
I can send you my latest work-in-progress. What’s the preferred data
transfer method? The module is about 11MB.
It’s a bit large for email. Is there someplace you can upload it?
–
J.
Thus emoted uckelman:
None that comes to mind…Any suggestions?
Thus spake Flaney:
Thus emoted uckelman:
It’s a bit large for email. Is there someplace you can upload it?
None that comes to mind…Any suggestions?
Could you put it someplace like mediafire.com?
–
J.
Here is the URL:
mediafire.com/file/fivf39x1x … lon5+.vmod
File->New Game
Click Empires along the top buttons and select…Centauri. This brings up the Empire control sheet showing three tabs: Empire, Scrapyard, ForcePool.
The Empire tab should show two GKC buttons [Economics], and [Year]. However, they are not displayed until the window is resized.
This is an ancient bug (the last thread was from 5 years ago…) see also:
https://forum.vassalengine.org/t/toolbar-bug-buttons-not-visible/3021/5
https://forum.vassalengine.org/t/zoom-capability-breaks-tabbed-map-panels/4307/1
For some reason all thye buttons on the toolbar are of size zero until you resize the map. Still looking for why
Thus spake george973:
For some reason all thye buttons on the toolbar are of size zero until
you resize the map. Still looking for why
The problem appears to be caused by the WrapLayout that’s set on the
Map’s toolbar. Specifically, it seems to be layoutContainer() that’s
the culprit, as I can’t reproduce the problem anymore if I comment it
out. I’m a bit leery of simply removing layoutContainer(), as I haven’t
been able to satisfy myself as to what exactly it does in the case
where it’s not simply delegating to super.layoutContainer().
I thought I might find some hint through archeaology:
WrapLayout is some old, old code. It appears to be substantially
unchanged from when it was introduced in VASSAL 1.7. It appeared on
someone’s blog in 2006, and there’s there’s a version of it which has
been in GWT since 2010. So far as I can see, both are almost the same as
ours—they even have the same typo in the thrid line of the long
comment in layoutSize(). It looks like both also have the same bug as
ours—either that, or we’re seeing this problem because we’re misusing
it.
–
J.
What it’s doing is definitely wrong as validate() trundles down the tree of views calling doLayout() for each one which ends up is each LayoutManager’s layoutContainer(). I think just commenting out that method is best. Why would you want to relay out higher level views because your prefered size has changed? The higher levels would have used your current prefered size in calculating the current layout. Looks like someone was trying to trap improper use (just laying out toolbar without its parent) which is not possible with proper use of invalidate(0 etc in swing. I’m not sure what WrapLayout gives you over FlowLayout (unless it calculates the size better).
Tried on TITE modules (which has a very large toolbar) and works fine with it commented out. So I’d consider chopping it since what it does is definitely invalid - and could cause infinite loops if other layout managers did the same.
Thus spake george973:
The problem appears to be caused by the WrapLayout that’s set on the
Map’s toolbar. Specifically, it seems to be layoutContainer() that’s
the culprit, as I can’t reproduce the problem anymore if I comment it
out. I’m a bit leery of simply removing layoutContainer(), as I
haven’t
been able to satisfy myself as to what exactly it does in the case
where it’s not simply delegating to super.layoutContainer().What it’s doing is definitely wrong as validate() trundles down the tree
of views calling doLayout() for each one which ends up is each
LayoutManager’s layoutContainer(). I think just commenting out that
method is best. Why would you want to relay out higher level views
because your prefered size has changed? The higher levels would have
used your current prefered size in calculating the current layout.
Looks like someone was trying to trap improper use (just laying out
toolbar without its parent) which is not possible with proper use of
invalidate(0 etc in swing.
The version of WrapLayout in GWT has the following comment in
layoutContainer():
// When a frame is minimized or maximized the preferred size of the
// Container is assumed not to change. Therefore we need to force a
// validate() to make sure that space, if available, is allocated to
// the panel using a WrapLayout.
Someone seems to be warning us that this code in layoutContainer() is
necessary, but I haven’t been able to find a situation where not
overriding layoutContainer() fails. If I minimize or maximize a window
containing the tool bar with a WrapLayout, I don’t see any any change
in the layout of the tool bar, regardless of whether layoutContainer()
is overridden.
So, yes, I don’t see it either. Can you make out what it is they mean
by this comment?
I’m not sure what WrapLayout gives you over FlowLayout (unless it
calculates the size better).
I can confidently answer this, at least: FlowLayout does not adjust
itself when the component for which it is the layout resizes. If you
substitute FlowLayout for WrapLayout, the initial layout will be
correct, but then if you reduce the width of the window, the children
to the right for which there is too little space simply won’t be shown.
If you do this with WrapLayout, those children will form as many
additional rows as necessary. Since this is the obvious thing for
FlowLayout to do given its name, I’ve always wondered why it doesn’t.
–
J.
OK I dug around. The problem is not that FlowLayout does not wrap its layout correctly but change in the required size is not reflected correctly.
So I modified the doLayout method in WrapLayout to:
a) call normal Flowlayout doLayout method.
b) then find out how big the target would have to be to contain all its components.
c) if the target is big enough that’s OK If not invalidate the target and revalidate it (Set a flag to prevent infinite recusrion if the target
cannot be made big enough)
This seems to work fine.
Toolbar buttons appear OK even if code adding HierarchyChangeListener to the toolbar in Map is removed. They also wrap correctly as well.
Released in revision 8081
Thus spake george973:
So I modified the doLayout method in WrapLayout to:
a) call normal Flowlayout doLayout method.
b) then find out how big the target would have to be to contain all its
components.c) if the target is big enough that’s OK If not invalidate the target
and revalidate it (Set a flag to prevent infinite recusrion if the
target
cannot be made big enough)
In what circumstances does the code you added to layoutContainer()
do something different from what super.layoutContainer() does? I’d
like to see what the difference is.
–
J.
If the laid out toolbar components (by suoer) don’t fit within the size allocated to the toolbar it causes the tree containing the toolbar to be laid out again. It does this in a correct manner so it always works (or seems to).
The Preferred size calculations within WrapLayout work with the last width allocated to the toolbar so upon a second iteration give a different height to the first so the toolbar is given more space.
Thinking about the problem I feel that a toolbar wrapping into multiple lines is the wrong solution. If there’s not enough space to display the whole toolbar it should become horizontally scrollable without a scroll bar but with a small button at each end with an arrow upon it rather like an over large menu but horizontal rather than vertical.
Thus spake george973:
In what circumstances does the code you added to layoutContainer()
do something different from what super.layoutContainer() does? I’d
like to see what the difference is.If the laid out toolbar components (by suoer) don’t fit within the size
allocated to the toolbar it causes the tree containing the toolbar to be
laid out again. It does this in a correct manner so it always works (or
seems to).
What I’m asking is: How do I bring this situation about, so that I can
see the difference between old WrapLayout and your modification?
Thinking about the problem I feel that a toolbar wrapping into multiple
lines is the wrong solution. If there’s not enough space to display the
whole toolbar it should become horizontally scrollable without a scroll
bar but with a small button at each end with an arrow upon it rather
like an over large menu but horizontal rather than vertical.
Do you know of any other programs which do this? Everything I can think
of which has tool bars wraps them rather than using scroll buttons.
–
J.
Uh… are you including the main window toolbar in this idea? You’d better take a look at the Federation & Empire module.
I think I’ve seen programs that do this, and it can work if it’s a fairly small toolbar, with just a little bit that doesn’t fit. Generally only a good idea if screen real estate is at an absolute premium.
I am seeing it more and more in newer programs
Here is an example toolbar. I wouldn’t call it a scrolling bar though - more of an extended hidden pane that is accessible by pressing the “>>” arrows. The scrolling part of the bar is in effect on the ribbon portion though via mouse wheel on the tabs
[attachment=0]toolbar.png[/attachment]
If you resize the window both work correctly with the toolbar wrapping. You see the difference in that my fix doesn’t show the bug which started this discussion. When you open the centauri map the buttons are displayed correctly.
This is because I use a different test to decide whether to redo the tree layout and I use the correct way to achieve this.
Firefox does the same with its bookmark toolbar. If there’s too many to display it puts a “>>” button at the end of the toolbar clicking on which gives the remaining entries as a drop down menu. This might not work for us but could work by scrolling the toolbar left to reveal more choices.
Thus spake george973:
What I’m asking is: How do I bring this situation about, so that I can
see the difference between old WrapLayout and your modification?If you resize the window both work correctly with the toolbar wrapping.
You see the difference in that my fix doesn’t show the bug which started
this discussion. When you open the centauri map the buttons are
displayed correctly.
Sorry, I didn’t ask about what I meant to. What I intended to ask about
was the difference in behavior between your code and simply not
overriding layoutContainer at all.
–
J.
WrapLayout calculates the preferred size on the basis of the current width and how that causes the toolbar to wrap. This size is used to allocate space for the toolbar. Since the width may change this calculation may be incorrect.
My extra code overriding layoutContainer just checks that the component as laid out fit within the size of the toolbar and force another round of layout if they don’t.
The effect of not having it there means that it is possible for buttons not to appear because they’re outside of the toolbar and therefore not drawn. How often it would happen is not clear since dragging a winow smaller causes lots of resizes. You’d need to put a breakpoint on the relayout code or a message and try lots of resizing to see how often it triggers.
Basically it’s an insurance policy which won’t always be needed but will stop the problem of toolbars not appearing.
Thus spake george973:
My extra code overriding layoutContainer just checks that the component
as laid out fit within the size of the toolbar and force another round
of layout if they don’t.
Ok. Merged to trunk@8083, 3.1@8085.
–
J.