Skip to content

Better keep track of which sets are complete in achievements.#2964

Open
somiaj wants to merge 1 commit intoopenwebwork:WeBWorK-2.21from
somiaj:add-completed-set-ids
Open

Better keep track of which sets are complete in achievements.#2964
somiaj wants to merge 1 commit intoopenwebwork:WeBWorK-2.21from
somiaj:add-completed-set-ids

Conversation

@somiaj
Copy link
Copy Markdown
Contributor

@somiaj somiaj commented May 2, 2026

Store the setID of all completed sets in the globalData hash when evaluating achievements. This allows achievements to use this data vs just counting the number of completed sets. One use case is being able to exclude optional sets, such as review sets, from some achievements without completely excluding them from all achievements.

In addition saving all the setIDs can avoid a double counting completed sets, as there is currently no check to ensure a set is not counted multiple times, and a student can earn the complete N sets achievements by submitting a problem on the same completed set over and over.

This also cleans up some typos in the comments and formats them better.

Copy link
Copy Markdown
Member

@drgrice1 drgrice1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than minor code quibbles this looks good.

Comment thread lib/WeBWorK/AchievementEvaluator.pm Outdated

$globalData->{completeSets}++ if ($allcorrect);
# Store the setID of all completed sets so they can be used in achievements and not double counted.
$globalData->{completedSetIds} = {} unless defined $globalData->{completedSetIds};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be

Suggested change
$globalData->{completedSetIds} = {} unless defined $globalData->{completedSetIds};
$globalData->{completedSetIds} //= {};

Comment thread lib/WeBWorK/AchievementEvaluator.pm Outdated
# Store the setID of all completed sets so they can be used in achievements and not double counted.
$globalData->{completedSetIds} = {} unless defined $globalData->{completedSetIds};
if ($allcorrect) {
$globalData->{completeSets}++ unless $globalData->{completedSetIds}{$set_id};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always use pre increment instead of post increment unless post increment is what is really needed. I know it was post increment before, but time to fix that. So make this

Suggested change
$globalData->{completeSets}++ unless $globalData->{completedSetIds}{$set_id};
++$globalData->{completeSets} unless $globalData->{completedSetIds}{$set_id};

Pre increment is more efficient because it doesn't need to save and return the original value.

Store the setID of all completed sets in the globalHash when
evaluating achievements. This allows achievements to use this
data vs just counting the number of completed sets. One use case
is being able to exclude optional sets, such as review sets, from
some achievements without completely excluding them from all
achievements.

In addition saving all the setIDs can avoid a double counting
completed sets, as there is currently no check to ensure a set
is not counted multiple times.
@somiaj somiaj force-pushed the add-completed-set-ids branch from 021ac3b to 268d5e3 Compare May 5, 2026 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants