AGS 4: autogenerate Custom Properties as extender attributes in script

[ivan-mogilko]

Issue

Custom Properties are useful, but they are inconvenient to access in scripts. One has to use syntax like object.GetProperty("name", value) and object.SetProperty("name", value). An alternative approach is to define extender methods for the object type, and hide calls to GetProperty/SetProperty inside.

In AGS 4's new compiler we now have a extender attribute support. It works like this:

attribute int AttributeEx(this T);

where T is the name of a struct.

This lets users define the custom properties as attributes and hide Get/Set function calls inside implementation.

See Also: a PR that displays custom properties in Property Grid #2604.

Proposal

Support auto-generating extender attributes for the custom properties, reducing amount of manual work for the user.

1. Add a new option in General Settings -> Compiler section, called like "Generate Custom Properties in script" or similar, enabled by default.
2. If that setting is on, Editor generates a new script module as a game script compilation pre-step. This is done very similar to how Global Variables are generated (see GlobalVariablesComponent.cs).
3. I am not sure where the code itself should be located. There's no Editor Component dedicated to custom properties, but we might as well add one, even if just for this task for now.
4. Generated script files should be called e.g. "_CustomProperties.ash" and "_CustomProperties.asc".
5. In the ash we declare extender attributes, using Custom Property Names as attribute Name (doing a conversion if necessary, see below); the attribute's type is decided from Custom Property's type (bool, int or String). The attributes are declared for selected built-in types, depending on the schema configuration.
6. In the asc file we define get_ and set_ functions for these attributes, which wrap minimal calls to Get/SetProperty methods, e.g.:

   int get_Attribute(this Character) {
       return this.GetProperty("Attribute Name");
   }
   void set_Attribute(this Character, int value) {
        this.SetProperty("Attribute Name", value);
   }
   

7. This script module is then added to the list of scripts to compile, above all else except autogenerated headers (I think it does not matter if this is above or below GlobalVariables generated module).
8. Because Custom Property's Name may contain any symbols (it's a regular string), it has to be converted to the script-compatible symbol. The simplest possible conversion is to remove invalid characters. OTOH we might want to enforce the PascalCase naming style matching builtin properties. But this is open for discussion.
9. The Property Schema dialogs should display future generated script names, for user's convenience.

[messengerbag]

My original off-the-top-of-my-head idea was that this would replace SetProperty/GetProperty (and SetTextProperty/GetTextProperty) entirely, as those would no longer be needed. It didn't occur to me to have both.

One thing to consider: what happens if the generated extender attribute name collides with an existing property?

[ivan-mogilko]

My original off-the-top-of-my-head idea was that this would replace SetProperty/GetProperty (and SetTextProperty/GetTextProperty) entirely, as those would no longer be needed. It didn't occur to me to have both.

I don't exactly see how that would be possible without a big overhaul of this system, unless by "replace" you mean "hide".
This proposal is a help, not replacement, but it's relatively easy to achieve.

EDIT: there may still be a use of accessing custom properties by the name using a string, for example if a game saves this data in a file or provides a console for tester to type property names in.

One thing to consider: what happens if the generated extender attribute name collides with an existing property?

Maybe we could issue a warning and ask to rename this custom property?

[ivan-mogilko]

My original off-the-top-of-my-head idea was that this would replace SetProperty/GetProperty (and SetTextProperty/GetTextProperty) entirely, as those would no longer be needed. It didn't occur to me to have both.

Or do you mean that we only declare these properties in script, but they are implemented in the engine somehow, by generating functions objects, which wrap custom property name, and are assigned to corresponding get_/set_ function names?...

But I'm not sure if the Get/SetTextProperty are not needed anymore. In the past I've been actually thinking to let assign arbitrary properties at runtime, disregarding editor's schema. Then this might work as a free Dictionary attached to an object.
OTOH, this could be a completely separate property of Dictionary type...

Maybe we should begin with the current proposal, since it does not add any long term restrictions, and then discuss bigger change separately...

[messengerbag]

Or do you mean that we only declare these properties in script, but they are implemented in the engine somehow, by generating functions objects, which wrap custom property name, and are assigned to corresponding get_/set_ function names?...

I just meant that I didn't think that get_Attribute and set_Attribute would use GetProperty/SetProperty under the hood. I'm not saying it's a bad way of doing it – in fact I agree that it has a number of benefits – it just wasn't what I originally had in mind. I was thinking more that it would create an array of ints to store the values, or something.

Because Custom Property's Name may contain any symbols (it's a regular string), it has to be converted to the script-compatible symbol. The simplest possible conversion is to remove invalid characters. OTOH we might want to enforce the PascalCase naming style matching builtin properties. But this is open for discussion.

What if the IDE enforced script-compatible property names? (I see you already suggested this for names that collide with existing properties.) I wouldn't enforce PascalCase, though.

[ericoporto]

I like the idea of generating code and I would go that way, but I would put it in the same file that has the global vars. Until we change the bytecode to give a few more bits to the section that has the index of the script module, it's best to be more economic.

[ivan-mogilko]

I like the idea of generating code and I would go that way, but I would put it in the same file that has the global vars. Until we change the bytecode to give a few more bits to the section that has the index of the script module, it's best to be more economic.

Last time I checked, the instance id field may be easily increased, because

• it is only assigned at runtime (linking time), and not serialized,
• it is applied to the opcode, and opcodes are not going to be large numbers.

[ivan-mogilko]

In addition to above, I am currently starting to rewrite a script executor class in the engine; this is mostly in attempt to make it easier to do #1409, but also with intent to get rid of "instance forks", that double the number of occupied instance slots.

[ericoporto]

If that setting is on, Editor generates a new script module as a game script compilation pre-step

That is not only it, it will also need to make a header that is available while writing the code and available to the autocomplete parser somehow

This script module is then added to the list of scripts to compile, above all else except autogenerated headers (I think it does not matter if this is above or below GlobalVariables generated module).

It also need to be on top for the autocomplete parser. Other approach is to have the header part directly in the autogenerated header.

[ivan-mogilko]

That is not only it, it will also need to make a header that is available while writing the code and available to the autocomplete parser somehow

I don't remember this by heart, how are GlobalVariables list processed for autocomplete? I suppose we might use the same method for these properties.

[ericoporto]

Great point, had to read to figure it out but apparently it is very simple, Autocomplete.ConstructCache take a single script and then it just keeps it in cache forever apparently unless you ask about it again. So it is just a call to it with the generated script, whenever something requires it to be regenerated.