Better keep track of which sets are complete in achievements.#2964
Open
somiaj wants to merge 1 commit intoopenwebwork:WeBWorK-2.21from
Open
Better keep track of which sets are complete in achievements.#2964somiaj wants to merge 1 commit intoopenwebwork:WeBWorK-2.21from
somiaj wants to merge 1 commit intoopenwebwork:WeBWorK-2.21from
Conversation
drgrice1
approved these changes
May 5, 2026
Member
drgrice1
left a comment
There was a problem hiding this comment.
Other than minor code quibbles this looks good.
|
|
||
| $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}; |
Member
There was a problem hiding this comment.
This should be
Suggested change
| $globalData->{completedSetIds} = {} unless defined $globalData->{completedSetIds}; | |
| $globalData->{completedSetIds} //= {}; |
| # 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}; |
Member
There was a problem hiding this comment.
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.
021ac3b to
268d5e3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Store the setID of all completed sets in the
globalDatahash 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.