Feature - Implemented suppress CLI output for set_property() when setting properties names "_tempXXXXX"

[rlbond86]

suppress CLI output for set_property() when setting properties names "_tempXXXXX"

I know that "quiet mode" when setting KoLmafia properties has been rejected. However, some of my scripts use properties to communicate and it is awfully spammy in the CLI. I was wondering if it would be possible to allow suppression of CLI clutter when using a property that, for example, starts with the name "_temp". This way it would address the issues of poorly-written or malicious scripts but still allow a way to avoid spurious CLI output.

 

[lostcalpolydude]

Since _templeHiddenPower and _tempuraAirUsed exist, it couldn't be that simple, though maybe some other term could be found.

You could use map_to_file() and file_to_map() to avoid lots of CLI output, right?

 

[rlbond86]

Since _templeHiddenPower and _tempuraAirUsed exist, it couldn't be that simple, though maybe some other term could be found.

Sure, I mean there is always something like _tmp or _temporary or whatever.

You could use map_to_file() and file_to_map() to avoid lots of CLI output, right?

I am a bit concerned about this though. The script outputs multiple times per execution and runs quite often, wouldn't it be kind of "bad" to be writing to disk hundreds or thousands of times?

 

[fronobulax]

Embrace the clutter?

To the extent that I agree with the technical and philosophical positions that resulted in the earlier rejection, I don't see much difference between this request and that one.

Now the clutter that I would like to see addressed is all of the preferences that script authors have stuck in user_prefs and GLOBAL_prefs that are no longer relevant to me because I no longer use the owning script. If there were a programmatic way to distinguish between KoLmafia generated preferences and preferences created by scripts then I would support an option to suppress some of the reporting when a created preference was set or changed. But there does not seem to be a groundswell of enthusiasm for my vision of preferences so I don't expect it will happen unless I get time and motivation to do it myself. A naming convention seems to me to perpetuate the problem that I would see fixed. IMO. YMMV.

 

[lostcalpolydude]

I am a bit concerned about this though. The script outputs multiple times per execution and runs quite often, wouldn't it be kind of "bad" to be writing to disk hundreds or thousands of times?

For many people (since it's the default value for saveSettingsOnSet), every set_property() call results in writing to disk.

 

[roippi]

Embrace the clutter?

To the extent that I agree with the technical and philosophical positions that resulted in the earlier rejection, I don't see much difference between this request and that one.

Now the clutter that I would like to see addressed is all of the preferences that script authors have stuck in user_prefs and GLOBAL_prefs that are no longer relevant to me because I no longer use the owning script. If there were a programmatic way to distinguish between KoLmafia generated preferences and preferences created by scripts then I would support an option to suppress some of the reporting when a created preference was set or changed. But there does not seem to be a groundswell of enthusiasm for my vision of preferences so I don't expect it will happen unless I get time and motivation to do it myself. A naming convention seems to me to perpetuate the problem that I would see fixed. IMO. YMMV.

Since user-created prefs that begin with _ are deleted at rollover, there's not much clutter potential from them.

 

[lostcalpolydude]

I thought they were just set to an empty value. There's no difference for someone writing a script, but it's still a line in the _prefs file.

 

[Veracity]

Correct.

I could have sworn that I once proposed an "unset_property" which would completely nuke a property unless it had an entry in defaults.txt. If we additionally had a function to return a map of all properties -> current value and a function to return a map of all defaults -> default, you could easily nuke non-built-in properties.

I have wanted this, now and then, since we have retired or renamed properties ourself.

 

[roippi]

Hmm. I was pretty sure that it fully deleted the line for _prefs, but I see some evidence in my prefs file that says otherwise. I'd say it should delete the line, in any case.

 

[lostcalpolydude]

I could have sworn that I once proposed an "unset_property" which would completely nuke a property unless it had an entry in defaults.txt. If we additionally had a function to return a map of all properties -> current value and a function to return a map of all defaults -> default, you could easily nuke non-built-in properties.

I wrote the code for an unset command and posted it over on the dev forum in response to your list, long before I had commit access. I doubt it's any harder to incorporate that command into mafia's existing code now. I had it reset built-in settings to their default and no way to tell if it was a built-in setting.

 

[fronobulax]

Correct.

I could have sworn that I once proposed an "unset_property" which would completely nuke a property unless it had an entry in defaults.txt. If we additionally had a function to return a map of all properties -> current value and a function to return a map of all defaults -> default, you could easily nuke non-built-in properties.

I have wanted this, now and then, since we have retired or renamed properties ourself.

I was doing this at a command line with awk, sed and a couple other command line utilities because I am barely literate in perl. More trouble than it was ultimately worth because it was a solution that only worked for me. I like your idea much, much better.

On the _ preferences - I don't have the code in front of me but I thought they were reset to something known and not necessarily deleted because some scripts tested for the reset value and not for presence/absence. Somewhat of a tangential point since there are scripts that created preferences that don't use that convention and those are part of the clutter.

 

[lostcalpolydude]

The function to report all of your settings would need a check for isUserEditable(), and maybe it would filter out all those [Window]Frame settings too.

 

[roippi]

So how about

- setting any preference that begins with _ and is not defined in defaults.txt does not print to CLI
- preferences that begin with _ and are not defined in defaults.txt are fully erased on rollover rather than just set to ""

 

[Bale]

Actually, that sounds awesome.

 

[Winterbay]

What would the check

get_property("_userDefinedSetting") == ""

generate if the setting was deleted on rollover? True? False? Would it also spontaneously generate said setting?

 

[fronobulax]

So how about

- setting any preference that begins with _ and is not defined in defaults.txt does not print to CLI
- preferences that begin with _ and are not defined in defaults.txt are fully erased on rollover rather than just set to ""

People tell me I remember things that never happened and this may be one of those. I seem to recall a user script or two that was in use by more than one person that created a user variable that started with _ and relied on mafia to reset it to zero "at rollover" and the script used it as an integer counter. So what does this change return in the case where an inquiry is made about a preference that does not exist? Depending on the answer it may break a script or two. No big deal but we should be prepared to assist with corrections.

This was driven by a request to suppress clutter when running one's own script. However, even if it is just a script specific preference I am often interested in what someone else's script is doing so I would be most comfortable if there were a way to not suppress the output if desired.

All of the above is somewhat philosophical. I'm not opposed to the suggestion.

 

[Veracity]

If you ask for the value of a preference which does not exist, you get a default value that is indistinguishable to what you get from a preference that does exist and is set to the default value.

> ash get_property( "big_bad_boogers" ).to_int()

Returned: 0

Since scripts cannot tell the difference, it's hard to imagine how they could break.

 

[roippi]

Right. It won't break anything because of the way ash already works. _prefs that don't have a default are just reset to "" now, which is coerced into 0 and false via their respective to_x functions.

This should do it, I think:

Index: src/net/sourceforge/kolmafia/preferences/Preferences.java
===================================================================
--- src/net/sourceforge/kolmafia/preferences/Preferences.java	(revision 12105)
+++ src/net/sourceforge/kolmafia/preferences/Preferences.java	(working copy)
@@ -83,6 +83,8 @@
 	private static final HashMap globalNames = new HashMap();
 	private static final TreeMap globalValues = new TreeMap();
 	private static File globalPropertiesFile = null;
+	
+	private static final TreeMap defaultsMap = new TreeMap();
 
 	static
 	{
@@ -601,6 +603,9 @@
 				String value = current.length == 2 ? "" : current[ 2 ];
 				desiredMap = map.equals( "global" ) ? Preferences.globalNames : Preferences.userNames;
 				desiredMap.put( name, value );
+
+				// Maintain a map of prefs that exist in defaults.txt
+				defaultsMap.put( name, value );
 			}
 		}
 
@@ -744,10 +749,24 @@
 			String name = (String) i.next();
 			if ( name.startsWith( "_" ) )
 			{
+				if ( !Preferences.containsDefault( name ) )
+				{
+					i.remove();
+					if ( Preferences.getBoolean( "saveSettingsOnSet" ) )
+					{
+						Preferences.saveToFile( Preferences.userPropertiesFile, Preferences.userValues );
+					}
+					continue;
+				}
 				String val = (String) Preferences.userNames.get( name );
 				if ( val == null ) val = "";
 				Preferences.setString( name, val );
 			}
 		}
 	}
+
+	public static boolean containsDefault( String key )
+	{
+		return defaultsMap.containsKey( key );
+	}
 }
Index: src/net/sourceforge/kolmafia/textui/command/SetPreferencesCommand.java
===================================================================
--- src/net/sourceforge/kolmafia/textui/command/SetPreferencesCommand.java	(revision 12105)
+++ src/net/sourceforge/kolmafia/textui/command/SetPreferencesCommand.java	(working copy)
@@ -139,7 +139,8 @@
 			return;
 		}
 
-		RequestLogger.printLine( name + " => " + value );
+		if ( !name.startsWith( "_" ) || Preferences.containsDefault( name ) )
+			RequestLogger.printLine( name + " => " + value );
 		Preferences.setString( name, value );
 
 		if ( name.startsWith( "combatHotkey" ) )

 

[roippi]

Let's give r12112 a try. Basically what I posted above but I changed the map to a set because I didn't need the second half of it. And some type safety.

To clarify:

What would the check

get_property("_userDefinedSetting") == ""

generate if the setting was deleted on rollover? True? False?

That statement has always returned true, and continues to do so.

Would it also spontaneously generate said setting?

No.

 

[Winterbay]

Thanks for the clarification. I thought that was the case, but it's always good to be sure