I'm reprogramming some A4 code and after entering in keyboard code, successfully compiling it, it only detects when I press an arrow key once, but doesn't do anything if I hold it down.

Here's a code snippit:

      al_wait_for_event(queue, &event);

      if(event.type == ALLEGRO_EVENT_KEY_DOWN) {
         al_get_keyboard_state(&keys);
         if(event.keyboard.keycode == ALLEGRO_KEY_ESCAPE)
            done = TRUE;
         else if(event.keyboard.keycode == ALLEGRO_KEY_UP && pacman.y > 0) pacman.y--;
         else if(event.keyboard.keycode == ALLEGRO_KEY_DOWN && pacman.y < (h-al_get_bitmap_height(pacman.img))) pacman.y++;
         else if(event.keyboard.keycode == ALLEGRO_KEY_LEFT && pacman.x > 0) pacman.x--;
         else if(event.keyboard.keycode == ALLEGRO_KEY_RIGHT && pacman.x < (w-al_get_bitmap_width(pacman.img))) pacman.x++;
         else if(event.keyboard.keycode == ALLEGRO_KEY_PRINTSCREEN) saveScreenshot();
      }

(using a pacman character in this example, not reprogramming my game though )

Anyhow, I need to tap any arrow keys like mad to get it to move at all. I seen some code that had:

if(event.type == ALLEGRO_EVENT_KEY_DOWN || event.type = ALLEGRO_EVENT_KEY_REPEAT)

but when I tried it I got an error about ALLEGRO_EVENT_KEY_REPEAT being undefined. Was this dropped from Allegro 5, or is there something I am missing? Not 100% certain on how to approach this.

I think you should use ALLEGRO_EVENT_KEY_CHAR and then check event.keyboard.keycode.
The ALLEGRO_EVENT_KEY_CHAR has a keyboard.repeat field, where the ALLEGRO_EVENT_KEY_DOWN field does not and I don't believe you'll get ALLEGRO_EVENT_KEY_DOWN for repeated keys.

Edit -

Well, you're only moving it one pixel at a time for each key press, what did you expect? Don't you want to just set the direction it is moving for each key press?

You have to use ALLEGRO_EVENT_KEY_CHAR if you want to get repeats.

KEY_DOWN and KEY_UP represent a physical key being pressed or released. KEY_CHAR indicates some character has been pressed or repeated.

Note that there is not a 1:1 map between KEY_DOWN and KEY_CHAR. Some characters may take multiple KEY_DOWN events to get triggered. Or one KEY_DOWN could trigger multiple KEY_CHARs (repeats, macros, etc...).

Solution #1: use KEY_CHAR

Solution #2: set a flag when receiving KEY_DOWN and clear it when receiving KEY_UP

Relying on repeating keys isn't generally good practice though. So I'd use #2 if you use events.

Solution #3: use the keyboard state functions instead of keyboard events. This is how Allegro 4 worked.

Assuming you have a timer installed:

There's a tutorial on the wiki (Here) that shows how to install a timer and handle game-logic + drawing.

EDIT: Fixed code.

Small correction to BillyBob's code - al_key_down takes two arguments - an ALLEGRO_KEY_STATE* and an int keycode. Which means you need to also declare an ALLEGRO_KEY_STATE object and use al_get_keyboard_state on it.

Actually I wrote this quickly for a test program I have (pixel perfect collision) and I just needed to move the character (pacman in this test) around until it collides in various ways to see if the codes works.

I just tried setting a flag, and it works, but it seems awful slow at times (I sped up the movement by +2 instead). Especially when drawing rectangles. I draw a rectangle around three bitmaps I am drawing to show the bounding box for collisions and when there is a collision I change the colour of the bounding box from green to red, that really seems to cause a bit of a spike in lag for a moment.

I'm thinking that I may have to ditch the idea of using events altogether or do something differently. This is a pretty simple program.

Thank you, I don't know what I was thinking Code corrected.

Is your code timed, like my example?

How often do you draw, and what rate is your timer set at? A simple program like that should be ridiculously fast. Show more code.

I haven't tried your new code yet. This works, but lags when you collide a little. Probably my collision code is a little slow, but I couldn't imagine it being THIS slow, it was faster before. I am reading and comparing pixels from the colliding bitmaps, so I am wondering if there is something extra I need to do before reading pixels etc...

main.c
~~~~~~

sprite_collide.c
~~~~~~~~~~~~~~~~

sprite.h
~~~~~~~~

sprite_collide.h
~~~~~~~~~~~~~~~~

#ifndef sprite_collide_h
#define sprite_collide_h

typedef struct RGB {
   unsigned char r;
   unsigned char g;
   unsigned char b;
} RGB;

int sprite_collide(sprite *object1, sprite *object2);

#endif

al_error.c
~~~~~~~~~~

// Allegro 5 error message

#include <stdio.h>
#include <allegro5/allegro_native_dialog.h>
#include "al_error.h"

void al_error(const char *func, int line, ALLEGRO_DISPLAY *display, const char *message)
{
   char text[256];

   sprintf(text, "%s(%d): %s", func, line, message);
   al_show_native_message_box(display, "Error", NULL, text,
                              NULL, ALLEGRO_MESSAGEBOX_ERROR);
}

al_error.h
~~~~~~~~~~

#ifndef _al_error_h_
#define _al_error_h_

// usage: error(AT, display, "your message here");
#define AT __func__,__LINE__
   void al_error(const char *func, int line, ALLEGRO_DISPLAY *display, const char *message);

#endif

And the two bitmaps I use:
dot.png: 603494

pacman.png: 603495

There, that's everything.

Now I'll try out your suggestions. I'm using this as a test project to basically get the hang of programming Allegro 5 before I do anything major.

EDIT: note some comments were left in from allegro 4 and no longer apply. Like testing the pixels in each bitmap, I now just add up the R G B values together and test, the transparent colour is black so if there is no pixels other than black, the colour will still be zero. I am sure this could be done better, but it was changed from allegro 4 on the fly. Also wouldn't mind speeding the code up where it checks the overlap.

Outside of your for loops where you get the pixel values with al_get_pixel, you should lock both of your sprite bitmaps with al_lock_bitmap(sprite1->img , ALLEGRO_LOCK_READONLY , al_get_bitmap_format(sprite1->img) , 0);. That should speed up access to the data. Access to video memory is slow, so that may be your problem, but on the other hand it might slow you down your drawing to use memory bitmaps instead. Don't forget to al_unlock_bitmap when you're done reading pixels.

Edit -
If you compare the ALLEGRO_COLOR that you get from al_get_pixel to al_map_rgba(0,0,0,0) it would probably be faster than decomposing the color components. Just declare

ALLEGRO_COLOR trans = al_map_rgba(0,0,0,0);

outside your for loops.

How can I compare the ALLEGRO_COLORs?

I changed pixel1 so that they are ALLEGRO_COLOR and then...

if (pixel1 != trans)

but you can't do that.

"\sprite_collide.c|70|error: invalid operands to binary != (have 'ALLEGRO_COLOR' and 'ALLEGRO_COLOR')|"

or did I misunderstand?

I changed it to:

  for(cy=0; cy < over_height; cy++) {
      for(cx=0; cx < over_width; cx++) {
         // sample a pixel from each object
         pixel1 = al_get_pixel(object1->img, (over_left-object1->x)+cx, (over_top-object1->y)+cy);

         pixel2 = al_get_pixel(object2->img, (over_left-object2->x)+cx, (over_top-object2->y)+cy);

         // compare the two pixels.  BOTH have to contain a colour other than black (transparent)
         // for a collision to be flaged.  IF either pixel is transparnet than no collision is present.
         if((pixel1 != trans) && (pixel2 != trans)) return true;
      }
   }

Seriously, you can't do that? ALLEGRO_COLOR is just a struct, it should just do a member by member comparison.

I tried it myself, and you're right, it doesn't work. Sorry.

If you lock the images with the same int32 format, you could directly compare their values in memory:

((uint32_t *)lock1->data)[x] != ((uint32_t *)lock2->data)[x]

// or 
trans = 0xff000000; /* whatever it is */
((uint32_t *)lock1->data)[x] != trans

Note that the data pointer points to the beginning of image memory, so you would also have to add (y * lock->pitch) to it.

I changed the collision detector so it locks the bitmaps and still adds up the values, but after looking at the struct for ALLEGRO_COLOR I see it has rgba values in it, so that saves a step.

It is running very smooth now, no lags. I suspect locking the bitmaps done it.

Hmmm, I just realized that a simpler
if (sum[0] && sum[1]) return true;
would do.

You forgot to unlock the bitmaps in your return statement and at the end of the loops.

Yeah, I'll do that... although it didn't seem to effect the program.

Also, I noticed that my adding of sums wasn't 100% effective, it didn't detect all collisions, I changed it to:

Which now works perfectly and detects them fine, and skips the step of adding them up which is probably an improvement.

Edit: I notice a tiny hiccup of a lag... not much, but still... I'll keep working on this, and try and implement your ideas for comparing the colours.

Edit2: I wonder if a simple memcmp() would do the trick?

Edit3: Okay, I just tried memcmp() and it seems to work nicely. I don't know if there are any problems with doing it this way, but it seems to be okay.
I updated the code above to show what I have now.

Edit4: Crap, forgot to unlock the bitmaps! <updated>