Properties

Veracity

Developer
Staff member
Lets talk about properties (or "settings" or "preferences").

They are, essentially, a mapping from string to string.

The "key" is a property name - a string
The "value" is the current value of the property - a string

A given property has a scope

"System" - Created by Java Runtime system and accessed via System.getProperty()
"global" - visible to / applies to all users - accessed via Preferences.getString
"user" - each user has own copy - accessed via Preferences.getString

There are also a handful of weird properties - "per-user-global" where the property is stored in the GLOBAL map as a name from defaults.txt with ".USERNAME" appended. There appear to be exactly three of these:

displayName
getBreakfast
saveState (not visible to gCLI or ASH)

These are done this way because the Login Frame uses them, having loaded GLOBAL prefs, without having to load user prefs.

Those don't seem to actually be accessible; they are in GLOBAL_prefs - and go into the "global" map - but get and prefref don't show the values. I'm not sure why.

Each property is of one of two types

"built-in" - Created & maintained by KoLmafia itself. These can be global or user
"custom" - Created and maintained by a script. These are (currently) always user

Each "built-in" property must be defined in defaults.txt, and therefore has a default value.
A "custom" property has no "default" value; it has whatever value it is given when it is created.

The "prefref" command lists properties and shows:

Name
Value
Default
Scope

It does not directly say whether a property is built-in or custom - although it will say "N/A" for a property that does not appear in defaults.txt.

(I am amused to see that preferences that moved from global to user remain in the global table but are now marked as N/A, since they no longer appear in defaults.txt - and, in fact, KoLmafia will never look at the global values.)

The "get" and "set" CLI commands will retrieve or set the neamed property from whichever map it is in.

The get_property() and set_property() ASH functions do the same. In fact, the latter is implemented in terms of the CLI "set" command. This is because the following all have special behaviors when they are set:

battleAction
customCombatScript
combatHotkey[0-9]
_userMods

And that is the extent of programming support.

I can think of a variety of things I'd like to be able to program.

- I'd like to be able to iterate over a (possibly filtered) list of all properties.
- I'd like to know if a property is built-in or custom, if it is user or global, and what the default is
- I'd like to be able to remove a custom property.

With those facilities, I could write the equivalent of prefref, and I could prune orphaned global properties that have been moved to the user map.

(If we ever decided to allow custom global properties, that last one would not work.)

I see a couple ways to do this.

1) Add a few functions:

boolean [string] get_all_properties( string filter )
boolean [string] get_all_properties( string filter, boolean scope )

the "boolean" is true for global, false for user

boolean remove_property( string name )

returns false (and does nothing) if the property is built-in, otherwise removes the custom property from whichever map it is in

boolean property_is_global( string name )
boolean property_is_custom( string name )
string property_default_value( string name )

2) Alternatively, I could create a "property" data type and make scope, type, and default be proxy fields rather than having those last three functions.

I need the three functions (or proxy fields) for a little project.

The others would be nice for sanity checking defaults.txt and eyeballing whether all seeming "custom" are actually built-in and should be in defaults.txt.

What do you think?
 
This sounds pretty well thought out, I'm guilty of orphaning a number of properties myself so being able to clean them up would be nice. I've also had to do silly things to try to determine if a setting is built-in or not, that I'd rather not do.

I'm on the side of having a property data type because I like having everything organized together but that's just one opinion. There is nothing wrong with having the functions either.
 

heeheehee

Developer
Staff member
Are choiceAdventureXXXX properties considered to be built-in or custom? Should we present those in a consistent manner, since not all are in defaults.txt for obvious reasons?

Is there a reason why delete_property(built_in_property) wouldn't just restore the default value?

I think I'd prefer a record, rather than a new property data type, especially because adding reserved keywords is a sure-fire way to break existing scripts.
 

Veracity

Developer
Staff member
A new data type or a record whose fields include what would otherwise be proxy fields are conceptually equivalent.
I see that RuntimeLbrary has custom records "itemDropRec", "maximizerResults", and "svnInfoRec".
An issue with doing something similar is that it's more memory to build them all at runtime when we return a list of properties, but the info we'd put in them is probably what any program iterating over the returned map would want, anyway.
I'll experiment and see how programs would look with a record vs. functions.

I agree that delete_property() on a custom property should remove it from the table and for a built-in property should reset to default. I also think that we want a second version which lets you specify global vs. user, since I want to be able to delete orphaned properties from the global table, evn if defaults.tt puts them in the user table.

choiceAdventureXXXX properties could all be in defaults.txt; if they are not, the default is "0" - manual control - which is a perfectly usable default. We should probably declare that all such are built-in, even if we don't support them directly. I don't see how that could hurt.
 

Veracity

Developer
Staff member
I decided not to have either a new data type or a new (anonymous) record type. Here's what I'm thinking:

---

In the following, a boolean for "scope" is true for global, false for user.

boolean [string] get_all_properties( string filter )
boolean [string] get_all_properties( string filter, boolean scope )

These return a map from string -> boolean which is property name -> scope
filter is a substring (ignoring case) of the property name.
If filter is "", all properties (in the specified scope) are returned.

boolean property_exists( string property )
boolean property_exists( string property, boolean scope )

These return true if the property exists (in the specified scope) and false otherwise.
All built-in properties always exist (in the scope defined by defaults.txt).
All choiceAdventureXXX properties as considered to be built-in even if they don't appear in defaults.txt and therefore exist.
All custom properties (those not appearing in defaults.txt) may or may not exist in the user scope.

string get_property( string property )
string get_property( string property, boolean scope )

These look up a property (in the specified scope) and return the current value.
Since all choiceAdventureXXX properties as considered to exist, "0" is returned if they are not actually present in the user scope.
If the property is not found (in the specified scope), "" is returned

void set_property( string property, string value )

This sets the value of an existing property (in whichever scope it is in) to the specified value.
If the property did not previous exist, a new custom property is created in the user scope with the specified value

void remove_property( string name )
void remove_property( string name, boolean scope )

If the property is built-in, sets property back to default value.
If the property is custom, removes property from the user scope
The version with the scope is provided to allow removal of built-in properties from the global scope that have subsequently migrated to the user scope

boolean property_is_global( string name )

If the property does not exist, returns false.
If the property is built-in, returns true or false as appropriate
If the property is custom, returns false, as all custom properties are (currently) in the user scope

boolean property_is_custom( string name )

If the property is built-in (in either scope), returns false.
If the property is custom (in the user scope), returns true

string property_default_value( string name )

If the property is built-in, returns the default value.
All choiceAdventureXXX properties as considered to be built-in even if they don't appear in defaults.txt. In that case, they default to "0"
If the property is custom returns ""

I considered adding the following:

void set_property( string property, string value, boolean scope )

This would allow creating a new custom property in the global scope. I am not sure if that is useful. It does complicate the semantics a bit:

- Can a script create a custom property with the same name as a built-in property but put it in the other scope? I say no.
- Can a script create a custom property with the same name as a custom property but put it in the other scope? I don't know.

It is simpler to declare that all custom properties have the user scope, as currently implemented. I'd have to hear a strong argument for the usefulness of custom global properties - as well as a good answer to the questions I posed - before going there.
 

Bale

Minion
If the single parameter version is used, it would return matches for both user and global properties? Or is it just user properties like get_property() is now? I suspect the later.

Is the filter case insensitive like the prefref command? I find that to be a useful trait of prefref and hope filters work that way.

Under no conditions should a script every be able to create a property with the same name as a built-in property under the other scope. Never. It would get too confusing.

Personally I am on the fence about whether it should be legal for the user to create global properties. I also don't know how to answer the situation where the user has identically named custom properties with different scopes.
 

Bale

Minion
string property_default_value( string name )

If the property is built-in, returns the default value.
All choiceAdventureXXX properties as considered to be built-in even if they don't appear in defaults.txt. In that case, they default to "0"
If the property is custom returns ""

What about the following properties? Should they also always be considered built-in?
itemBoughtPerAscensionXXX
lockedItemXXX
skillBurnXXXXX
skillLevelXXX
 

Veracity

Developer
Staff member
What about the following properties? Should they also always be considered built-in?
itemBoughtPerAscensionXXX
lockedItemXXX
skillBurnXXXXX
skillLevelXXX
Near as I can tell all of those are in defaults.txt; unlike choiceAdventureXXXX where only those that we have code to let you configure via the GUI are in defaults.txt. There are only a handful of skills that have "levels" that we keep track of, and so on.
 

Veracity

Developer
Staff member
If the single parameter version is used, it would return matches for both user and global properties? Or is it just user properties like get_property() is now? I suspect the later.
I was suspecting the latter; the "boolean" value of the returned map tells you which map the property is in. However, given that we have some properties that we moved from one map to the other that are (erroneously) in both, perhaps it should just be user properties. In which case, perhaps we don't need the single argument version of the function,

Is the filter case insensitive like the prefref command? I find that to be a useful trait of prefref and hope filters work that way.
filter is a substring (ignoring case) of the property name.
Yes.

Under no conditions should a script every be able to create a property with the same name as a built-in property under the other scope. Never. It would get too confusing.
Yep.

Personally I am on the fence about whether it should be legal for the user to create global properties. I also don't know how to answer the situation where the user has identically named custom properties with different scopes.
I need to hear a compelling use case for user-created global properties.
 

Bale

Minion
Near as I can tell all of those are in defaults.txt; unlike choiceAdventureXXXX where only those that we have code to let you configure via the GUI are in defaults.txt. There are only a handful of skills that have "levels" that we keep track of, and so on.

skillBurnXXXXX is similar to choiceAdventureXXXX since all valid value for XXXXX will affect mafia's function, in this case mana burning. Having them in defaults.txt will only change the default value.
 

fronobulax

Developer
Staff member
Thanks for the work and laying out a design for comments.

I am inclined towards saying that only mafia can create Global Properties. Any property created by a user/script is a user property.

I am inclined to require unique names for created proprieties and creation would fail if the name conflicts with a Global property. I would define uniqueness as being case insensitive. This is a philosophy issue - if I redesigned preferences I would deprecate the distinction between global and user preferences. Sometimes only having one scope is worthwhile.

I wonder about another form of get_property. string get_property(string prop_name, string def_value) If prop_name exists then this returns the value. If it does not exist then it returns def_value (but does not implicitly create anything). This would be useful in the case where the empty string is considered a valid value of a property. This is definitely a convenience function since checking for existence before fetching the property would lead to the same result. It has potential use in a situation where null and the empty string and a non-empty string have different semantics. This case is most likely to occur when ash is manipulating data created elsewhere.
 

zarqon

Well-known member
I also don't see a need for user-created properties in the global scope.

I'm liking where all this seems to be headed. I've had the same properties/settings files for years, and by now I'm quite certain that they contain plenty of extraneous properties, from both old mafia and old scripts. These functions will allow for extremely easy coding of settings file cleanup utilities. Yay!
 

ckb

Minion
Staff member
I also don't see a need for user-created properties in the global scope.

I'm liking where all this seems to be headed. I've had the same properties/settings files for years, and by now I'm quite certain that they contain plenty of extraneous properties, from both old mafia and old scripts. These functions will allow for extremely easy coding of settings file cleanup utilities. Yay!

This. And This.
I endorse this effort and this message.
 

Veracity

Developer
Staff member
skillBurnXXXXX is similar to choiceAdventureXXXX since all valid value for XXXXX will affect mafia's function, in this case mana burning. Having them in defaults.txt will only change the default value.
This is the first I've heard of this. I see a small list of these in defaults.txt with value -100. And I see the following comment in ManaBurnManager:

Code:
			int priority = Preferences.getInteger( "skillBurn" + skillId ) + 100;
			// skillBurnXXXX values offset by 100 so that missing prefs read
			// as 100% by default.
			// All skills that were previously hard-coded as unextendable are
			// now indicated by skillBurnXXXX = -100 in defaults.txt, so they
			// can be overridden if desired.
So, yes - this exists, but there is no GUI to manipulate them, unlike choiceAdventureXXX, where some (almost all) of the ones in defaults.txt are visible in the GUI.

That comment seems it indicate that the default for these should be "" (which reads as "0" when interpreted as an integer), just like choiceAdventureXXXX.
 

Bale

Minion
So, yes - this exists, but there is no GUI to manipulate them, unlike choiceAdventureXXX, where some (almost all) of the ones in defaults.txt are visible in the GUI.

Partly true. When jasonharper made those preference so that mana burning could be user controlled, Jason also wrote a relay script to be the GUI for those preferences: Pyromania.

Edit: Ninja'ed!
 

Veracity

Developer
Staff member
I wonder about another form of get_property. string get_property(string prop_name, string def_value) If prop_name exists then this returns the value. If it does not exist then it returns def_value (but does not implicitly create anything). This would be useful in the case where the empty string is considered a valid value of a property. This is definitely a convenience function since checking for existence before fetching the property would lead to the same result. It has potential use in a situation where null and the empty string and a non-empty string have different semantics. This case is most likely to occur when ash is manipulating data created elsewhere.
Well, the little property library I am pondering has the same concept: providing a default value to custom properties which will be applied if the property does not exist. This would allow your script to define a bunch of user properties which don't have to actually exist unless you want to give them a non-default value. It wasn't intended to allow "" to have a special meaning, although that would be an effect of it.

In ASH, it would be something like this:

Code:
string get_property( string prop, string default )
{
    return property_exists( prop ) ? get_property( prop ) : default;
}
Which doesn't seem like it has to be built-in...
 

Veracity

Developer
Staff member
Here's what I've implemented and am testing:

boolean [string] get_all_properties( string filter, boolean global )

This returns a map from (case insensitive) name to a boolean which is true if the property is supported by KoLmafia itself or false if its for/by user scripts.

The "global" boolean argument says whether to get properties from the GLOBAL prefs file or the NAME prefs file of the current user.

Ideally, everything in "global" prefs is "built-in", but we have pesky obsolete former global properties which either migrated to the user settings or are simply no longer used.

boolean property_exists( string name )

Returns true if the named property exists in either the global or user map, and false otherwise

boolean property_exists( string name, boolean global )

Returns true if the named property exists in the specified map, and false otherwise

boolean property_has_default( string name )

Returns true if the named property in has a default value from defaults.txt. Having such is the definition of whether this is a "built-in" property.

string property_default_value( string name )

Returns the default value (which can be "") for a built-in property, otherwise "".

string get_property( string name )

As before. You can ask for "System.XXXX" to get a property defined by the Java Runtime, otherwise, it will get it from the global or user map as appropriate.

string get_property( string name, boolean global )

This will look up the property in the specific map. It shouldn't be necessary, except for those pesky obsolete globals...

void set_property( string name, string value )

As before. This will invoke the CLI command "set NAME=VALUE", which will invoke any side effects of changing a property value. In fact, some of the things you can "set" with this command - like customCombatScript - aren't actually "properties". I also don't like that the "set" command logs the change to the gCLI for every property that is not a non-built-in "_" property.

void remove_property( string name )

If its a built-in property, resets to default.
Otherwise, removes from the user map.

void remove_property( string name, boolean global )

If its a built-in property, resets to default.
Otherwise, removes from the specified map.

Yet again, the only reason we need this is for obsolete global properties.
 
Last edited:

Veracity

Developer
Staff member
For get_all_properties( ..., false ) - the "user" properties - every "choiceAdventureXXXX" or "skillBurnXXXX" property is tagged as "built-in", whether or not it appears in defaults.txt.

For property_exists( name, false) - the "user properites" - every such property is declared to exist - even if it is not in defaults.txt. If you ask for the default, you'll get "" (which to_int() makes into a 0), which is how KoLmafia itself treats such properties.
 
Top