Bug - Fixed Some mafia settings don't actually change unless you change them in the UI

taltamir

taltamir

Member
Most noteable are the settings
Code: 
[COLOR=#333333]breakableHandling 
[/COLOR]currentMood
are not actually changed when changing them via script. only changing them via the mafia UI works.

Take for example currentMood
In the UI I create a mood called test with the contents of
Code: 
Always, !why_is_this_being_run

I use the UI to quickly change moods to show it works
Code: 
Preference currentMood changed from apathetic to slimemom
Preference currentMood changed from slimemom to test

I then run the script
Code: 
set_property("currentMood", "apathetic");
adv1($location[The Arid\, Extra-Dry Desert], -1, "");

Which outputs
Code: 
[COLOR=olive]> call scripts\tt_test.ash[/COLOR]

currentMood => apathetic
Preference currentMood changed from test to apathetic

[COLOR=red]Unable to invoke !why_is_this_being_run[/COLOR]

and typing in CLI
Code: 
[COLOR=olive]> prefref currentMood[/COLOR]

    [TABLE]
[TR]
[TH]          Name[/TH]
[TH]          Value[/TH]
[TH]          Default[/TH]
[TH]          Scope[/TH]
[/TR]
[TR]
[TD]                      currentMood[/TD]
[TD]                      apathetic[/TD]
[TD]                      default[/TD]
[TD]          user[/TD]
[/TR]
[/TABLE]
So the preference was clearly changed to apathetic. mafia just doesn't change it because the change was not done via UI

Incidentally I tried getting a debug log of this but this is all it shows
Code: 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
        KoLmafia v20.3 r20019, Windows 10, Java 1.8.0_251
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Please note: do not post this log in the KoLmafia thread of KoL's
 Gameplay-Discussion forum. If you would like the KoLmafia dev team
 to look at it, please write a bug report at kolmafia.us. Include
 specific information about what you were doing when you made this
 and include this log as an attachment.
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Timestamp: Mon Apr 20 00:46:24 MDT 2020
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=






> call scripts\tt_test.ash


Starting ASH script: tt_test.ash
currentMood => apathetic
Preference currentMood changed from test to apathetic


Unable to invoke !why_is_this_being_run


Finished ASH script: tt_test.ash
 
Last edited:
T

the dictator

Member
For what it's worth, you can change mood with "cli_execute("mood barffarming");", though that doesn't explain why changing currentMood doesn't have the same effect.
 
lostcalpolydude

lostcalpolydude

Developer
Staff member
I think the solution is to add
Code: 
PreferenceListenerRegistry.registerPreferenceListener( "currentMood", this );
somewhere in MoodOptionsPanel, but doing so looks nontrivial.
 
Veracity

Veracity

Developer
Staff member
lostcalpolydude said:
I think the solution is to add
Code: 
PreferenceListenerRegistry.registerPreferenceListener( "currentMood", this );
somewhere in MoodOptionsPanel, but doing so looks nontrivial.
Click to expand...
It is actually easy. Make the combo box a listener, listen on the preference, and when the listener fires, update from preferences.

Code: 
		private class MoodComboBox
			extends JComboBox
			implements Listener
		{
			public MoodComboBox()
			{
				super( MoodManager.getAvailableMoods() );
				this.addActionListener( new MoodComboBoxListener() );
				PreferenceListenerRegistry.registerPreferenceListener( "currentMood", this );
				this.update();
			}

			public void update()
			{
				MoodManager.updateFromPreferences();
			}

			public class MoodComboBoxListener
				implements ActionListener
			{
				public void actionPerformed( final ActionEvent e )
				{
					Mood mood = (Mood) MoodComboBox.this.getSelectedItem();
					if ( mood != null )
					{
						MoodManager.setMood( mood.toString() );
					}
				}
			}
		}
I will submit that, but the core issue is that the "currentMood" property doesn't actually control the mood: MoodManager has this:

Code: 
		String currentMood = Preferences.getString( "currentMood" );
		MoodManager.loadSettings();
		MoodManager.setMood( currentMood );
		MoodManager.saveSettings();
The MoodManager.setMood method sets MoodManager.currentMood, which is what is used, rather than looking up the setting all the time.
The "set" command has special code to handle some properties which behave like that. "battleAction", for example.
ASH "set_property" calls the CLI command specifically for that reason.
I think that needs to handle currentMood specially, too.
I'll try that.

Regarding breakableEquipment, I see no bugs. When you update that in the CLI, the GUI updates, and the only place that uses that (EquipmentManager) always gets the property.
You'll need to explain/demonstrate what you think is not working correctly.
 
Veracity

Veracity

Developer
Staff member
The mood manager is sort of a mess. When you set the mood, it sets the selected item in the mood list - which is what the MoodOptionPanel displays. I don't think that's correct usage of the MVC pattern. But, I think that, as coded, the only change needed is for special handling in the "set" command for the currentMood property.
 
Veracity

Veracity

Developer
Staff member
Yeah. Revision 20025 changes the "set" command (or the set_property function) to tell the MoodManager when you change currentMood.
Not seeing an issue with breakableHandling.

This Bug Report is so vague that I can't tell if this completes it. "Some mafia settings don't actually change unless you change them in the UI" and then, "most notably" two specific properties.
One of those was a bug, the other not, but it looks like you maybe, might, perhaps have other properties in mind that were not "notable"?

If you can show breakableHandling not working correctly, I will look at it.
If you can name SPECIFIC properties that are not working, I will look at them.
Otherwise, I'm just going to close this.
 
Veracity

Veracity

Developer
Staff member
So, why does the "set" command need special handling for certain properties? Why not just have the component which needs updating register a preference listener?

Sure, the MoodOptionsPanel could do that and update its View. But the MoodManager effectively signals it by changing the selected index on the Model. So why not have the MoodManager itseld register a listener? Because the MoodManager is an abstract class and there's not actually an object that can handle an update() signal. Well, that was an implementation decision that, presumably, simplified some things, but necessitated kludges elsewhere.
 
taltamir

taltamir

Member
My apologies about the vagueness. I figured I would add more debugs as needed if it is not a simple fix that solves all of them.
Actually, since they each seem to be a separate issue. You can go ahead and close this and if I make a reliable way to reproduce any other such case I can make a separate thread for it.
 
You must log in or register to reply here.
Top