Re: [Warzone-dev] improved doubleclick input.c patch
The Watermelon schreef:
Anyways,attached the updated patch with the redundant checks removed
and added comments:
Looks better than the previous one, I'll look further at it when I'm
back at home.
One thing I noticed already though:
@@ -116,12 +121,12 @@
for(i=0; iKEY_MAXSCAN; i++)
{
-aKeyState[i] = KEY_UP;
+aKeyState[i] += KEY_UP;
}
for (i = 0; i 6; i++)
{
-aMouseState[i] = KEY_UP;
+aMouseState[i] += KEY_UP;
}
pStartBuffer = pInputBuffer;
Why this?
To quote myself:
In inputInitialise:
a*State[i] = KEY_UP;
is changed to:
a*State[i] += KEY_UP;
Considering that a*State[i] is uninitialized at that time its value is
undefined. Adding to an undefined to number will still have an undefined
number as result, so sticking with the assignment operator is probably better.
--
Giel
signature.asc
Description: OpenPGP digital signature
___
Warzone-dev mailing list
Warzone-dev@gna.org
https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] improved doubleclick input.c patch
Resending this mail, as it seems it never reached its destination (i.e.
this mailinglist.
The Watermelon schreef:
Anyways,attached the updated patch with the redundant checks removed
and added comments:
Looks better than the previous one, I'll look further at it when I'm
back at home.
One thing I noticed already though:
@@ -116,12 +121,12 @@
for(i=0; iKEY_MAXSCAN; i++)
{
-aKeyState[i] = KEY_UP;
+aKeyState[i] += KEY_UP;
}
for (i = 0; i 6; i++)
{
-aMouseState[i] = KEY_UP;
+aMouseState[i] += KEY_UP;
}
pStartBuffer = pInputBuffer;
Why this?
To quote myself:
In inputInitialise:
a*State[i] = KEY_UP;
is changed to:
a*State[i] += KEY_UP;
Considering that a*State[i] is uninitialized at that time its value is
undefined. Adding to an undefined to number will still have an undefined
number as result, so sticking with the assignment operator is probably better.
--
Giel
signature.asc
Description: OpenPGP digital signature
___
Warzone-dev mailing list
Warzone-dev@gna.org
https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] improved doubleclick input.c patch
Am Mittwoch, 18. April 2007 schrieb The Watermelon: Anyways,attached the updated patch with the redundant checks removed and added comments: Question: Why do you zip a 10KB patch? signature.asc Description: This is a digitally signed message part. ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] improved doubleclick input.c patch
On 4/18/07, Dennis Schridde [EMAIL PROTECTED] wrote: Am Mittwoch, 18. April 2007 schrieb The Watermelon: Anyways,attached the updated patch with the redundant checks removed and added comments: Question: Why do you zip a 10KB patch? to protect against potential corruption by email client/server...and I think some of our maillinglist subscribers have 56Kish/bandwidth limited connexion... ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] improved doubleclick input.c patch
Am Dienstag, 17. April 2007 schrieb The Watermelon: replaced the frame timer with SDL Ticks timer,and added a '#define DOUBLECLICK_TIME 300',so 2 singleclicks within 300ms(0.3 sec) will be detected as doubleclick event. Looks ok so far, even though the ifs get rather complex. Any objections? --Dennis signature.asc Description: This is a digitally signed message part. ___ Warzone-dev mailing list Warzone-dev@gna.org https://mail.gna.org/listinfo/warzone-dev
Re: [Warzone-dev] improved doubleclick input.c patch
Dennis Schridde schreef:
Am Dienstag, 17. April 2007 schrieb The Watermelon:
replaced the frame timer with SDL Ticks timer,and added a '#define
DOUBLECLICK_TIME 300',so 2 singleclicks within 300ms(0.3 sec) will be
detected as doubleclick event.
Looks ok so far, even though the ifs get rather complex.
Any objections?
Actually, yes. I do have objections, because it doesn't look OK to me at
all.
In inputInitialise:
a*State[i] = KEY_UP;
is changed to:
a*State[i] += KEY_UP;
Considering that a*State[i] is uninitialized at that time its value is
undefined. Adding to an undefined to number will still have an undefined
number as result, so sticking with the assignment operator is probably
better.
In inputProcessEvent in the SDL_KEYDOWN switch there seems to be a
copy-paste error. There are two identical if-statements for
KEY_RELEASED, the latter should probably be KEY_PRESSRELEASE.
When that is fixed we still have this rather large and bulky range of
if-statements (which IMO are in need of a comment):
if (aKeyState[code] KEY_UP)
{
aKeyState[code] = ~KEY_UP;
}
if (aKeyState[code] KEY_RELEASED)
{
aKeyState[code] = ~KEY_RELEASED;
}
if (aKeyState[code] KEY_PRESSRELEASE)
{
aKeyState[code] = ~KEY_PRESSRELEASE;
}
A one line statement plus comment would probably do better:
// Unset KEY_UP, KEY_RELEASED and KEY_PRESSRELEASE
aKeyState[code] = ~(KEY_UP | KEY_RELEASED | KEY_PRESSRELEASE)
The same goes for another bunch of if-statement sequences which do
nothing more than unset a certain series of bits.
Then at some places you seem to check whether a bit is 1 before setting
it to 0. Simply setting it to 0 unconditionally will have the same
effect but result in less lines of code plus being more readable.
For example, this:
if (aMouseState[event-button.button] KEY_DOUBLECLICK)
{
aMouseState[event-button.button] = ~KEY_DOUBLECLICK;
}
would be exactly the same as:
aMouseState[event-button.button] = ~KEY_DOUBLECLICK;
Also some comments would really, really help, especially in your double
click code (in switch branches SDL_MOUSEBUTTONUP and SDL_MOUSEBUTTONDOWN).
Some questions about your design choices:
* Then lastly why do you use bitmasks at all?
* That is where do you need to have two states or more?
* If it is in the mouse code then why do you pollute the keyboard code
with those bitmasks?
* Why do you set an enum's members to hardcoded values? For bitmasks I
know that much yes. But you could have made a wrapping inline function
like this (pseudocode):
static inline UBYTE bitFromEnum(enumVal)
{
return 1 enumVal;
}
Fact is this patch has very, very hard to read code, which doesn't
improve maintainability of code.
Please bear in mind that I do not mean to be rude at all with this mail.
So don't feel insulted, just motivated to increase code quality.
--
Giel
signature.asc
Description: OpenPGP digital signature
___
Warzone-dev mailing list
Warzone-dev@gna.org
https://mail.gna.org/listinfo/warzone-dev
