adventuregamestudio / ags

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

Deprecate 'function' keyword #2463

Open messengerbag opened 4 months ago

messengerbag commented 4 months ago

Describe the problem The function keyword is a legacy of ancient AGS scripting. Many of the default functions that are called by the engine use it as their return type. Because it is implemented as an alias of int but is used even if the function doesn't actually return anything, it means that AGS script cannot properly distinguish int and void return types.

It also creates inconsistency, with two/three different ways to write the same function. This arguably creates confusion and makes the AGS scripting language harder to learn.

Suggested change Remove function as a keyword. Change all function definitions to use int or void as appropriate, and make sure the editor uses these when creating event handlers. This also requires an update to the manual and to the templates.

Optionally, update the compiler to require a correct return type (do not allow a bare return; if the return type is declared as int, or a return 1; if the return type is declared as void).

Optionally, add a backwards script compatibility option to retain function as a keyword (still as an alias for int). (Note that if the other optional part is implemented, this would still require users to change all functions that need to have void as their return type, unless that is also disabled if the script compatibility option is set.)

Alternative suggestion Alternatively, don't remove function as a keyword, but change all the built-in functions to either int or void as appropriate (including in the manual and templates), so that function never appears unless you specifically add it.

AlanDrake commented 4 months ago

I guess I'll go check the manual for old examples with function.

EDIT: There are are 147 references, of which most are event handlers. Do we want to start steering users away from function right now? If so, I could start with references that aren't event handlers, since those are still created as function xxx() by the editor and writing them differently in the manual could be confusing, unless we're also planning to update 3.x soon to discourage the legacy keyword.

ericoporto commented 4 months ago

About ags3 , check https://github.com/adventuregamestudio/ags/issues/1331#issuecomment-877709952

I think void in ags3 is actually int for compatibility because some old games returned something in a void function and it was kept to avoid breaking old games.

messengerbag commented 4 months ago

Yes, I thought so. That's part of why I think AGS 4.0 is a good opportunity to fix it.

ivan-mogilko commented 4 months ago

I think void in ags3 is actually int for compatibility because some old games returned something in a void function and it was kept to avoid breaking old games.

To clarify, today void is a valid keyword in compiler and not an int alias, but the engine "secretly" still returns a integer '0' from the functions of void type. This is done because old API was not declared as void but as function, even though it did not return anything meaningful, and unfortunately some users have used this return value by mistake.

fernewelten commented 2 months ago

I think void in ags3 is actually int for compatibility

The old compiler returns 0 in the AX register for void functions IIRC.

The new compiler does not do this: When a function is void then the compiler doesn't load the AX register with anything when the function returns, either by return or by falling through to the end of the function body.

The new compiler already balks when it encounters return …; in a void function. → Cannot return a value from a 'void' function.

fernewelten commented 2 months ago

The header file for AGS code has #define function int. In my opinion, that's a kludge in and of itself. It allows absurd code such as:

function test(function param)
{
    function i = 5, j = 5;
    for (function k = 0; k < j; k++)
        i += k;
}
ivan-mogilko commented 2 months ago

I think there are two reasonable paths:

If we want to keep "function" as a valid keyword, then it must be implemented in compiler as a keyword, with the rule to be only used in function declaration/definition.

If we want to stop using "function", then:

  1. In standard api header this macro declaration may be hidden under "#ifdef SCRIPT_COMPAT_v399" (for instance). This will disable it in ags4, but still let users to enable if they switch compatibility level.
  2. Replace "function" with "void" or "int" as appropriate for all the standard api declarations.
  3. Replace these in the ags4 templates.
  4. Replace these in the manual (for ags4 only? idk).
AlanDrake commented 2 months ago

When I see a function without extra type definition syntaxes, I expect that it can return any type. I guess this could be achieved by hunting for return statements and resolving their type during compile, but unless we actually do that it may be better to remove it from ags4.

For 4. and the current manual, I could replace function from entries that aren't explicitly generated by the editor. I actually have this change already prepared in case it get greenlighted.

messengerbag commented 2 months ago

I think there are two reasonable paths:

If we want to keep "function" as a valid keyword, then it must be implemented in compiler as a keyword, with the rule to be only used in function declaration/definition.

If we want to stop using "function", then:

1. In standard api header this macro declaration may be hidden under "#ifdef SCRIPT_COMPAT_v399" (for instance). This will disable it in ags4, but still let users to enable if they switch compatibility level.

2. Replace "function" with "void" or "int" as appropriate for all the standard api declarations.

3. Replace these in the ags4 templates.

4. Replace these in the manual (for ags4 only? idk).

As mentioned in the original issue, it would be possible to start by doing steps 2-4, so that function would remain a valid keyword for the time being, but not used anywhere by default. If there is agreement to move away from it, doing so doesn't depend on anything else.