Feature request: some way to hint if a custom property exists

[ericoporto]

Describe the problem
Custom Properties are super useful, but when using them in script modules, they become necessary to be created, even if the module feature using them is optional. So the game will crash until the custom properties are created, even if they aren't used - like a person using my rellax module just for the camera smooth but not the parallax.

Suggested change
A script API that returns either true or false if a custom property exists or not. Unfortunately they are needed to be added individually for each thing right now. (See #1228 )

Alternative approach 1: some macro that gets automatically created if a custom property exists - this would be more work on the Editor, but then I would use #ifdef PROPERTY_NAME_MACRO instead. This would be useful that it could also use the define macro for GetProperty(PROPERTY_NAME_MACRO), which is more reliable than correctly typing the strings (thanks to autocomplete!).

I know this may not be perfect (a property may exist to the wrong thing), but it's enough to work in practice (a person would only create said property with intent to use it, so if it's in the wrong thing, it's alright to crash to tell the dev to adjust!)

I used "hint" in the issue title to clarify that a perfect check isn't necessary for the particulars of custom property.

Alternative approach 2: return false or null if some property doesn't exist, and just log a warn instead of crashing. I like the crashing for this case, because it's quick to diagnose something is wrong, but this is the lowest amount of code changes approach I can think to get the result.

[ivan-mogilko]

There's another way, which was considered before, but not added for some reason (maybe dropped from the mind) is to let add any property at runtime, so schema becomes only a way to set default values. This would allow to add anything at runtime, and make it unnecessary to create custom properties in schema for modules to work, as modules will be able to add their own custom properties in script.

Technically afaik there's nothing that prevents this, as properties are simply dictionaries.

If going this way, there's an option to replace properties API with a Dictionary reference: this was also proposed when dictionaries and sets were added. Dictionaries already have all necessary functions for adding, removing, lookup, checking if key exists, etc.

[ivan-mogilko]

return false or null if some property doesn't exist, and just log a warn instead of crashing. I like the crashing for this case, because it's quick to diagnose something is wrong, but this is the lowest amount of code changes approach I can think to get the result.

According to #1488, engine should stop crashing on incorrect script calls that do not lead to anything fatal, and log warnings instead, so this sounds like a correct thing to do anyway, and a minimal solution to the problem, as you say.

There's one problem with this though, that while Text properties may return null, but Int properties do not have a special "nothing" value, so in their case one would have to rely on warnings.

So, I guess, having an ability to check that property was "set" at all might perhaps be still useful...

[ivan-mogilko]

So, I guess, adding a function to check for a property may make sense until properties API are overhauled into a Dictionary, for example.

Something like HasProperty, or DoesPropertyExist?
There are 5 types that need this:

• Character,
• Object,
• Hotspot,
• InventoryItem,
• Room.

[ericoporto]

About the dictionary, the issue with dictionaries is they are dedicated to strings and custom properties can be int values. So I would delay that - in true most of the time when scripting I need a int,int dict or int set, but that is it's own issue.

Something like HasProperty, or DoesPropertyExist?

PropertyExists () looks alright.

There is no urgency here so it's alright to take time if a dict approach is preferred. This is only something I always have to think for the rellax module. But it's just a small thing to note in it's documentation.

[ivan-mogilko]

the issue with dictionaries is they are dedicated to strings and custom properties can be int values

Custom properties store everything as strings, the GetIntProperty is converting from string to int. This is something that may be replaced with a String.ToInt call, for instance, or a helper method in script that does this checking for a null.

[ericoporto]

Something like HasProperty

So, it's a bit inconsistent depending of the type - hotspot apparently has a generic schema that you can check, but rooms, characters, objects and inventory items you are only checking that specific thing, so if it's at default value or hasn't been set later in script, it will return false.

Example, for characters, I couldn't figure how a static HasProperty would be implemented... There is like game.charProps and play.charProps but neither has information if the property exists for characters in general, only for each specific character? Apparently in GameSetupStruct::read_customprops all values are set whether they are default values or not, so maybe it's just that the code is confusing?

It looks more like an IsPropertySet call, per individual object, because leaving at default value doesn't actually set things to the "dictionary"? Or maybe they do.

The problem is I thought in the schema properties existed separated by type, but this isn't how it actually works, if a property exists in any type, it exists in the schema. Perhaps this means that instead we would need a Game.HasProperty ? But afaict a property that exists but doesn't exists for characters would still crash it?

Here's my initial attempt: https://github.com/ericoporto/ags/tree/add-has-property

This will be a slow process to unwind this all as each properties implementation for each thing is slightly different under the hood.

[ivan-mogilko]

I must clarify this.

First of all, indeed, everything in the engine uses same schema object. At runtime the game does not distinct schemas for object types, like it does in the editor.

Then, for each type of object there are two dictionaries: one with default values and another with current values.

For global types (characters and invitems) default dict is stored in GameSetupStruct, and current dict is stored in GameState.
For room and room elements (room objects and hotspots) default dict is stored in RoomStruct, and current dict is stored in RoomStatus.

When reading property values engine does:

• checks schema for this property
• tries to get "current" value,
• if that is not available, tries to get "default" value.

When writing property value engine does:

• checks schema for this property,
• writes "current" value.

It is supposed to work same way for all objects.

[ericoporto]

The issue is I can't try/catch the GetProperty call, I want a way to do a check that tells me I can do the call safely right away.

There's no way to check if Characters have a certain property. Either it exists globally in the schema or it doesn't, and I need to hope both game.charProps and play.charProps exists for all character - afaict, they are resized to match the number of characters anyhow, but nothing guarantees this is the case, it could be that if no properties exists in the editor those wouldn't exist. So get_int_property gets called with the map of that specific character from both game.charProps (what comes from the editor) and play.charProps (values set at runtime), and the schema default value.

When reading property values engine actually does:

• get dictionary for that specific object for both the editor time values set and runtime values set
  • this means if any of these doesn't exist, we crash! We can't statically check - or we assume character 0 always exist?
• checks schema for this property
• tries to get "current" value, from runtime values set
• if it can't find it in it's dict, it gets the editor time values
• if that is not available either, tries to get "default" value from the schema.

I think I can lie, and just add the property for every element, but they all do the exact same thing and just check the global schema, but I am not sure yet this works.

Overall it's a very weird usage of map, the character id could be smashed together with the property name and then a single dictionary be used, and then things could fail more safely more easily.

I think I can switch the scope here to a IsPropertySetAtRuntime or something with a shorter name and have simpler call, but the original HasProperty idea I had is a bit hard to do due to how different the Engine and the Editor implementations are.

[ivan-mogilko]

AFAIK property dictionaries are always created for all objects when the game is loaded, regardless of whether there are any properties in schema or not. If you want, this may be double checked and guaranteed to be created.

If you are concerned that properties are stored separately, then these dicts may perhaps be moved to respective structs. For instance, charProps moved to CharacterInfo, thus guaranteeing that if character exists, then the property dict exists too. It's just that this may have additional complications in the current codebase, where same structs are shared between Editor and Engine. That's why I'd rather not do that right now, at least until of a struct refactor which I'm currently planning.

Other than that, what is the actual goal here? Is this to check that certain property exists for certain type of object?
Or being able to do what in script?

[ivan-mogilko]

So, to clarify the situation more, right now Getting and Setting property that does not belong to the particular object type is a valid action in AGS. This may seem inconsistent with the Editor's view on this, but it's just a fact, that's how properties were implemented historically.

For example, if you define a property "RoomProp" for Rooms only, and then try to Get that property at runtime from a Character, that will work, and return default value of property from the schema. You may then Set a new value to this property for a character, and that will also work.
EDIT: i checked, and character default dictionaries are NOT filled with def values unless these values are explicitly set in the Editor, and since "non-character" properties won't show up in the editor and cannot be set, then characters will never have these filled.

EDIT 2: There were times when properties were read-only, in which case the situation explained above seems tolerable. But then, around AGS 3.4.0 I added SetProperty, but did not change this schema implementation. I think that made things worse in terms of consistency, but I did not realize back then.

In these circumstances the primary question is whether you want to change this behavior to something more logical (but the consequence would be a change in game's behavior), or you want to do a "property exists" check within current system, where any object can access any property in schema?

It may be also good to keep potential future development of properties in mind; like, how, theoretically, we would want properties to work in the end? This is to make sure that any addition won't move engine into an opposite direction.

---

EDIT 3: Just to remind, one option here is to let any object set any property freely regardless of schema (even if it does not exist at all). In this situation the schema will perhaps be a general source of default values and nothing else. In which case "Has Property?" would mean whether the value was ever set, this way or another (in schema, in default properties, at runtime).
This approach should be taken with some care, as the obvious downside is that this may become more prone to typos in property names.

[ericoporto]

I kinda just wanted to be able to have optional features in a module IF a property existed, but the solution to that is open, I don't know the right approach. I like custom properties because we have a graphical interface for them in the Editor, so setting them can be done without much intrusion on ones codebase, and this also means a module API can be simpler.

The issue is the mismatch between the editor and game data, ideally I should be able to query the schema for the specific property of the specific type being available (maybe even it being int or text, through an enum parameter, but that IS in the game data, so it's not a problem), but I think you are leaning towards a generic dictionary available to all objects, and in this sense there would be no schema at all - so instead of a static call it would be more a per object call like "PropertyIsSet". This is somewhat like in Python when all attributes can be queried/set through a dictionary.

In summary, the issue is more of deciding what to do given everything, I don't know what is best. As always, designing things is hard.

[ivan-mogilko]

Hmm, i might look into this problem again bit later when I have more spare time (maybe after several days).

But in general the decisions should go in this order:

1. What kind of a "check" in API we would want; what does it check for conceptually.
2. What would be a "correct" result of this check in the existing system; in other words, which situation should result in positive and which in negative result.
3. How to implement this.

To give an example: if you take the PropertyIsSet variant, in the current situation there's no distinction between what had "set" the value (schema, default dict, or runtime dict), because GetProperty will return a valid value in any of these cases. (BTW iirc values of runtime dict are not written in saves when they match defaults, for disk space optimization.) So if you'd like to have this in API, then PropertyIsSet would return true simply if there's such property in schema (it's always "set" to something).
If later we decide to support adding any properties without schema (or along with it), then it will have to check runtime dict + schema.

EDIT: In regards to checking schema, adding a "type of object" flag there is possible (it may even be merged with the existing set of flags that tell value type). I'm not sure how well testing for this would work in the situation I described, where calling Get/SetProperty was valid regardless of how property is configured in the editor.

[ericoporto]

In regards to checking schema, adding a "type of object" flag there is possible

First, I am not saying use this, I just wanted to implement something like this to see what happens, so here it is

https://github.com/ericoporto/ags/tree/add-has-property-in-schema

As far as I can tell, the information of the type is not in the struct data, so for this I added a new version of the custom property data that has this. Unfortunately the C# side of it is also done in a less extensive way so that doesn't look pretty.

PropertyIsSet would return true simply if there's such property in schema (it's always "set" to something).

In this situation, I am thinking maybe a simple Game.PropertyExists that would return true if the property exists at all would suffice, and then later at some point some note can be added in the editor that the checkboxes are about not storing information for these - if I understand, the current situation is a property that in the Editor only exists for Rooms can still be set at runtime for an Object.

This being true, my above implementation is NOT a good call.

I will stop writing here for a while as I don't think this issue is that important - specially to use your time on this - it's just something that slightly bothered me, but at the time I hadn't looked at any of the code behind the scenes.

[ivan-mogilko]

https://github.com/ericoporto/ags/tree/add-has-property-in-schema

As far as I can tell, the information of the type is not in the struct data, so for this I added a new version of the custom property data that has this. Unfortunately the C# side of it is also done in a less extensive way so that doesn't look pretty.

Just a note, initially i had an idea to store the object type as flags mixed with the property type, as we do not have alot of types, and less types that support custom properties, and these types do not change. That could work until the properties system is overhauled in some way.

Other than this, the concept of HasProperty as shown above might actually work for the time being.

EDIT: What still bothers me, I'm not entirely certain how this would be used in script modules, is this going to be a condition like

if (Object.HasProperty("x")) {
     n = object.GetProperty("x");
     ...
}

[ericoporto]

More like

if (Object.HasProperty("x")) {
    for(int i=0; i<Room.ObjectCount; i++) {
        n = object[i].GetProperty("x");
        ...
    }
}

Just a note, initially i had an idea to store the object type as flags mixed with the property type

Right, I did it that way in case we eventually have something like a PropertySchema singleton to add types through script, but I guess we can't do that since we don't have a generic way to pass a type into the script API to extract it's name. :/

[ivan-mogilko]

I did not mean specifics of what is happening inside the condition, I meant more, how convenient that would be to have this everywhere?

[ericoporto]

Well, the other idea is it doesn't crash but returns a default value (nullptr or 0) and it warns on the log file, but then in a script module I have to always check it, in case the default value of an int property just so happens to be 0 too - the log can get annoying when doing so in rep exec, but in the future maybe there is a way to just say "don't warn me about X".

(erh, maybe I should just change my module API and avoid using custom properties in it, I just didn't pay as much attention to my module and now apparently a lot of people are using it besides myself, or maybe ags should have some form of smooth camera or parallax or something)

[ivan-mogilko]

I suppose for the future versions I prefer the idea of having property bags without schema in the script (at least not in the current sense), that is where any object may have their own properties individually; and property getters would have "default value" as an argument. I might open a suggestion ticket about this, to start a discussion about this, although this would of course be meant for ags4.

If property check has to be added within 3.*, then it's best to keep things as simple as possible.

[ericoporto]

"default value" as an argument

Tbh that would be perfect, an additional parameter to the right that is DEFAULT value and if the property isn't in the schema it returns that and done. Need to think a bit on this on what are the issues of this approach, but API wise it is a HasProperty that you do in one object with expectation to infer about all objects and the behavior of the API could change to represent the actual thing in a world without property schema.

I prefer the idea of having property bags without schema

Just imagining an editor without a schema, in my head instead of a separate screen, it would be a Collection, right in the Property Pane, and then on right click you can Add Property to this Character, Add Property to All Characters, Remove Property from this Character, Remove Property from all Characters, and once you add, it would add individually on the XML of that character or all characters, and the interface would be in some way it's simple to have it to all objects from components that would support it.

Of course, the good thing about property schema is it can be checked and there may be some niceties to workaround what you lose since there is no type system, like, perhaps there could be some warning through some sort of static analysis if the passed string doesn't belong to a group of strings, but not sure how the machinery of this would be.

There's also the approach to extend the character struct and add real properties there through script generation and have a has property for any property, but this is maybe way to hard. But just thinking freely.