New Content - Implemented KoL broke our Store Manager on 9/10/2012

lostcalpolydude

Developer
Staff member
I think this might be worth releasing an official version for, this is probably going to cause a bunch of people some grief if they're not on dev builds.

I agree. I'll wait a few days to make sure nothing was missed with this patch and to make sure no new mall changes pop up suddenly.
 

Theraze

Active member
If fixing clan snapshots isn't too painful, adding that in would eliminate non-functioning parts I'm currently aware of in release-mafia... Nice to give the people who hate updating some bonus "Oh, and we fixed this that only one of you actually noticed" too. :)
 
If fixing clan snapshots isn't too painful, adding that in would eliminate non-functioning parts I'm currently aware of in release-mafia... Nice to give the people who hate updating some bonus "Oh, and we fixed this that only one of you actually noticed" too. :)

Well, there's also that problem where when under the effect of a distention pill, eating at Chez Snootie does not remove the effect of the pill... that one never got enhanced.
 

Catch-22

Active member
I found the needed regex change, and then I saw that Catch-22 already posted it.

Well you should check the thread more often, it can be discouraging for people in the community to provide patches for no reason (even though it looks like you used the exact same substring/matcher optimization I did anyway).
 

Veracity

Developer
Staff member
it can be discouraging for people in the community to provide patches for no reason
It can be disappointing to the devs when people in the community provide patches for no reason, too. :)

I tended to be reluctant to look at a patch unless it was for a bug that I already thought was worth fixing, regardless; a provided patch does not automatically make a bug report more interesting or important. In fact, I remember a whole series of patches "helpfully" provided to add features which I did NOT want to see implemented. Giving negative pseudo-item numbers to sushi, for example.
 

Theraze

Active member
Well, they did allow for targetting sushi with ASH commands directly. ;)

Sorry about that (and the slew of weird bug reports it caused from other people who were using them 9 months later) btw.
 

Catch-22

Active member
I tended to be reluctant to look at a patch unless it was for a bug that I already thought was worth fixing.

In my defence, I don't believe I have ever provided a patch for a bug that was not worth fixing.

Regardless of that, there's many different ways the code could've been fixed and I'm certain that lostcalpolydude used the work I had done, because the solutions are exactly the same.

I made the d+ change, and did something with the substring() result.

Doesn't sound like any acknowledgement of anyone else's work, and the commit notes are no different. To me, it almost sounds like taking the credit for ones self.

Optimizations might only come out as a few lines of code in the finished product but it may have taken several different approaches to get to that optimization. It's disappointing when someone simply takes the solution you have provided, tweaks it slightly and credits you for nothing.

All it would've taken was "thanks for your help in nutting this one out" or "I tweaked what you came up with and committed it in 11486". In my opinion, if someone has saved you as little as 5 minutes of work, it's worth taking 5 seconds to type out a "thanks", or am I being my usual unreasonable self again?
 

fronobulax

Developer
Staff member
In my defence, I don't believe I have ever provided a patch for a bug that was not worth fixing.

But you have provided patches for bugs that no one else thought were worth fixing. I think V's point was just because a patch was submitted does not mean it is entitled to be used. And some patches are bad ideas. I've had a few reverted myself ;-)

On the other point, the devs tend to acknowledge the work of others when they use it. When they don't it can be a simple oversight or error on the commit notes or any number of benign reasons implying no disrespect. However there have been cases where the work was done independently and there was no reason to give credit. There is a reason most publishing houses do not accept unsolicited manuscripts, for example. If the most important thing about a patch is receiving credit then it is probably best to communicate one on one with a sympathetic dev rather than post.
 

Catch-22

Active member
If the most important thing about a patch is receiving credit then it is probably best to communicate one on one with a sympathetic dev rather than post.

I'd say it's more important that the community has the fix, but it is nice to have work acknowledged. I do see your point though, thanks.
 

Theraze

Active member
I found the needed regex change, and then I saw that Catch-22 already posted it. I made the d+ change, and did something with the substring() result. 11486.

Well you should check the thread more often, it can be discouraging for people in the community to provide patches for no reason (even though it looks like you used the exact same substring/matcher optimization I did anyway).

All it would've taken was "thanks for your help in nutting this one out" or "I tweaked what you came up with and committed it in 11486". In my opinion, if someone has saved you as little as 5 minutes of work, it's worth taking 5 seconds to type out a "thanks", or am I being my usual unreasonable self again?

As you pointed out, lostcalpolydude had already made and tested his change BEFORE noticing that his work duplicated your own. Since he came up with the solution independently though, crediting you with coming up with the same solution seems... silly. The commit was based on his work, not yours. You ended up saving him 0 minutes of effort, since he wasn't aware he could have saved the effort until it was already spent...

Were the code changes the same? Apparently.
Was he aware you'd made a patch? Nope.
Does that mean he used your work, since the code changes were the same? Nope.

If you post a patch and the devs use it wholesale, they'll probably credit you. If the devs need to mangle it beyond recognition, but used it as inspiration? They may still credit you. If they come up with their own solution? Let them take some credit...
 

Catch-22

Active member
If they come up with their own solution? Let them take some credit...

I agree with you, but you may have missed this part of my post:

I'm certain that lostcalpolydude used the work I had done, because the solutions are exactly the same.

If you believe lostcalpolydude came up with the solution entirely independently, that is for you to decide. I don't believe this to be the case.

I'm happy to take on board what fronobulax has said, though.
 
Last edited:

Theraze

Active member
I'm just taking what he said... which is that he came up with a solution, checked the forum, and saw you'd come up with one as well. He appears to (possibly) have used slyz's d+ change, but your regex appears to have been independently achieved... But anyways, this doesn't really matter. What matters is that the bug is fixed, daily-mafia works again, and users can rejoice. :)
 

lostcalpolydude

Developer
Staff member
The patch did save me a little bit of time on the non-regex change. I have a hard time remembering to attribute changes to the person that wrote them in general, even in cases where I start by looking at a patch and then don't make any changes, so not mentioning it wasn't a statement about how helpful the patch was.
 

roippi

Developer
Doesn't sound like any acknowledgement of anyone else's work, and the commit notes are no different. To me, it almost sounds like taking the credit for ones self.

Sigh. I'm going to be very blunt here. And advance warning, this post is going to be pretty harsh.

There's a very good chance you'd already be a full-fledged dev with commit access if you hadn't had a previous incident like this before where you were whining about not getting credit for submitting a patch. And now you're here doing it again. Well I'll let you know how being a dev works: 95% of the time, nobody thanks you for fixing stuff. You just do it because you like fixing stuff. So you should really suck it up and just get used to that fact - committing something to sourceforge is not "taking credit" for anything.

Moreover, whining like this lessens the chance that devs even look at future patches you submit, much less commit them. If you wonder why some of your patches languish, well there you go.

And all of this sucks, because you are a really talented coder with lots to contribute to mafia, and it's being ruined by whatever this... thing is. So really, cut it out, because there's no need to lose your talents because of meaningless junk like this.
 

fronobulax

Developer
Staff member
Sigh. I'm going to be very blunt here. And advance warning, this post is going to be pretty harsh.

There's a very good chance you'd already be a full-fledged dev with commit access if you hadn't had a previous incident like this before where you were whining about not getting credit for submitting a patch. And now you're here doing it again. Well I'll let you know how being a dev works: 95% of the time, nobody thanks you for fixing stuff. You just do it because you like fixing stuff. So you should really suck it up and just get used to that fact - committing something to sourceforge is not "taking credit" for anything.

Moreover, whining like this lessens the chance that devs even look at future patches you submit, much less commit them. If you wonder why some of your patches languish, well there you go.

And all of this sucks, because you are a really talented coder with lots to contribute to mafia, and it's being ruined by whatever this... thing is. So really, cut it out, because there's no need to lose your talents because of meaningless junk like this.

I agree with everything you say, but in defense of Catch-22 and perhaps others, it has been said that the way to get commit access is to submit patches. Beyond that, it gets fuzzy and I can imagine someone thinking that acknowledged patches are the standard being used, when in reality it is a combination of factors including the quality of the patch and the nature of the interactions with the rest of the devs and community here and elsewhere. Whining about anything is going to be a black mark but if someone is under the misunderstanding that the number of patches is the driving factor, then receiving credit does seem like a reasonable thing to be concerned with.
 

Catch-22

Active member
The patch did save me a little bit of time on the non-regex change. I have a hard time remembering to attribute changes to the person that wrote them in general, even in cases where I start by looking at a patch and then don't make any changes, so not mentioning it wasn't a statement about how helpful the patch was.

I am glad the patch saved you some time. Really, attribution is not my major concern but knowing that my efforts did not go to waste is important to me. It encourages me to provide a patch for the next thing that might go wrong. I know you have credited me in the past for things and I appreciate that.

Well I'll let you know how being a dev works: 95% of the time, nobody thanks you for fixing stuff. You just do it because you like fixing stuff. So you should really suck it up and just get used to that fact - committing something to sourceforge is not "taking credit" for anything.

I, too, enjoy fixing stuff. I guess the difference is when you commit to sourceforge, your name is already attached to it. I think it's good to keep track of who is responsible for what, if something breaks or doesn't quite work properly it's good to have a reference back to who it was who originally wrote that piece of code. I know if something I wrote stopped working, I'd also like to be the one held accountable to that.

Having said that, I do take the rest of what you have said on board and I appreciate you being straight with me. Hopefully next time I can better handle myself.

Edit: In the time of me writing out what was hopefully a well thought out response, Frono posted.

I can imagine someone thinking that acknowledged patches are the standard being used, when in reality it is a combination of factors including the quality of the patch and the nature of the interactions with the rest of the devs and community here and elsewhere.

I think this is also valid, it comes back to what I mentioned above about accountability. It's good to have a record of who is responsible for what for other reasons than having a track record before becoming a dev but yes, it's certainly part of it. Also, I tried not to whine and I didn't intend for it to come across as whining. As you said earlier, perhaps a private message would've been more appropriate.
 
Last edited:

Veracity

Developer
Staff member
Until I committed the "fix all the non-Unix line breaks" revision, I was never able to apply a patch using the patch program. It took years, literally, before I realized that the reason a handful of files patched successfully but most files did not was an issue in line breaks. So, my method for "applying a patch" that somebody provided was to manually insert each chunk into the appropriate place of the appropriate source file. This also allowed me to look closely at the code. It was extremely rare that I didn't want or need to change multiple things - either because there was a better way to do something (or, at least, a way that I preferred), there was something else not accounted for, or there was an outright bug.

After doing that, there was the matter of testing. The purpose of testing is not to "prove that there are no bugs." The purpose of testing is to find bugs. Therefore, a successful test is one in which at least one bug is found. I have had many successful tests on community-submitted code. :) (To be fair, I have had many successful tests on code submitted by fellow devs - as well as myself, obviously.)

By the time all that work is done, I "own" the change - in the sense of understand it enough to further modify it - at least as much as the person who submitted the original patch. More so, probably, since that person has not had a chance yet to look at the changes that I had to make to their code. When I commit the change, it may or may not occur to me to credit the person who submitted the patch. Considering that it required considerable work on my part, what with the manually insertion, and all that, it generally did not save me much time, compared to implementing it myself - especially if I had to do lots of debugging and add lots of code to cover cases that were not considered.

It never occurred to me that "getting your name in lights" and "appearing to take credit for somebody else's work" - as opposed to "getting a fix into the codebase for the benefit of everybody" - were actually overriding concerns of some people. Until Catch-22 started making these posts - and the similar incident from that person, a while ago.

I'm of (at least) two minds about community-submitted patches. Keep in mind that it is NEVER as easy as "run the diff file through the patch program and say 'svn commit'". It takes (took) my time to manually insert the code, look at it, fix and adjust it, add things that were missing, and try it out - and THEN comes the 'svn commit'. Even ignoring the issue I mentioned earlier - changes to the codebase that I do NOT want to go in, either because I reject the feature or a proposed bug fix is not addressing the root cause of the issue (or, even, because the supplied code is simply not up to snuff) - it takes MY TIME to deal with the patch - and, perhaps, I'd prefer to spend my time making some other fix - or not working on KoLmafia at all.

I feel sorry that people like, for example, IronTetsubo and Catch-22 have submitted lots of patches which have languished. That is not necessarily because the code is unworthy. It is almost certainly because no developer has felt like taking the time to do the code insert/check/fix/debug/commit. Remember - it is NOT as simple as "apply patch and commit".

I suppose that is a result of my own iron-fisted hold on the program for many years. I wanted only top quality, functioning, low bug count, code to be submitted. This was at least partly because I, myself, used the program, and I did not want it to be rendered unusable by somebody who might submit bugs and then go to bed, say, leaving it up to me to fix the bugs in order to proceed with my KoL day.

Perhaps that was unreasonable. Perhaps a better approach would be to open commits to anybody and everybody and trust them to stay up and monitor the bug reports after committing a fix and hop to fixing bugs as they are reported.

And then there is that "trust" thing. We've never had any malicious code inserted into the code base. I'd llike to keep it that way. At this point, that probably requires a dev to build up the trust within the community first, before being let free to submit the unvetted code of your choice into the source tree. KoLmafia is Open Source and I expect that if anything suspicious was inserted into it, it would be spotted almost immediately, but it would way preferable to not even approach that bridge, much less cross it.

I have retired from KoL. And, hopefully, have all-but retired from here. I quite like the new team. I think they understand the program well enough to do whatever is needed. And roippi, at least, understands the GUI and Swing BETTER than I ever did. I think the program is is good hands. And it is up to the new team to decide how they want to treat community-submitted patches. lost has admin access to Sourceforge. He can add (or remove) devs.

I will watch with interest - and distance - how this project develops. :)
 

Catch-22

Active member
I will watch with interest - and distance - how this project develops. :)

Veracity, I just want you to know that although we have probably butted heads a bit in the past I always did consider it a privilege to have any of my patches reviewed by you, due to your meticulous attention to detail. I've seen some of what you do on other forums and it is fair to say you are what I'd like to call somewhat of a "seasoned veteran" and I respect that.

I guess this is getting way off-topic, but I'd like it to be known that I do appreciate all the feedback that I get from the devs and the community, even if at times it can come off as abrasive. Being a part of a community driven process is something that I find quite enjoyable.
 
Last edited:
I feel sorry that people like, for example, IronTetsubo and Catch-22 have submitted lots of patches which have languished. That is not necessarily because the code is unworthy. It is almost certainly because no developer has felt like taking the time to do the code insert/check/fix/debug/commit. Remember - it is NOT as simple as "apply patch and commit".

I have studiously avoided this thread, but since my name popped up, I will chime in - I'm glad I've been able to help enough that I'm "on the radar", and I also totally understand and appreciate the time investment on the various dev's parts to review, test, and accept the patches we all do submit. We all have different reasons for submitting patches - my own personal one is because a. I need kolmafia to let me spend less time playing kol, and b. because I enjoy the "meta" game of solving this incredibly complex puzzle that is the intersection of kol & kolmafia. My reward comes in when my patch gets committed, because it means I "solved" whatever that particular problem was, and actually got it (vs. my usual state of bouncing back and forth between various functions scratching my head on what the heck's going on). I've had patches with credit, and patches without - for me, personally, it's not a big deal one way or another because I expect that even in cases where I don't get "credit", whatever dev reviewed and committed in will, in the long run, remember and be ever so slightly more willing to invest their precious time reviewing the next patch/bug. Well, hopefully - I'm sure there's also been times where trying to understand my "logic" made the problem harder to solve as well.

Anyways - enough rambling. Thanks for all the hard work, devs! Please keep it up, we all very much appreciate it.
 

Partyhat

New member
Thanks for the info IceColdFever, time to load up my store and test it out.

Edit: Okay, the new page basically splits the reprice fields in to 100 items, so the first 100 items have the form fields price0 and limit0, the next 100 are price1, limit1, etc. This shouldn't be too hard to fix.

Something else that came up is that I noticed when you have a lot of items in the store the store inventory request takes a really really long time (potentially manifesting itself as a "freeze" for impatient folk), I am guessing this is due to the lookahead in the matcher, so I'll try to fix that too.

Edit 2: Here's the patch. The previous issue I had of not being able to reprice certain things was due to me telling KoLmafia not to price things at minimum sell price, d'oh! This patch updates the store manager to use the new behaviour of splitting reprice requests per 100 items. I also optimized the way manageprices.php was being parsed so now the frame should load noticeably faster when you have a bunch of stuff in your store (ie. >100 items), but it's also faster generally.

I am new to this type of thing, just curious as to how I can apply the patch file you have provided us with. If you can assist me I'd be grateful

cheers
 
Top