I think there is a minor inconsistency in how the data sources on the mouse device are handled.

The available axes start counting at 0.
The available buttons start counting at 1.

The code below is a snippet from "mousenu.c"

and the documentation said:
{"name":"602354","src":"\/\/djungxnpq2nug.cloudfront.net\/image\/cache\/d\/b\/db78e00ddbf7cc1b33594eec282a7799.png","w":821,"h":391,"tn":"\/\/djungxnpq2nug.cloudfront.net\/image\/cache\/d\/b\/db78e00ddbf7cc1b33594eec282a7799"}

Now, as I see that, there are two options now.
option 1) fix the code (and the existing examples) to make the first button 0

option 2) add a remark to the documentation to those two functions, which says something like "first axis is 0" / "first (left) button is 1"

Option 2 has the benefit of not destroying any existing code which uses Allegro5 (less intrusive).

I would personally prefer option 1, so that "the first" of whatever collection of thing is always 0.

I'd agree, "mouse_state.buttons & (button - 1)" or "event.button - 1" make no sense. The only thing the currently required -1 is likely to do is cause someone to get their mouse button code wrong at first. On the other hand it's a fairly minor issue and breaks existing mouse button code in a somewhat bad manner (e.g. the A5 examples themselves would crash if 0 can now be returned in "buttons[event.mouse.button-1]").

In any case, we need to add information about what the button field does to the ALLEGRO_EVENT and ALLEGRO_MOUSE_STATE documentation. Like for example say that 1 or 0 (depending on if we change it) is the left button.

I think all index related functions should start at 0, so I would vote to change it. It doesn't return a bitfield, not in that instance. It shouldn't matter that the underlying structure is a bitfield.

ALLEGRO_MOUSE_STATE.buttons is a bitfield, so the buttons are 1, 2, 4, ... So either way there will be differences:

state | func (curr) | func (new)
---------------------------------
  1         1            0
  2         2            1
  4         3            2

Concerning the necessary refactoring of the library itself (and any other code included with Allegro5 inside the SVN), a safe way to catch all the code using the "al_mouse_button_down" function is to rename it temporarily. The compiler will then complain about being unable to find the function wherever it is used. The function can then be changed (to start with 0 for the left button). Each code file/line (identified by compiler) which uses the function can then be adjusted for the changed way of numbering the buttons (without renaming the call to the function though). Then after all the calls to it have been changed, the function can be renamed to its current name.

This does not fix any usercode (not included with Allegro5) of course, but if the release notes for the next version include a well visible hint that that has been changed, users can adjust their code accordingly.

I think the current behavior might be a relic from the old Allegro (pre 5) api, where "mouse_b" was a bitfield (same that is now button in the state structure). In the old Allegro, there was no function to test for any specific mouse button (it had to be done by manually comparing the bits).

In the current code, there are two different ways to test for a button state (which I like, because the user isn't forced to use either the event code or the manual state get/check).

In the event handling code, the user would still have to compare the "button" bitfield to 1, 2, 4 to get which button was pressed, whereas with "al_mouse_button_down", currently 1,2,3 must be passed for the button parameter.

"0" for "no button held down" does not make sense with the event handling code, because there are only the events "ALLEGRO_EVENT_MOUSE_AXES", "ALLEGRO_EVENT_MOUSE_BUTTON_DOWN" and "ALLEGRO_EVENT_MOUSE_BUTTON_UP".
(or in other words, if no button was held down, there is no event, so no button field to inspect)

Obviously, since C arrays start at 0, any index returned should be 0-based.
In this case, I would opt to make the return field a bitfield, also because more than one mouse button could (in principle) be held down. Somewhat unusual, I know.

I'm not actually too worried about introducing bugs due to that in Allegro itself, which mostly records the information and passes it on to the user. I don't think any part of Allegro's internals poll the state-query functions.

You can get an ALLEGRO_EVENT_MOUSE_AXES with the buttons in either state, depending on whether the mouse was moved or dragged. I know, you can keep track of this yourself easily as a user. I would personally prefer a bitfield to tell me which buttons are held down, but again, not a big deal.

But you already have that:

ALLEGRO_MOUSE_STATE m;
al_get_mouse_state(&m);
if (m.buttons & 1) {
  // first button is pressed
}
if (m.buttons & 4) {
  // third button is pressed
}

The function works like:

ALLEGRO_MOUSE_STATE m;
al_get_mouse_state(&m);
if (al_mouse_button_down(&m, 1)) {
  // first button is pressed
}
if (al_mouse_button_down(&m, 3)) {
  // third button is pressed
}

The joystick buttons go from 0 to n-1, don't they?

PS: Should the function be named al_is_mouse_button_down? I don't know how we are handling similar function names elsewhere.

ALLEGRO_EVENT_MOUSE_BUTTON_DOWN - a mouse button was pressed.

Fields:
mouse.x,
mouse.y,
mouse.z,
mouse.button,
mouse.display

I don't remember what it is without looking at it, but I'd assume it was only the individual mouse button pressed. For example, if I did this:

• Press & hold left button

• Press & hold right button

• Release both at same time

I'd assume there would be four events triggered, each with an indexed button (given that the field name is singular.)

If only three events are triggered, then they'd obviously need to be a bitfield.

Personally, I don't care which way the event system works, as long as it is documented correctly...

The button is not a part of the ALLEGRO_EVENT_MOUSE_AXES event fields.
{"name":"602360","src":"\/\/djungxnpq2nug.cloudfront.net\/image\/cache\/1\/5\/15b653af3290c3148a4e442c45d779a8.png","w":578,"h":564,"tn":"\/\/djungxnpq2nug.cloudfront.net\/image\/cache\/1\/5\/15b653af3290c3148a4e442c45d779a8"}

So in the event field "button" is not a bitfield? (see strikethrough in his last post)

If that's so, that's good, as long as it starts counting at 0. Then all that has to be done is have al_mouse_button_down start at 0 as well. (The buttons in the state can still stay a bitfield internally.)

Same here but it would be nice if things were consistent across different functions with similar names and purposes.

edit,append:
Looking through the code, it seems that in the event.button field, 1 is the first mouse button. So that's consistent with 1 being the first button in al_mouse_button_down.

Changing that to 0 in both would result in a lot more changes necessary than I first thought, because each platform code would have to change the function that fills the event structure as well.

The more I think about it, the more I lean towards voting to just adjust the documentation for event.button and al_mouse_button_down.
Make it clearly state that the mouse buttons are 1,2,3.. and everything should be ok.

But then that's inconsistent with the joystick button numbers and the axis numbers..

Yes, I made the mistake of posting too early in the morning.
It would certainly be useful if there was a "buttons" bitfield, in addition to the "button" field (which would still only be defined for button events).

Ah, that gives me a half-decent reason to number mouse buttons from 1: as the "button" field is undefined in axis events, we should fill it with a nonce value that the user can't accidentally use, such as zero. That doesn't explain why joystick buttons are numbered from 0, but anyway...

Maybe we can rename the button field and also add the additional bit-field. That way we fix the issue with existing code (instead of .button there's two new fields). And it would allow to do things in a consistent way now.