[Freeciv-Dev] Re: (PR#3727) Rectangular selection with right-click-and-drag

["a-l@xxxxxxx" <a-l@xxxxxxx>]

On Wed, 19 Mar 2003 00:14:23 -0800
"Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx> wrote:

> I haven't fully figured out this code, but I do have some comments.
> 
> > +/**************************************************************************
> >  ...
> >  **************************************************************************/
> >  gint move_mapcanvas(GtkWidget *widget, GdkEventButton *event)
> >  {
> >    update_line(event->x, event->y);
> > +  if (!hover_state) {
> > +    update_rect_at_mouse_pos();
> > +  }
> >    return TRUE;
> >  }
> 
> Is hover_state really the correct check here?

I wouldn't want to draw a rectangle and goto lines at the same time.
Actually, I think I rather want to disallow changing hover_state when
rectangle is active, and vica versa.

> 
> > +/* This changes the behaviour of left mouse button */
> 
> Changes how?

This is just like Control-Left-Click in the City List Window.

Here's an example of how it works:

Say you want to change production to "Musketeers", for {all cities except
one on a particular island, plus for one city on another island}.

You drag mouse around the island - all cities within rectangle become
highlighted. *Area Selection mode is now active*. You plain left click
on the exception city - it becomes un-highlighted. Click on the city
on the other island - it becomes highlighted.

Now Shift-Left-Click on a Musketeer or Phalanx, or on a city producing
that, to put in clipboard, if neccessary.

Now Shift-Right-Click anywhere, and the goal is accomplished.
*Area Selection mode is now de-activated* because you performed
an action. The left mouse button returns to normal behaviour, and all
highlighting goes away on the map.

As you can see, cities outside of the initial rectangle can also
become highlighted. That's why "whole_map_iterate" was needed later on.

Furthermore, every time you toggle the highlighting of a city on map,
the City List Window highlighting toggles accordingly. So you can also
use Area Selection to visually select which lines to highlight in that
window, and perform advanced actions there (CMA and so on).

You can also Shift-Right-Click on a single city when Area Selection
mode is off.

> (wrapping issues)

> > +    whole_map_iterate(x, y) {
> > +      ptile = map_get_tile(x, y);
> > +      if (ptile->selected) {
> > +        ptile->selected = 0;
> > +        refresh_tile_mapcanvas(x, y, TRUE);
> > +      }
> > +    } whole_map_iterate_end;
> 
> This function is very inefficient.
> 
> - Is whole_map_iterate necessary at all?  Can't we just limit the 
> iteration to those tiles covered by the rectangle?

As I explained in the example above, the rectangle only defines the
initial area, and activates Area Selection. Although user may not
choose to do this often, he can scroll the map (with keys!) and
highlight additional tiles anywhere. And because I wanted to also
introduce unit highlighting later (see below), iterating owned cities
would not suffice.

My goal is to allow user to do stuff on the things he sees in front of
him with minimum hassle.

> - Calling refresh_tile_mapcanvas here is quite inefficent and results in 
> many unnecessary draws (up to 7x as many as needed) in iso-view. 
> update_map_canvas() would be better (but again you have to consider the 
> range of the rectangle).

I think maybe.. upto a dozen or two tiles would need refreshing in
practical cases, assuming ICS. I don't know what's more efficient.

> - You need not pass write_to_screen==TRUE here (with the new flush code 
> it should pass FALSE instead).

That's a useful patch, didn't realize it was comitted for non-SDL.
But, wouldn't that patch precisely make using
        refresh_tile_mapcanvas(x, y, FALSE)
less inefficient again?

> > +++ rectangle/common/map.h  Thu Mar 13 14:36:44 2003
> > @@ -62,6 +62,8 @@
> >    struct city *worked;      /* city working tile, or NULL if none */
> >    unsigned short continent;
> >    signed char move_cost[8]; /* don't know if this helps! */
> > +  int selected;             /* Area Selection. 1=city. Client only.
> > +                               Leave as int. */
> >  };
> 
> I suppose it is most efficient to add a new field to the map structure, 
> that is only used by the client.  But why is it an 'int' not a 'bool'?

I also wanted to pave the way for a Mass Orders patch and so on.
The next step is to have

    ptile->selected == 2

to mean a tile containing owned units is highlighted (with a different
color sprite). Then, maybe I want to do something like "Assign all
appropriate units in highlighted tiles to Battlegroup 4", as another
visual tool.

> You may not 
> understand the different coordinate systems well enough to provide an 
> implementation that can correctly handle different topologies with both 

Quite, I'll have to wrap my mind around those, and look at Mike's
tools again. (I've seen that some of the harshest critics of Freeciv
in open forums complain the iso map is not like in Civ II.) Uhm.. I'm
not quite sure what question to ask. Say I make it so that it looks
rectangular and flat-on-top, instead of rhomboid, in current iso mode;
would that still be correct with a new topology?


Arnstein