Cleanup modelCourse achievements.

[somiaj]

Cleanup and update the default achievements in the modelCourse.

• Remove the same comment from the bottom of all evaluators. This is hard to maintain, instead put all the information in achievements_readme.txt and point to this file from other evaluators.

• Cleanup the achievement evaluators:

  • Simplify the code, don't use else statements when there was a return inside the previous if. Clean up some of the logic.
  • Take advantage of the new $globalData->{completedSetIds} hash to avoid looping through all problems to determine if a set is completed.
  • Ran perltidy on them using an 80 character limit.

• Remove some evaluator duplication. The complete n problems or n sets can all be done using a single evaluator, since the only difference is the $maxCounter setting from the achievement.

• Remove all unused achievement evaluators. There are lots of these that are really just the same example achievement over and over again to see if particular problems from particular sets were completed. The images are still available for helping people think of names, but the evaluators don't need to be included. Instead provide four example evaluators for completing specific problems and/or sets.

• Add perltidy of .at achievement files to the run-perltidy dev_script and the github workflow.

[somiaj]

Okay, I have tested the most of the achievements all work and fixed a few issues/typos I found. Only ones I didn't test were the "Finish after 8 hour break", "Finish over a 48 hour period", and and ones that require finishing late at night or early in the morning. So this is now fully ready for review.

[Alex-Jordan]

Could we move the distribution achievements to the assets folder in some way? Then set up symlinks in templates/achievements and html/achievements to the right place with the assets folder tree?

And then include these symlinks in the symlink verification business that is part of doing a course upgrade.

The goal is to make it so future impovements of achievement files automatically are experienced by all courses, even old courses or clones of old courses.

There would be something to work out for when an old course still has an actual achievement file in templatex/achievements that clashes with the central assest folder version...

[drgrice1]

Ideally we could even entirely eliminate the model course!

[somiaj]

One issue to work around with using symlinks, is then users cannot add their own evaluators easily because of now being linked to a shared directory. We would probably need some override system where achievements are looked for in both a system location for default ones and a local location for user achievements and overrides.

I think if we decide to change where achievements are saved, and setup some user override system, that should be a PR separate of this cleanup, and not be part of this cleanup.

[drgrice1]

I agree. Limit this pull request to the cleanup.

[drgrice1]

This generally looks good, but there is some more clean up that should be done while we are at it.

It would be good to add .at files to those that are formatted by perltidy when the bin/dev_scripts/run-perltidy.pl file is run. This can be accomplished by changing line 93 of that file to return unless $path =~ /\.p[lm]$/ || $path =~ /\.t$/ || $path =~ /\.at$/;.

It would also be good to check these in the formatting workflow. Change line 29 of .github/workflows/check-formats.yml to perltidy --pro=./.perltidyrc -b -bext='/' ./**/*.p[lm] ./**/*.t ./**/*.at && git diff --exit-code to make this happen.

[somiaj]

In adding the .at files to the perltidy scripts and workflow. What would be needed to add the -l 80 for these files? Would a separate line be needed? I ran these with the 80 character limit to match what the pg perl tidy is doing and these files to be easier to see in the editor on small screens.

[somiaj]

Applied all of @drgrice1 suggested changes.

[drgrice1]

With the run-perltidy.pl script and the workflow, you really can't set a separate line width. You would have to use a separate .perltidyrc file, and that isn't worth it. You can already use the run-perltidy.pl script on these files by passing the file name, and I did so. It doesn't change any of the current formatting. perltidy generally doesn't consolidate onto one line if the code is already broken onto multiple lines. So the 120 character limit is fine for this.

It is better that general formatting be enforced for repository files.

[somiaj]

Okay, .at files added to the run-perltidy and github workflow.

[drgrice1]

Looks good.