added a comment - - edited
That looks pretty good. I have a bunch of minor complaints, and one serious one, number 6.
> $oldattempts = array_keys(get_records_select( ...
1. can we avoid doing this potentially expensive database query unless it is really necessary?
2. The logic is wrong. The student may have attempted other quizzes, we are only interested in their attempts at this quiz, aren't we?
3. The logic is wrong in another way. You need the quiz_attempt.uniqueid, not quiz_attempt.id in $oldattempts.
> function get_usable_questions_from_category(...)
4. I don't really like passing the whole of $cmoptions in here. I would rather leave $questionsinuse, and add an extra parameter, perhaps $avoidquestionsfromattempts
5. Please use preg, not ereg.
6. You are possibly using two or three database queries where previously there was one. This is bad for performance. Hmm. I see, it is more complex than that because of the $this->catrandoms caching. I guess the code now does the right thing, it is just not very clear.
7. Good idea that get_usable_questions_from_category returns just the question ids, rather than objects with just an ->id field.
So, what to do about 6?
It would be nice to do everything in one database query. The following is my idea. I have not tested it, so it probably does not work. It also needs to be tested for performance, but the idea is worth pursuing.
SELECT q.id, COUNT(qst.id) AS numprevioususes
FROM mdl_question q
LEFT JOIN mdl_question_states qst ON qst.answer LIKE CONCAT('random', q.id, '-%')
WHERE q.category IN ($categorylist)
AND q.parent = 0
AND q.hidden = 0
AND q.id NOT IN ($questionsinuse)
AND qtype NOT IN ($this->excludedqtypes)
AND qst.attempt IN ($avoidquestionsfromattempts)
AND qst.seq_number = 0
GROUP BY q.id
ORDER BY numprevioususes ASC
Do that query with get_records_sql_menu. That will give you an array of questionid => number of times that question has been used in previous attempts, in order of number of attempts.
That way, you can use the first part of the array to get questions that have never been used before, then use the next bit to get questions that have been used once before, and then questions that have been used twice before, and so on.
I hope all that makes sense. Thank you for working on this.