Feature - Implemented daily deeds revamp [patch]

roippi

roippi

Developer
In brief,

  • Create new preferences: dailyDeedsOptions and dailyDeedsVersion
  • dailyDeedsOptions is an overloaded, comma-delimited global preference that contains the order of all included deeds.
  • Add an API for user-customizable deeds - see here
  • Add a daily deeds tab in preferences.
  • "add custom" button invokes the custom deed builder, a graphical tool to help the user build custom deeds.

Patch is slightly over forum size limitations for .patch files, so file extension is .txt.
 

Attachments

  • ddcustomprefs.txt
    107.7 KB · Views: 43
Veracity

Veracity

Developer
Staff member
I submitted this in Revision 9622, pretty much as is (although I edited manually, rather than having patch fail for me.)

I did not test this. I am assuming that everybody else will test it. Have at it! ;)
 
roippi

roippi

Developer
One of these days I'll figure out how to make a patch that works for people.

For the record, my method which appears to be giving people headaches: using tortoiseSVN, right click on the base SVN directory (kolmafia), select "create patch...", untick .project, add any unversioned files which I've created. Save as something.patch.

I really do wish I were in a *nix environment for stuff like this. So much easier.
 
fronobulax

fronobulax

Developer
Staff member
roippi said:
One of these days I'll figure out how to make a patch that works for people.

For the record, my method which appears to be giving people headaches: using tortoiseSVN, right click on the base SVN directory (kolmafia), select "create patch...", untick .project, add any unversioned files which I've created. Save as something.patch.

I really do wish I were in a *nix environment for stuff like this. So much easier.
Click to expand...

For the record that is what I do except I click on the src directory and don't recall having a .project to untick.
 
Veracity

Veracity

Developer
Staff member
One issue:

On Windows, the line break is ^J^M.
Everywhere else, it is ^J

When my editor opens a file, it looks at the line breaks and conforms. Since I'm on a Mac, new files get ^J, but if I open a file created and saved in Windows, it comes in with ^J^M and my editor preserves that and adds new lines with that line break.

I have noticed that some Windows editors - including yours - take in a file with ^J line breaks and convert them all to ^J^M when you you save the file. That is why quite a few KoLmafia source files have the Windows-style linebreaks; although both hola and I primarily used a Mac in the early days (and I still do), as Windows users with commit access changed this or that file, gradually they got converted.

The problem is that "diff" considers those lines different and when you so-convert a file, it looks like every single line was deleted and replaced by new lines. It is impossible to see what the real differences were.

You did that to PreferenceListenerRegistry.java. I ended up taking in your file, un-munging the Windows line breaks, and re-saving. Having done that, I could finlly see what you'd done to the file. (which I am curious about. I don't understand why you needed to trap and handle the ConcurrentModificationException. Why were you getting that - and was that really the best way to deal with it? If I knew the answer to the former, I might have an opinion about the latter.)

Bottom line: please configure your editor to preserve the style of line-breaks, rather than forcing them to Windows line-breaks.

Thank you.
 
roippi

roippi

Developer
Veracity said:
(which I am curious about. I don't understand why you needed to trap and handle the ConcurrentModificationException. Why were you getting that - and was that really the best way to deal with it? If I knew the answer to the former, I might have an opinion about the latter.)
Click to expand...

If you open up the adventure tab before logging in, a new DailyDeedsPanel is created. There are some bugs that happen when it is populated with items in this way, so I just don't populate the panel when userName == GLOBAL.

I also attach a listener to preference dailyDeedsOptions. This causes the panel to repopulate every time the preference is changed.

The problem is that the preference listener fires during login as the preference is being modified and throws a ConcurrentModificationException. You can see this by putting a RequestLogger print in catch(), loading a frame that contains DailyDeedsPanel before logging in, and then logging in.

I welcome a better way to handle it, but it seemed to me the "optimistic" approach: concurrency happens so rarely, so just try again in the rare cases it happens. I know that there are other ways to do it, such as declaring the method synchronized, but as I understand it that is a great way to slow down a multi-threaded application.

___

Re: line-breaks

Aha! I -could not- figure out why diff was choking on that one file. Thanks, will do in the future.
 
AlbinoRhino

AlbinoRhino

Active member
First, I would like to say ... awesome job ! However, I did run into a small glitch.

When I press the "Help" button on the Daily Deeds tab of the preferences manager, the help pops into a non-scrollable window which is larger than the screen size. The only way to close the window is with Alt+F4 and it appears some of the help text may not be viewable.

Using Windows 7, 64 and r9623.


dd-help.jpg
 
Winterbay

Winterbay

Active member
I decided to try this out today and try to set a preference for Ingio-casting just for testing. I went to Preferences -> Daily Deeds. First I read the help (no problem like the above here) and then I clicked on AddCustom and choose skill whereby I got the following dedbuglog (and mafia got locked in a pending state).
 
Winterbay

Winterbay

Active member
I was planning to create my own (called _InigoCasts or similar) but when I clicked on the "skill"-option only the first three fields were usable and the debuglog was created. After that I tried filling things in but since I coudlnt' set the limit to 5 I decided to restart Mafia and post the log instead.
 
lostcalpolydude

lostcalpolydude

Developer
Staff member
Winterbay said:
I was planning to create my own (called _InigoCasts or similar) but when I clicked on the "skill"-option only the first three fields were usable and the debuglog was created. After that I tried filling things in but since I coudlnt' set the limit to 5 I decided to restart Mafia and post the log instead.
Click to expand...

With an empty new variable _inigoCasts I was unable to create the custom skill deed because it wouldn't validate. Setting it to 0 allowed it to validate. After saving the deed, the Current Deeds list says I have a BooleanItem custom value, and I did it an extra time to make sure I was getting a BooleanItem when trying to add a skill deed.
 
roippi

roippi

Developer
lostcalpolydude said:
With an empty new variable _inigoCasts I was unable to create the custom skill deed because it wouldn't validate. Setting it to 0 allowed it to validate. After saving the deed, the Current Deeds list says I have a BooleanItem custom value, and I did it an extra time to make sure I was getting a BooleanItem when trying to add a skill deed.
Click to expand...

Argle. How did I not catch that. One line fix.

But yes - you must create the preference before the builder will allow you to set it. That's a feature.
 
roippi

roippi

Developer
Line 1426 of AddCustomDeedsPanel is:

Code: 
			String deed = "$CUSTOM|BooleanItem|" + display + "|" + pref;

and should be:

Code: 
			String deed = "$CUSTOM|Skill|" + display + "|" + pref;

Embarrassing.

Winterbay said:
I was planning to create my own (called _InigoCasts or similar) but when I clicked on the "skill"-option only the first three fields were usable and the debuglog was created. After that I tried filling things in but since I coudlnt' set the limit to 5 I decided to restart Mafia and post the log instead.
Click to expand...

Oh - so the debuglog was created on clicking the card selector.

Hm. I've never had that happen. I did not create CardLayoutSelectorPanel, but I'll try to debug.. can you reproduce?
 
Winterbay

Winterbay

Active member
roippi said:
Line 1426 of AddCustomDeedsPanel is:

Code: 
			String deed = "$CUSTOM|BooleanItem|" + display + "|" + pref;

and should be:

Code: 
			String deed = "$CUSTOM|Skill|" + display + "|" + pref;

Embarrassing.



Oh - so the debuglog was created on clicking the card selector.

Hm. I've never had that happen. I did not create CardLayoutSelectorPanel, but I'll try to debug.. can you reproduce?
Click to expand...

Yes it was, but no I can't. No idea why it didn't work then. I'll go and create the preference first this time :)
 
T

Turias

New member
roippi

roippi

Developer
Well, skill is going to be broken until I get a bugfix for that issue that lost just pointed out.

Instead, you can more or less replicate the functionality with the multipref option - just use "cast inigo" as the command.
 
You must log in or register to reply here.
Top