On Mon, Oct 20, 2014 at 09:51:28PM +0100, Jonathan Dowland wrote: > Dear Thibaut Girka, > > On Fri, Jul 04, 2014 at 11:44:28PM +0200, Thibaut Girka wrote: > > The X/X/Y test method of squishyball is completely broken. > > The attached patch fixes it and shouldn't break any of the > > other modes. > > Thanks for this, sorry for not replying sooner. I haven't yet wrapped my head > around your change -- but I've passed it on to upstream for comment, and I > have > a test -4 .deb build with it applied. I'm going to hope to consider this > properly prior to the freeze and get that package uploaded if all seems well.
Sorry for the late reply! So, let me explain the bug more properly: Quoting the manpage, “X/X/Y testing is a form of A/B/X testing in which the order of all samples is randomized and the position of the 'X' sample is not known ahead of time to be in the third position. In each trial, the user selects which of sample 1, 2 or 3 is believed to be the sample that is different from the other two.” So, in X/X/Y mode, when presented with the three samples, one has to find the “odd one out” and is not expected to know to which file the sample corresponds to. Now, if you try this mode using two completely different sound files, it will become evident that the results are sometimes off: $ squishyball --xxy --score-display /usr/share/games/wesnoth/1.10/data/core/music/heroes_rite.ogg /usr/share/games/wesnoth/1.10/data/core/music/main_menu.ogg Unfortunately, my proposed patch was wrong too, as it broke A/B trial. In the portion of code I modified, “flip_to” indicates the sample singled out by the user: 1 if he typed “!”, 2 if he typed "@", 3 if he typed “#”. “randomize” and “cchoice” are set by randomize_samples: * In the case of A/B trial, randomize[0] has a random value and randomize[1] is equal to “1 - randomize[0]”. “cchoice” is always 1. This is the case my patch broke, as the new condition “flip_to == choice” would not depend on the randomization anymore. * In the case of A/B/X trial, randomize[0] = 0, randomize[1] = 1 and cchoice is whatever randomize[2] is. In this particular case, randomize[x] == x, for x ∈ [0, 1] so both my patch and the original code had the same behaviour. * In the case of X/X/Y trial, “randomize” contains a random selection of 0 and 1s (with one of the value different from the two others). “cchoice” is set to the index of the “randomize” element that is different from the others. In this case, “randomize[flip_to] == cchoice” is obviously wrong, since in the case cchoice is 2, there is no way to select the correct answer. The fix is then to set “cchoice” to the value of the different element (see attached patch). Best regards, Thibaut Girka.
--- a/main.c +++ b/main.c @@ -222,7 +222,7 @@ r[1] = 1-r[1]; r[2] = 1-r[2]; } - *cchoice = (r[0]==r[1] ? 2 : (r[1]==r[2] ? 0 : 1)); + *cchoice = r[(r[0]==r[1] ? 2 : (r[1]==r[2] ? 0 : 1))]; break; } }
signature.asc
Description: Digital signature