[Panel-devel] Patch for KickerTip

[Martin Raum]

Am Freitag, 3. Juni 2005 16:31 schrieb Aaron J. Seigo:
> On Friday 03 June 2005 01:40, Martin Raum wrote:
> > Just move the function updateData() from protected: (it should be in
>
> then the Client will need to check if the tip is currently showing and if
> it is tipping for that Client. the Client should be able to call
> updateDate() (or whatever) whenever it feels like without having to worry
> about those details. that was the entire point of my earlier patch, really
> =)
>
> btw, this doesn't look correct either:
>
> +bool KickerTip::isReasonable()
> +{
> +    return true;
>
> in any case, i'm a big fan of simplicity. the patch you coded up is nice
> and detailed, but i'm not sure the extra data members and code really
> provide something useful over a simply Client::update() method. so
> enlighten me as to the benefits of this patch, e.g. real use cases for
> timed updates, otherwise i'd like to stick with simplicity.

The 'return true' is a misstake, yes. It occurred when I hunted a bug and 
disabled that to ensure this part didn't cause the bug.

In regards of the checking you're probably right. You had to save and check 
the client;  This would blow up the code. I thought of the clock, when I 
wrote the patch, but connecting the update signal with a simple update 
function is probably much better and when ever you want the Tip to be updated 
in a previewed time you can set up a single shot yourself. This will make the 
code more explicite and easier to read.
So I vote for your patch, too. You convinced me.

But another another reason, why I wrote mine, were some statements, which are 
very implicite in KickerTip itself, espacially in regard of the frametimer 
(you will see I renamed it in my code).
In dissolveMask() the code
    else if (m_dissolveSize < 0)
    {
        m_frameTimer.stop();
        m_dissolveDelta = 1;
    }
is wirred, in my opinion. It may lead to the vanishing of the tip, if it has 
to be repainted very often. And time is wasted too, if the dissolveMask is 
used, although the plainMask is enough.
Perhaps one could think of changing at least those parts.

Martin