Scroll Areas don't continue to scroll when in the mousePressed event

[GoogleCodeExporter]

I noticed this a bit ago, but haven't reported this issue until now.
Basically, when you press down on the scroll buttons in the scroll area,
the scroll content will move down only one tick for every time the mouse
button is pressed. It'd be nice to make the scroll areas behave like other
scroll areas and continue to scroll as long as it's pressed, and to do
that, there's a few options that I can see to address this, and you can
comment back on what you think would work best for GUIChan (or even if you
have a different solution that you think would work better, that's fine as
well).

1. Add a new mouseHeld() event, which will continue to poll as long as the
mouse button is held. This might be a little tricky, depending on how it's
done, but ideally, I think this would probably be the best way to address
this. This would also tie in rather nicely with issue #7, as timestamps for
when the mouse is pressed would help a lot in determining when the event
should get consumed. It'd be nice to be able to set a default time interval
between event firings if you did that, but it wouldn't be the end of the
world if the developer couldn't control the firing intervals, or if they
just ran continuously while it's held.

2. Modify the logic() in the ScrollArea to keep firing off mousePressed()
events when it detects that the mouse is held. I personally don't like this
option as much, but it is a lot simpler to execute. The main reason being
that I think that GUIChan could benefit from an extra event type for mice,
since this is a separate behavior, so to speak, much like a mouse pressed
and mouse released events when combined are like one mouse clicked event.

Any thoughts on what might be the best way to address this?

Original issue reported on code.google.com by irar...@gmail.com on 21 May 2009 at 4:30

[GoogleCodeExporter]

Why not just maintain a state variable? Set it on mouse press and unset on mouse
release. Then every few logics (preferably configurable) if the state is set, 
scroll
some more.

Original comment by jaxad0...@gmail.com on 21 May 2009 at 5:49

[GoogleCodeExporter]

The problem with timing is that Guichan doesn't know anything about time. 
That's why 
the ScrollArea is implemented the way it is. And about a mouseHeld event, the 
mouse 
is held between a mousePress event and a mouseRelease event, so I don't really 
think 
it's necessary to add a mouseHeld event. If Guichan knew about time Scroll Area 
could 
easily be changed to scroll down after a certain amount of time when the mouse 
is 
held.

I have been thinking about addin a Timer class or something to Guichan before, 
but 
I've come to the conclusion that it doesn't seem to be very usefull, except 
when 
implementing backend independent stuff - but that's something users of Guichan 
seldom 
do.

Original comment by olof.nae...@gmail.com on 23 May 2009 at 9:31

[GoogleCodeExporter]

Good point. Still though, it'd be nice to have some method in which to control 
scroll
speed, even if the user of GUIChan has to override it. In that case, how would 
you
feel about making the mousePressed event in the ScrollArea just define what 
button is
pressed, like it does now, but then spinning off the scrolling portion to 
another
function, and then letting the implementer call it within their logic loops to 
allow
for smooth scrolling in their own projects. Here's a snippet to demonstrate 
what I
mean (and a semi-implementation as well, I suppose):

void ScrollArea::mousePressed(MouseEvent& mouseEvent)
{
...
    if (getUpButtonDimension().isPointInRect(x, y))
    {
         mUpButtonPressed = true;
    }
    else if (getDownButtonDimension().isPointInRect(x, y))
    {
         mDownButtonPressed = true;
    }
    else if (getLeftButtonDimension().isPointInRect(x, y))
    {
        mLeftButtonPressed = true;
    }
    else if (getRightButtonDimension().isPointInRect(x, y))
    {
        mRightButtonPressed = true;
    }
...
    scroll();
}

void ScrollArea::scroll()
{
    if (mUpButtonPressed)
    {
        setVerticalScrollAmount(getVerticalScrollAmount()
                                - mUpButtonScrollAmount);
    }
    else if (mDownButtonPressed)
    {
         setVerticalScrollAmount(getVerticalScrollAmount()
                                 + mDownButtonScrollAmount);
    }
    else if (mLeftButtonPressed)
    {
        setHorizontalScrollAmount(getHorizontalScrollAmount()
                                  - mLeftButtonScrollAmount);
    }
    else if (mRightButtonPressed)
    {
        setHorizontalScrollAmount(getHorizontalScrollAmount()
                                  + mRightButtonScrollAmount);
    }
}

This would be sufficient enough to allow the programmer to get their job done
properly, but at the same time, I still think that it would be a good idea to 
have
smooth scrolling handled by default, and not need to be a programmer task. And 
it's
also approximately what I meant by #2, although I probably could have phrased it
better. Feel free to comment on what you think on this, as well as to propose
differences if you don't fully agree with this resolution as well (however, I 
think
that given what you said, this should satisfy the issue to your liking).

Original comment by irar...@gmail.com on 23 May 2009 at 4:44

[GoogleCodeExporter]

I guess there isn't any harm in adding the scroll method. And I second it would 
be
nice with smooth scrolling, but it's pain to implement in a clean way.

Once I tried using standard C++ timer stuff, but it's not usable as it's not 
properly
implemented on some platforms (such as GNU/Linux).

Original comment by olof.nae...@gmail.com on 28 May 2009 at 12:57

[GoogleCodeExporter]

Heh, that does suck a bit. But in any case, with SDL and that method, you'd 
just have
to add a poll to the logic loop which would call the scroll method on the timer 
and
overload mousePressed to reset the timer on mousePress, which wouldn't be too 
bad
overall (Probably the same with other timers as well). IMO, that'd be clean 
enough
and simple enough that it wouldn't be all that painful to do.

Oh, and as a small correction, since I didn't want to post it or delete my 
previous
comment and repost over a simple mistype, I meant that that method is 
approximately
what I meant by #2 without the timer.

But on to the timer issue. Have you ever thought about just making an abstract 
Timer
class that doesn't actually do anything, but allows for the programmer to 
overload
its functions with the timer library of their choice (much like you have for 
events
like the MouseEvents, etc.)? If you did that, then it's the programmer's
responsibility to supply their own timer to allow for things like smooth 
scrolling,
or other actions that they need to do (we use one to throttle frame rates on our
end), and GUIChan wouldn't be required to have any timers itself to work. It 
could
even be overloaded in library, if a specific library binding that you provide 
has its
own timers. I think that that's at least worth thinking over a bit. The next 
thing to
do would then be to figure out exactly what's needed. Personally, I think it'd 
be
better to do it on some sort of stamp, and then compare it, so that you don't 
need to
keep incrementing the timer, and can calculate the number of ticks by just 
comparing
the current stamp to the old one. So in that case, I think that the appropriate
methods would be a reset() and a timeSince(timer) (just as some example names). 
All
that would need to be done in that case is to make a global timer class that 
the top
widget manages, and resets its time stamp periodically, and then assign a timer 
to
any class which needs it, which that widget would be responsible for resetting 
as
needed. Then, to get the ticks, it just compares its stamp to the top widget's 
timer.

Just something to think about there. If you see any specific problems with such 
a
simplistic method for timers, then feel free to call me on it. My main concern 
here
is trying to figure out what's best for GUIChan, and then helping you guys get 
to
that as easily as possible. That, and I do like constructive feedback.

Original comment by irar...@gmail.com on 28 May 2009 at 6:18

[GoogleCodeExporter]

Since no one has replied to this yet, and I'd like to make some additional 
comments
on the idea that I proposed a bit ago, I'm going to take the liberty to do that 
now,
since it might help out a bit with deciding what to do on this issue.

First off, the function names I suggested might not be the best, and the more I
thought about it, the more I thought that update() and compare(timer) would be
better. And to reiterate, this is exactly what the abstract timer would look 
like
when coded up (condensed to save lines):

class AbstractTimer
{
AbstractTimer() {}

virtual void update() {}

virtual unsigned int compare(AbstractTimer timer) { return 0; }
}

Now, I say unsigned int right now, since that should be sufficient, but it'd be
better if we could make it so that the implementer could specify the return 
type that
they want. As to why I think that only having two functions is sufficient, and 
why
there is not an incrementer to the timer, I'll answer it this way: When we're 
dealing
with GUIs, I think that it's more important to make the gui appear smooth. So 
if we
were to do it accurately, we could occasionally see a skip here and there if 
it's a
bit slower than when we attempted to catch it, in which case, the extra should 
just
be discarded, since most users won't notice a few milliseconds off. Even then,
though, this method for timers does not discourage the implementer from being 
precise
if they want to be, since they can still assign update() to update a number of 
ticks,
rather than to pull a time stamp, so even then, it's still flexible enough to 
allow
anyone to do what they'd like to do.

As to what should have an AbstractTimer, this is a bit debatable, since I don't
anticipate that all widgets should have one, or even need one. So, you could 
create a
global one when creating the GUI, do nothing and specify that the user of 
GUIChan, if
they want this functionality, should implement a global one in their own GUI 
class
(probably the best approach), or to do it in the top widget. However you slice 
it, I
don't see this as actually implementing timers within GUIChan, but rather just
recognizing that there happen to be areas within GUIChan where some users of the
class might benefit from having this control, and reducing the needed work on 
the
implementer's end.

And if that still doesn't sell an AbstractTimer class, you already have one if 
you're
talking about mouse clicks. In my opinion, it'd be nice to abstract the timer 
from
checking clicks outside of there, and then delegate this handling to the 
implementer
of the GUIChan class, rather than having GUIChan be directly responsible for it.

Lastly, I think that actually doing it this way respects your wishes a bit of 
not
implementing actual timers in GUIChan, which I can agree with you on that 
providing
timers to the user shouldn't need to be GUIChan's responsibility, just like 
providing
every conceivable widget type or combinations shouldn't be your responsibility 
as
well. The only thing that I think this does do is that it concedes that there 
are
certain expected functionalities within GUIChan that need timers in order to 
work
properly, and that you'll provide the hooks for it for those situations (since 
it's
simple enough to do), but helping to make it clear that if the user wants that
functionality, that it is their responsibility then to write their own 
implementing
class, much like you're already doing for widgets.

Hope that helps clarify what I was proposing a bit more, and helps in stepping 
this
issue closer to a resolution.

Original comment by irar...@gmail.com on 5 Jun 2009 at 6:44

[GoogleCodeExporter]

irarice dont' be suprised no one answer because I was very motivated to 
implemented
stuff on guichan and I don't know how they do but maintainers convinced me it 
was a
bad idea.
I wanted to add support for windows CE and I had reported some suggestions 
about tab
handling for instance but nothings has changed...

Original comment by v.richomme@gmail.com on 6 Jun 2009 at 9:56

[GoogleCodeExporter]

v.richomme, we have the repository moved to git now so everybody can just clone 
it
and requests changes to be merged back later. I'm not exactly sure what you're
expecting from us, but I'm sure Windows CE support would be welcomed.

In any case, I don't see the relation to this timer thing. The timer is a lot 
harder
to discuss about since there are so many ways of doing it, in addition there 
was the
original design decision to not have Guichan care about timing. I think Ira's 
best
option is to show something that works and looks moderately acceptable. 
Personally I
can't have a strong opinion about it since I don't have time to look into the 
issue.

Original comment by b.lindeijer on 6 Jun 2009 at 10:09

[GoogleCodeExporter]

One thing we could do with the scroll area is to assume the logic() method is 
meant
to be called a fixed amount of times per second, and then allow the user to set 
a
"scroll internal" in terms of logic calls. So when you're calling logic 100 
times per
second and what the button to scroll 4 times per second while being held, you 
set
scroll internal to 25. Sounds simple and effective to me, with the only 
downside that
it won't be able to support the case where logic() is called at irregular 
times, but
in that case I think logic() should have been passed the time delta since the 
last
call anyway.

Original comment by b.lindeijer on 6 Jun 2009 at 10:13

[GoogleCodeExporter]

Bjorn, no offense, but your solution there makes GUIChan time dependent, which 
Olaf
was saying that he was trying to avoid in GUIChan. My solution here doesn't make
GUIChan time dependent, but acknowledges that there happen to be instances 
where this
might be needed, and then moves the responsibility for hooking in your own 
timer to
the implementer, since that seems to be what Olaf wants. And given just how 
hairy SDL
timers are in order to ensure that they're accurate, I fully agree with Olaf 
now that
packaging your own timers should not be a GUIChan core function (although that
doesn't mean that some sort of hook for timers shouldn't be present. However, I 
see
no problem with providing an SDL timer implementation, so long as it stays in 
the
contrib folder). As for there being multiple ways of implementing timers, yeah, 
I
acknowledged that, but they all comes down to the two simple methods that I
mentioned, which is updating them and comparing them against something else 
(even if
it's 0). If you want more than that, then it's easy to extend it in an 
implementing
class to do what you need, but it all comes down to that those are the only two
things that GUIChan should care about.

Honestly, this isn't all that complex of a patch, and it shouldn't be difficult 
to
follow (I've explained out in length on what this would do and why it is a 
sufficient
abstraction in order to try to help move things along. I'm trying to work 
towards a
patch here that would be acceptable to the GUIChan team as a whole, which is 
exactly
why I have been trying to discuss possible implementations first, and to try to 
get
some counterproposals going, rather than to say here's an implementation, take 
it or
leave it. So in my opinion, I feel that what I have been doing right here has 
been a
lot more respectful than just giving out a patch that developers here might not 
want
to do). Not to mention, that the implementation I put up as the base does 
nothing by
default, which basically makes it function just like it already does now, but 
allows
for implementers of the GUIChan class to hook in their own timers as needed.

However, if you really need it to be "show me the code" and need me to show you 
the
code, rather than trying to discuss this on a higher level than the code, then 
so be
it. I never said that I wasn't willing to follow it through all the way, but I 
just
wanted to make sure that I was going in a direction that was acceptable to the
GUIChan team. I've made a clone just now, and will push out all of the patches 
on
Monday to do this, since it won't take a lot of work, as well as an 
implementing SDL
timer in the contrib folder. You can then evaluate it there in code form.

Original comment by irar...@gmail.com on 6 Jun 2009 at 4:49

[GoogleCodeExporter]

v.richomme, we never had anything against a Windows CE port, we just didn't 
want to add a rushed 
unicode implementation as Guichan doesn't include unicode support, and if it 
should we want a 
clean implementation. Your proposal has been suggested before and has been 
rejected.

I want Guichan to be as simple as possible so it can be maintained. People come 
with improvements 
and suggestions all the time, and that's great, but I'm not keen for 
suggestions and improvements 
that requires a lot fo work for me and the other developers. I have other 
things to do besides 
Guichan. In fact I have lots of other stuff to do.

Users of Guichan that contribute patches come and go (usually they stick around 
a year or so, 
then they get bored of their project) and we get left behind with broken code 
(most contributed 
code contains at least one or two bugs).

We decided to skip timers early on as most users make derived classes for 
widgets anyway and can 
easily add their own timer code into their widgets (just like the Guichan FF 
demo). But of 
course, no one can make backend independent widgets that use time. I know we 
use some sort of 
time in Guichan with mouse clicks so we could change that into a Timer class I 
guess. It would be 
nice if we could have some kind of vote or poll just to get the idea what users 
of Guichan want.

Oh, and irarice, my name is Olof, not Olaf ;)

If we should add a Timer I think this interface suffice:

class Timer
{
public:
    virtual int getTime() = 0;
    virtual void logic() {}
};

And we add a DefaultTimer that simply returns 0.

Original comment by olof.nae...@gmail.com on 7 Jun 2009 at 6:43

[GoogleCodeExporter]

Alright, that can get changed, but I pushed out the proposal already. Oh, and 
Bjorn,
just so that you know why I hadn't done this sooner, it's because in my opinion 
we
were still in the design phase for a solution here, and coding up a solution 
before
an appropriate method is agreed on is putting the cart before the horse. 
However, I
did code up a solution this time, but I'd appreciate it, and probably others in 
the
future, if you didn't try to push for seeing a solution before an agreement on 
the
design is worked out.

As for Olof, there was a reason why I simply did not call it getTime() and made 
it an
unsigned int. And that is because I feel that the implementation would be more
flexible if you allow for different timers to be compared against each other, 
as well
as allow for more timer variations than simply calling a getTime() function. If 
no
timer is given to it, then it can still return an adjusted value against an 
internal
timer, if there is one. And the unsigned int makes it so that we're always 
dealing
with a positive value internally with GUIChan, as well as allow for higher tick
count, both of which could be advantageous (just like how getWidth() and 
getHeight()
use unsigned integers).

Also, if it's called getTime(), then my assumption as an implementer would be 
that
GUIChan must take whatever internal time value it's given, and then handle it
accordingly. However, GUIChan can't quite do that without knowing how it's
implemented, nor should it care about how it's implemented. If you'd rather not 
allow
for comparison against other timers, then I'm not sure what an appropriate name 
would
be that wouldn't cause ambiguity in what should get returned. I'll have to 
dither on
that a bit more, if you still want to go that route after giving the code a 
look over.

As for calling update logic(), yeah, that fits a bit more in the GUIChan way of 
doing
things, but the name seems a little clumsy to me in comparison. Perhaps logic() 
could
just call update() in this situation? However, you're the head of the GUIChan
project, along with Finalman, so if that's what you'd like to do then great, I 
can
change that (or you could), but I'll wait on doing it until I see confirmation 
that
you are wanting to go that route.

As for the Timer class, I tried to make it as simplistic as I could see making 
it
without taking away some robustness. The main reason why I got into getting an
abstract version into the GUIChan library in the first place was that I didn't 
see
any logic behind saying that GUIChan has no use for timers internally when it's
already got a few portions which use/need their own internally in order to work 
like
the user expects them to. So to me, abstracting all of the timer functionality 
into
one location, and then making it something that the user of the GUIChan class is
expected to implement on their own seemed reasonable enough, and isn't avoiding 
the
issue like I thought you were proposing, but making it clear that the 
responsibility
lies outside of the library. And the way I implemented it, GUIChan is still time
neutral with the abstract timer class, and isn't really too many more lines to
maintain in order to do it as well. It doesn't even use timers at all unless 
the user
supplies one.

In any case, feel free to look over what's been pushed out in the timer-proposal
branch (a bit prematurely, in my opinion, since the interface wasn't agreed on 
yet),
and feel free to comment on/change it. Either way works for me, since I did 
attempt
to break down the commits into smaller pieces so that each commit could be 
taken as
deemed appropriate.

Oh, and sorry about getting the name wrong. That wasn't intentional.

Original comment by irar...@gmail.com on 8 Jun 2009 at 5:12

[GoogleCodeExporter]

Ok, so as for following up to your idea, Olof, I think that if getTime() was 
instead
called getTicksSinceReset() (I know the name's long. Still trying to figure out 
a
better condensed version), and update() was called reset(), it could then 
perform
similarly to what you were wanting, but still allow for more flexibility with 
timer
implementations (and improve clarity a bit). Logic then, if used, would update 
the
ticks since each reset, while reset() would be a mechanism through which the 
timer
can get set backwards to 0, or whatever internal format the timer might use, 
whether
it's a time stamp, etc. But this then leaves a bit of a hole still in the 
design,
which I'm not quite sure how it'd be best to address without bloating up the 
design
too much, and that'd be that some timers, like counting logic ticks, would be
dependent on knowing what their parent timer's tick is, if there is a parent. 
So, in
this case, you would need to have something like a private getParent() or so, 
as well
as being able to call the logic functions in all of the children timers. 
However, not
all timers use this sort of a design, so this might be redundant in their case.
Still, if it isn't addressed directly in the abstract class, then these types of
timers wouldn't be able to properly update themselves.

Oh, and while I was making a sample implementation, I did run across another 
case
within the ScrollArea which needs timer intervention, but I didn't address it
directly, since the solution to it probably should get discussed first rather 
than
just jumping in and fixing it directly. And that is that when you click on the
scrollbar's empty rectangle space, it also only advances one tick as well. Now, 
this
could be solved simply by continuing to scroll in the same direction as long as 
the
mouse is held (which seems to be a rather common way of handling this), or you 
could
be a bit more sophisticated and handle where it continues to scroll to based on 
where
the mouse is at scroll time. Since this is your library, I'd like to hear what 
you'd
personally like done on that, and then get it handled the way that you decide on
before this issue gets closed.

Original comment by irar...@gmail.com on 9 Jun 2009 at 4:53

[GoogleCodeExporter]

I think you are overcomplicating things. Most APIs has a function similar to
SDL_GetTicks() or GetTickCount() (Win32). Just make the interface contain the 
methods
int getTime() and void logic(). Return an int for easy use with timer 
implementations
(SDL returns an unsigned int, whereas Windows return a signed int). The method 
logic
can do whatever is necessary for a specific timer (should be called during a 
Gui's
logic phase, hence the name logic and not update or something else). I presume 
logic
usually will do nothing.

And the abstract class (which isn't very abstract, it's more of an interface) 
should
be called Timer as with everything else in Guichan. We have Graphics, Input, 
Font and
Widget. Not AbstractGraphics, AbstractInput, AbstractFont nor AbstractWidget.

A DefaultTimer should be added, just as the DefaultFont, which acts just as the
DefaultFont in Widget. If no timer is set, the DefaultTimer is used which simply
returns 0 as its time.

We don't need to be able to compare timers, we just need an integer that is 
updated
frequently at a regular pace. We want things as simple as possible.

And about my name, non Swedes always get it wrong and call me Olaf, so don't 
worry
about it ;)

Original comment by olof.nae...@gmail.com on 10 Jun 2009 at 11:24

[GoogleCodeExporter]

Ok, but I still think there's a good reason for keeping a reset() function in 
the
design so that we can keep tick updating and resetting functions separate, as 
well as
keeping in some way to mark when we've utilized the timer. And as I did think it
over, there probably are a few more simplifications to the design that I can do.

For a timer which requires a global timer or comparisons, I was originally 
thinking
that we'd need a _logic() to be able to send updates to the children timers in
logic(), but after I thought it through, I didn't see a need to any more, since 
the
user would be able to implement one through their own derived classes, and then
implementing their global timer where they need it without the need of GUIChan
intervention.

So, now my proposal is rather similar to your own, but with a reset() function 
in the
Timer class as well, and just to make sure that things aren't ambiguous, here's 
what
I think the Timer class should look like now:

class Timer
{
public:
    virtual void reset() = 0;
    virtual int getTickCount() = 0;
    virtual void logic() {}
};

If this is alright, then I'll go through and make the appropriate changes in 
git, and
the issue can be half resolved (but would be easy to close afterwards as it'd 
only be
dependent on resolving what should be done on clicking on the rectangle area not
covered by the scroll marker).

Original comment by irar...@gmail.com on 10 Jun 2009 at 4:22

[GoogleCodeExporter]

I don't really get the reset function. For instance, I would implement an 
SDLTimer 
like this:

class SDLTimer: public Timer {
public:
     int getTime() { return SDL_GetTicks(); }

};

What should the reset function do?

Perhaps Timer should be called Time as we are not really interested in timer 
capabilities, we are only interested in "an integer that is updated
frequently at a regular pace", to quote myself.

Original comment by olof.nae...@gmail.com on 10 Jun 2009 at 4:31

[GoogleCodeExporter]

Fair enough on SDL_GetTicks(), but you're moving a lot of responsibilities 
outside of
the timer and onto the class which uses it if you do that. I think that all 
timer
functionality should stay within the timer itself, so that all GUIChan needs to 
care
about is whether a specified interval has passed or not. If we're doing what 
you're
saying, then on every GUIChan check for a time, we must do three actions: One, 
get
the time and store it, two, get the time later and store it as well, and then 
three,
take the two times and then compare them to get whether we have gone the 
specified
interval yet or not. This is extremely messy if you don't know how the timer is
supposed to work internally, and imposes only one type of timer on the user, 
instead
of being flexible with the implementer, and letting them map their timer type 
to what
GUIChan expects. If you let the timer handle all of this (which your current 
design
doesn't allow), then you open up yourself to more timer variations.

For instance, someone could come along later and decide that they want to count 
logic
ticks, just as an example (not necessarily a good one, but Bjorn mentioned it 
as well
as me in the documentation). If there's a reset within the base API, then they 
can
use reset() to set it to 0 and then continue ticking away in the logic() 
function.
They'd also avoid from having to override your three check system for their own,
which doesn't correspond well to your system. Put simply, if you provide a way 
of
getting the number of ticks elapsed since the reset, have a way to reset the 
timers,
and can increment the timers, then you've covered all of the behaviors that any 
timer
would use and give the implementer more freedom to do their timers however they 
want
to, rather than forcing one specific behavior on the user without providing 
them a
way to map to what they need, and keep from duplicating comparison code when
comparing times.

I hope that answers your question sufficiently. It's not that I'm trying to 
make this
complex (as yes, there are some simplifications that I will be doing to the next
revision), I'm just trying to provide as much flexibility as possible and keep
duplication of code to a minimum.

Original comment by irar...@gmail.com on 10 Jun 2009 at 5:36