AGS 4: Make linking events to functions more intuitive

[messengerbag]

Problem
For many years, one of the most common support requests from new AGS users is that they can't get an event handler to run. This is almost always because they've forgotten (or don't realize that they need) to link it to the corresponding event via the event pane. Even experienced users sometimes forget this step, adding delays as they test the game in order to discover and fix the oversight.

There is also an inconsistency in the fact that global event handlers don't need to be linked, but use a predefined name that is called at the right time if it exists.

Suggested change
See previous forum discussion, where several alternative solutions were proposed, including:

• Detecting unlinked functions that match the default event handler names at build time and displaying a warning
• Automatically link event handlers by default, without a need to do so manually
• Marking linked event handlers in the script editor (in the left-hand column), and a warning icon if not linked
• Link event handlers explicitly in script rather than through the event pane, using delegates

Perhaps there are new ideas now. One possible "quick fix" is to simply improve the documentation of this feature/quirk, for example in an FAQ section of the manual.

[ivan-mogilko]

For the reference, note that after #2534 is merged it is possible to have the handler functions in any module (except for room and room object events). The module is selected by the special property per global object (gui controls share one with their parent GUI).

[ericoporto]

The reverse issue is documented in the ticket #1363

Delegates is documented in #1409

Detecting unlinked functions that match the default event handler names at build time and displaying a warning

This feels like something for a linter if we had that - to display the message right in the scintilla component itself, perhaps with a ⚠️symbol in the left bar where there are line numbers and a mouse tooltip on mouse over, and maybe squiggly lines in the line itself.

[ivan-mogilko]

I have a uneasy feeling about implicit automatic linking, because if we implement delegates then linking any event with a function without clear user's instruction may be contradicting to user's intent.

I'd like to mention another option: introduce annotations in script, which may be added before the function and instruct editor to link them. This is not necessarily the only solution, but maybe some will like to have.

If such annotations are added when you link a function from the editor, and are present in the template's script, then users copying the existing functions will notice annotations and may figure out to edit them to match the event.

For a quick example, I mean something similar to:

/// [cRoger:Lookat]
function cRoger_Lookat()
{
}

or using # directive:

#event [cRoger:Lookat]
function cRoger_Lookat()
{
}

[AlanDrake]

I've always been told that annotations are evil.
Though you are right to be uneasy, the source of uneasiness in the first place was that users had to manually associate functions, which likely led them to create the wrong type of event and later relink it elsewhere.

Were there no need to link manually from the editor, a user could simply name or rename the function event without extra passages.

[ivan-mogilko]

And how do you link same function to multiple events without manual linking?

I mean, either way you have to tell which function connects to which event, would that be through delegate system, annotations or UI. UI is just another way to do this. IMO annotations in code are a workaround in case delegates do not exist (although I'd imagine that they may stay useful even if delegates are supported).

EDIT: I think that the problem of linking from the editor's pane is not so much that it exists, but that it exists on its own, not reflecting the code.
If you take C# WinForms, for instance, their events pane reflect the events linking code in WindowName.Designer.cs.
I think my proposal is to eventually come to the similar thing.

[AlanDrake]

And how do you link same function to multiple events without manual linking?

By using the editor. I just think defaults that are following the naming convention should sort themselves out without extra burden.

[ivan-mogilko]

If the delegates are implemented, the events become dynamic entities, meaning they may be linked and unlinked at runtime.
In my opinion, in such situation implicit automated linking by matching a function name only becomes a contradiction to the delegate system, and a potential problem.

I also furthermore believe that the editor UI should reflect the code, meaning that events pane only shows the connections specified in the code itself. Meaning, there has to be ways to designate such connection in the code.

I've always been told that annotations are evil.

What are the arguments for this?

[AlanDrake]

What are the arguments for this statement?

The same old "things that are not code, changing the behavior of code", better you use the directive version if you really have to do it.
The annoyance is that it becomes extra boilerplate (you end up writing things twice in most cases).

[messengerbag]

I think the biggest problem is that the process is fairly unintuitive for new users, and that even doing it doesn't guarantee that users form the right mental model/understanding of how it works (in part because clicking to create an event handler moves the focus over to the script). For example, one user today thought that clicking in the event pane was just a shortcut to generating the function header in the script, not realizing that it is auto-filling the field in the event pane that is the important part.

Improving efficiency/convenience is very much a secondary concern, IMO.

To me, that suggests two alternative paths: Changing the model of how this works completely, to something that is more intuitive. Or assisting users to avoid the mistake and understand what needs to be done.

Making event functions auto-link if they match the default name would be an example of the first approach, but IMO this has a number of drawbacks. A major one is that if you still retain the ability to link functions manually, the way it works becomes less understandable, since now you have even more inconsistency.

Using warnings is an example of the second approach: stick with the way it works currently (for now), but help users to use it correctly. I think that's a better approach.

[messengerbag]

Would it be possible to have annotations as the in-script manifestation of the event property fields? IOW, whenever there is an event annotation to a function, the event field has the function name, and vice versa: the two are guaranteed to always be in sync.

If so, I can see that being fairly intuitive: the association is explicit in the script, so someone who copies an example can see it and include it. At the same time, we preserve the ability to view it from the other perspective in the event pane, and to auto-generate it with a click. (Will have to think about what the annotation should look like if the same function is linked with multiple events.)

This could even be extended to the global event handlers, so that they would no longer rely on "magic names." (That's a pretty major change, though, which needs careful consideration.)

Edit: I see that ivan edited his comment.

I think that the problem of linking from the editor's pane is not so much that it exists, but that it exists on its own, not reflecting the code.
If you take C# WinForms, for instance, their events pane reflect the events linking code in WindowName.Designer.cs.
I think my proposal is to eventually come to the similar thing.

Yes, exactly. The problem is that the link is currently "hidden"; if it is reflected in the code, a lot of the problem goes away. And if you can also just write it directly into the code, that can be more convenient in some cases.

In fact, it might open up a way for module authors to hook into events more easily.

[ericoporto]

Just a note that in Winforms, if you delete the code but keep the it in the properties menu VS designer crashes and this is still true on VS2022. They appear to be using the code to fill the properties but there's some illusion going there. Actual implementation of this in ags editor interface without code generation in a separate file (if the event += func was visible in script editor) would require a very good AGS script parser even with delegates.

The Kate Editor, which also uses Scintilla has linter support through LSP (#1067). When Atom was a thing I used a generic linter on top of the AGS Script treesitter grammar from edmundito and it picked up a few coding issues, so I guess there's some way to port something from looking at these two editors somehow - not sure how.

My perception is to either warn through compiler or perhaps better to do it in the editor directly through a linter - the obvious issue with the linter is that our Editor doesn't support/has one, so it isn't easy to "just add a new rule", and even in this case it would require some table of lateral information to be able to have it pick up if the editor has linked or not the function.

[ivan-mogilko]

There's another possible situation, which I witnessed recently, where user first created and linked these functions correctly, then renamed the Hotspot's name, and decided to also rename function names alongside, but did not change their names in the table.

I suppose that the Editor could check if functions are connected, and if function names match certain pattern (with object's name in them), and if they do, then rename them automatically when the object's name changes.

[messengerbag]

If I understand you correctly, @ivan-mogilko, you're saying that when you change the name of entities in the editor (Characters, Hotspots, etc.), the IDE would check if there are any linked events for that entity, and if there are and their names contain the old entity name, automatically replace that substring with the new name? I guess that would be a nice accelerator feature, but I would probably ask the user for confirmation rather than make it completely automatic.

If it's not done and the user renames the function manually without changing the event property (or annotation, if those are introduced), there will be a broken reference in the event pane, and in that case I think the IDE should show a warning or error at build time. (Or in principle even as a "live" project status notification while editing; but that would probably require a completely different approach.) Maybe it already does so?

[ivan-mogilko]

there will be a broken reference in the event pane, and in that case I think the IDE should show a warning or error at build time.

Right, showing error at build time could help too, and it must not be too hard to do.

Now I remember that we already have an open issue for this: #1363

[ericoporto]

The warning messages mentioned were done in #2556 . I noticed double clicking on them doesn't show the pane that you would go to either delete the dangling event or jump from to a script (through the three dots property grid)

[ivan-mogilko]

I noticed double clicking on them doesn't show the pane that you would go to either delete the dangling event or jump from to a script (through the three dots property grid)

Mmm... this would have to be researched. In theory that would require at least attaching an object reference to the message, but that likely should not be actual memory reference, but some key (type name, script name, etc). That's because we cannot predict whether the object will be still available in memory, or available at all in the project, when user clicks on the message.

[ericoporto]

I think only a string name of the object would be fine, there is a version of going to things pane that uses the type name that I used in the Go To Definition that was component based, but in theory all the data is too in the Project Explorer Tree. Selecting the right node could be done by name as string in it. (A naive code for it)

[ivan-mogilko]

Right, objects must have unique name in the same namespace, so maybe type is not needed.
But it also will have to have Room index in case it's inside a room, and parent's name for GUI controls.

[ericoporto]

If there was a type of error message for missing handlers they could hold this information in them so these could be recovered from the entries in the output panel.