Bug - Fixed lastFriarsNC pref should be reset at ascension start

ckb

Minion
Staff member
The properties lastFriarsElbowNC, lastFriarsHeartNC, lastFriarsNeckNC should be reset to default when starting a new ascension.

The current list in 'resetOnAscension' in Preferences.java is :
"lastFriarElbowNC",
"lastFriarHeartNC",
"lastFriarNeckNC",

without the 's' for Friars
 

fronobulax

Developer
Staff member
Nope. See PR. The code to update the quest in KoLmafia updates the wrong preference when the quest changes and several scripts are also using the wrong name.
 

heeheehee

Developer
Staff member
Does it really matter which name we go with, as long as we're consistent?

r26878 (the change that introduced the preferences in the first place) specified lastFriars... in QuestManager.java and the corresponding tests, but lastFriar... in defaults.txt and Preferences.java. So we actually never used the lastFriar... preferences, in practice.

It would be nice if this were named consistently with lastFriarCeremonyLocation, but changing a preference name is always a challenge.
 

fronobulax

Developer
Staff member
Does it really matter which name we go with, as long as we're consistent?

r26878 (the change that introduced the preferences in the first place) specified lastFriars... in QuestManager.java and the corresponding tests, but lastFriar... in defaults.txt and Preferences.java. So we actually never used the lastFriar... preferences, in practice.

It would be nice if this were named consistently with lastFriarCeremonyLocation, but changing a preference name is always a challenge.

I was in a hurry but I thought the no s name was used elsewhere in mafia and the s version only when tracking the quest. So my solution would be to fix the quest tracking to obtain consistency because there were fewer places to change. If I was wrong or if the s version is in widespread use in scripts then I have no problem with declaring the s version to be the One True Preference and changing the at least two places in mafia that would be inconsistent.

But the problem is inconsistent preference names and not just the list of things that get reset.
 

ckb

Minion
Staff member
I don't have any stake in which option we use.
lastFriar... seems to be the original intention, but lastFriars... is the one that gets used and updated currently in practice. I don't know what other scripts use, and they both would have problems (either not updating or not resetting).

That said, my PR to just change a typo is about the limits of my java and git knowledge. If we need more detail and a new test it is beyond me.
 

fronobulax

Developer
Staff member
Reopened the PR and added a change to defaults so that the "s" version of the preference is the correct/supported one.
 
Top