Empty space in an inventory window doesn't fire click events

[morganwillcock]

When enabling the setting "Override built-in inventory window click handling", click events inside the inventory window which are not an inventory item don't seem to be handled anywhere. According the description in the setting:

When the mouse is clicked in an inventory window, on_mouse_click is called rather than using AGS's default processing

...but it seems this is not true unless you are clicking on an inventory item (where events appear using eMouseLeftInv, eMouseMiddleInv or eMouseRightInv). I think this is probably a design oversight, but it means that with the current behaviour you never need a null check when trying to get the clicked item, and there is no direct way to handle a click in an empty space.

The only solution I could find for processing clicks in the empty space, is using on_event and eEventGUIMouseDown, but then you have to do a lot of checks about where the click was, and then poll the mouse to see which button is down.

if (event == eEventGUIMouseDown &&
    data == gInventory.ID &&
    GUIControl.GetAtScreenXY(mouse.x, mouse.y) == invCustom &&
    InventoryItem.GetAtScreenXY(mouse.x, mouse.y) == null &&
    mouse.IsButtonDown(eMouseRight))

I'm suggesting that eMouseLeftInv, eMouseMiddleInv and eMouseRightInv events should appear when the empty window is clicked, as well as when clicking on an inventory item. This would be a change in existing behaviour, so I've labelled as AGS 4.

[ghost]

Currently click on GUI element does not fire on_mouse_click at all, the only exception is a click on inventory item. If clicking on inventory window's background would trigger on_mouse_click as well, then this would make the GUI reaction inconsistent, imho.

Perhaps, an alternative is to add OnClick event for InventoryWindow, or even to all available GUI elements, in a GUIControl base class?

[morganwillcock]

If clicking on inventory window's background would trigger on_mouse_click as well, then this would make the GUI reaction inconsistent

I get your point, but none of the other GUI components have a game option to manually handle their click events.

In an ideal world, I prefer the approach of base objects that do nothing, and then behaviour added on top (so you could attach the clickable ability, which also encapsulates the script, to any type of object). I'm not sure if there has been any discussion on modifying the event / object hierarchy?

[ghost]

In an ideal world, I prefer the approach of base objects that do nothing, and then behaviour added on top (so you could attach the clickable ability, which also encapsulates the script, to any type of object).

Well, this sounds like Component system, AGS has a more old-school class hierarchy.

I'm not sure if there has been any discussion on modifying the event / object hierarchy?

If there were any, perhaps years ago.

[ghost]

I get your point, but none of the other GUI components have a game option to manually handle their click events.

This is true, but there are quite a few things in AGS that had built-in behavior, and at some point engine received ability to override it. This option may have historical reasons, and may even be considered what @AlanDrake calls "legacy features" (perhaps arguably). For example, buttons also have old "automatic" behavior, if you switch ClickAction to "SetCursorMode".

[ghost]

If we are speaking on changing/introducing behavior, I'd even put a question following way: is it convenient to handle a click on inventory item in the generic on_mouse_click at all? Could that be a better idea to create e.g. OnItemClick event handler? Such approach would, for example, allow to have separate handlers for separate InventoryWindows.

[ghost]

Sorry, did not mean to divert this topic and did not plan to post so many comments :) . It's just that personally I always found AGS complicated with inconsistent event handling. So, if it comes to redesigning mouse clicks behavior, I am biased towards introducing more event handlers for each separate case and deprecating eMouse***Inv pseudo-buttons.

Regarding your original suggestion, technically I don't see any problem; from the user's point of view... depends on how often users test Inventory.GetAtScreenXY's return value against null. (TBH I do not remember if I do.)

[morganwillcock]

is it convenient to handle a click on inventory item in the generic on_mouse_click at all?

I would say the key difference there is, if you want to run your code entirely in a script module, you can handle on_mouse_click in the module. If using a variant of OnClick, I think this forces you to use a function in the global script (at least my attempt to use a function from a script module didn't work, although I didn't get an error).

Sorry, did not mean to divert this topic. It's just that personally I always found AGS complicated with inconsistent event handling. So, if it comes to redesigning mouse clicks behavior, I am biased towards introducing more event handlers for each separate case.

No problem. I've also found it quite problematic for the updated templates. The existing templates all relied on IsGamePaused() and then the whole thing falls apart if the user change the automatic pause on a GUI. The only solution seems to be constantly check GUI visibility and try to suppress normal game actions if the GUI is not full screen (in case the cursor can still touch characters/objects/hotspots). Once keyboard shortcuts are added to the mix, the problem gets even worse...

Regarding your original suggestion, technically I don't see any problem; from the user's point of view... depends on how often users test Inventory.GetAtScreenXY's return value against null. TBH I do not remember if I do.

I guess the problem would be where not explicitly checking the button type (i.e. last else block in an if statement) and you suddenly end up running code that wouldn't be run previously.

[ghost]

I would say the key difference there is, if you want to run your code entirely in a script module, you can handle on_mouse_click in the module. If using a variant of OnClick, I think this forces you to use a function in the global script (at least my attempt to use a function from a script module didn't work, although I didn't get an error).

I forgot to reply to this earlier, and recent discussion on forums reminded me of this another, perhaps pretty logical option: add a dedicated callback for clicking on GUI control that may be used in any script, something like: function on_gui_click(GUI *gui, GUIControl *control, MouseButton but);

PS. I am still not certain if that may be considered consistent to hande mouse clicks on items in same function as clicks on the room, maybe there could be even additional callback for that.