adventuregamestudio / ags

AGS editor and engine source code
Other
705 stars 159 forks source link

on_event: expand certain event parameters #2524

Open ivan-mogilko opened 2 months ago

ivan-mogilko commented 2 months ago

2468 task required to add x,y coordinate parameters for on_mouse_click.

But there are also eEventGUIMouseDown and eEventGUIMouseUp events handled by on_event that also may require these. For instance, some of the game templates are handling these, and use mouse.x/mouse.y to know the position, although, since on_event is scheduled, this position may already be different.

Another thing, there's event eEventGameSaved, but there's no way to tell if it was successful or not. Right now the engine simply displays a standard hardcoded message in case a save file failed to open. I think it should not be doing that, and let game developer handle this instead. This may be done by passing an extra parameter along eEventGameSaved, which tells the boolean result of the save operation.

ericoporto commented 1 week ago

on_event currently pass one int as data, so this int could be a 1 or 0 for the eEventGameSaved case. This assumes we only successfully saved or have whatever error - other alternative is 0 is the success and for whatever error we get any other number that corresponds to the specific error, may be a bit confusing at first but you save a not operator in the if clause.

For the eEventGUIMouseUp/Down case is the idea to add a second data parameter to on_event or to pack the two mouse positions in the existing data int - say assume 0-65535 is sufficient and pass as upper and lower half of the current data int.

ivan-mogilko commented 1 week ago

on_event already has 2 ints in 3.6.2 and ags4, after dialog events added.

More could be added without breaking backwards compatibility, as engine supports calling a script callback declared with less parameters. This might be enough for a certain period of time, but for the future it may worth to redesign this into a callback that accepts a struct...

ivan-mogilko commented 1 week ago

By the way, I did not notice this earlier, but eEventGUIMouseDown and eEventGUIMouseUp already use 1 parameter... It is "gui number" which was beyond the mouse.

So for 2 cursor coordinates they would require 3 ints now...

ericoporto commented 1 week ago

for the future it may worth to redesign this into a callback that accepts a struct...

Uhm, GUIControls have that incredible magic that you can do an AsButton, AsLabel, ... which is nice, so let's say an Event struct could have AsMouseEvent, AsDialogEvent and whatever, along with a type enum. It would give a nice API on one hand, OTOH it would require adding quite a few new objects - they could be ScriptUserObjects but I don't know how adding new fields to those would work if we need in the future.

If we instead want to use a single struct, I am not super sure how it would look API wise, perhaps it has a type and unrelated fields are just not set, but that will require looking at the documentation the same way the current "unnamed" int parameters require.

ivan-mogilko commented 5 days ago

other alternative is 0 is the success and for whatever error we get any other number that corresponds to the specific error, may be a bit confusing at first but you save a not operator in the if clause.

We might have a use for "engine error code" enum. I've been thinking about save file prescanning operation, and it might report error code on failure too.

ivan-mogilko commented 3 days ago

Added extra params for eEventGUIMouseDown, eEventGUIMouseUp 3c1c6376ac66f914ae359bdf023b657b9400b9c7

This ended up having 4 total "data" params... (gui, button, x, y) which is bit more than I expected. Hopefully we won't have to add more into this callback prototype, or that will become ugly.