FMCorz / moodle-block_xp

A gamification plugin for Moodle allowing students to gain experience points and level up.
https://levelup.plus/?ref=github
149 stars 41 forks source link

"Time required between identical actions" is not working for "completion event" #39

Closed pooryamd closed 7 years ago

pooryamd commented 8 years ago

Hi, I have an issue with event.

I had set completion criteria for all my modules and tried to use user event name "\core\event\course_module_completion_updated" to get them XPs. It worked but the problem is that every time a completion event is logged for a specific module (e.g a page is viewed and completed!), user get xp and "Time required between identical actions" is not working identical for context as mentioned in help.

Help text for "Time required between identical actions" is saying:

In seconds, the minimum time required between identical actions. An action is considered identical if it was placed in the same context and object, reading a forum post will be considered identifical if the same post is read again.

But I think that Identical context is not working for the event named: "\core\event\course_module_completion_updated"

If I set time for 1000 second, any of completions in any context will work in this time period but first; but if i set it to 1 sec all completions will get them XPs.

Can you help me with this?

p.s: sorry for my bad writing.

FMCorz commented 8 years ago

Hi @pooryamd,

This was intended by design, but I understand that this is not what you expect. The cheat guard is making sure that a student is not cheating the system by triggering more than one identical event at once, an easy way for them would be to refresh a page. Now, imaging that they are posting in two different forums, the event name will be the same "post_created" but they are did not cheat as they are posting in two different locations, are they?

It is the same with the completion, they are not cheating as they are not repeating the event in the same location, they are completing two different modules. As an example, I am pretty sure that if you set "manual completion" on an activity, and you try to toggle multiple time on/off the completion status as a student the cheat guard will kick in.

I could perhaps add an option to the cheat guard to do loose checks, but that kind of goes against rewarding students based on their actions.

What do you think?

pooryamd commented 8 years ago

Thank you,

This was intended by design, but I understand that this is not what you expect. ...

Actually, I'm saying that cheat guard (exactly, "Time required between identical actions") is not working for completions in a context. It is working for course. if I set "Time required between identical actions" to 1000, no completion will work except first one in whole course (not in a specific context - module). this is my problem, I want it to be available for other modules:

It is the same with the completion, they are not cheating as they are not repeating the event in the same location...

And it is not.

And if I set it low, ( for example 1 or 2 Sec.) all completions even in the same context will be considered (this part is working well).

FMCorz commented 8 years ago

Oh, so I got it completely wrong. My bad! I'll have to look into in more details.

I'm really surprised because I do consider the context for each event in the cheat guard. And the context seems to be properly set in the course_module_completion_updated event.

So if I wanted to reproduce this, I'd:

Expected

Actual

Correct?

pooryamd commented 8 years ago

That is correct if you "Complete 3x activities in a row" inside "Time required between identical actions" frame.

FMCorz commented 8 years ago

Hi @pooryamd,

I'm afraid I cannot replicate this, here are my steps:

  1. Enable completion
  2. Create a blank new course with completion enabled
  3. Add 'Level up' block and set a rule on {{Event name - is equal to - \core\event\course_module_completion_updated}} giving 10 points
  4. Add 1 page activity with completion rule: when activity is viewed
  5. Duplicate the page twice and rename it page 2, and 3. We get Page 1, Page 2, Page 3.
  6. Enrol a student in the course
  7. As a student, login and access each page

What happens

The student gets 30 points for completing the page module, which is what I expects. Note that the default time frame for identical actions is 180 seconds so it should kick in if that was the problem.

Am I doing something wrong?

pooryamd commented 8 years ago

Thats exactly what I did and I have the problem. Let me explain my case again:

-A course (completion tracking enabled) with plenty of modules, all have completion criteria set (view, grade and date criteria depending on module type). -"Level Up" block enabled with all default rules (c,u,r ...) overwritten to zero. -New rule on {{Event name - is equal to - \core\event\course_module_completion_updated}} giving 35 points -Cheat guard se as : Max. actions in time frame: 4 Time frame for max. actions: 5 (no need to be more because default rules are overwritten to 0 and has no effect) Time required between identical actions: 1 or 280000

Problem :

Moodle is logging completion for every module, every time its criteria happens. so, if I set "Time required between identical actions" to 1 (or something low), student will get 35 point for every page view (as completion criteria is "view this page" and every time student views a page, moodle logs a completion status!) and if I set "Time required between identical actions" to something like my course time span or more (to prevent every repeated completion criteria - in what you call same context - to be calculated for XP), only first completion reached in whole course will be considered for XP.

Because there is no match in block "Level Up!" log and course log for completions, I think there is something wrong with cheat guard and completion. "Level Up!" log is full of zero XP logs for overwritten default rules.

I also want to suggest some improvements for logging in your plugin:

FMCorz commented 8 years ago

Hi,

You got me even more confused, now I don't know what to think.

First of all I am not sure why you need to cheat guard for anything here. Your activities should be completed only once, if the completion_updated event happens more than once for the same activity it may simply be a bug in Moodle itself.

Secondly, Max. actions in time frame: 4 should not be so low. Even if Time frame for max. actions: 5. What that means is that if there are more than 4 events (even with a 0-rule) within 5 seconds they will be automatically ignored. The idea behind this check was to prevent a student from being too smart and clicking everywhere very quickly.

The logging was intended to be very brief and only there for debugging, I can add more to it but I'm not sure if it really is necessary. In any case, I'd see that a separate issue to this one. I do like the idea of enabling debugging on the cheat guard though.

Moving forward, I see two options:

a) The event _completionupdated has to be considered buggy and fixed at the source b) The plugin should implement a new event rule (On activity/module completed) which will ensure that it is only triggered when required. Though I find it a bit out of scope and very specific to your case. That said, it should be pretty easy to write.

pooryamd commented 8 years ago

Oh! I'm sorry making you feel confused. I tried to make it more clear. And I really want to thank you for the time you are spending here.

I thought you know how Moodle act storing completions. Moodle does not check if a module got completed, It simply update completion again and again. here is when I thougt one of cheat guard options (I mean "Time required between identical actions") may help me.

That's true that I may not need cheat guard when I overwrite defaults to zero. That's why I set two "Max. actions" options to something that I think may cancel their effect.

I thought that "Time required between identical actions" may be enough for me.

I inspected database schema of "_block_xp_log" and I found that you are not logging context (like module ID). So you maybe are storing chat guard information in session (or somewhere I can not find). I think that's why: 1- cheat guard is session dependent and if a user logs out and in, he/she can bypass cheat guard (you mentioned in another issue, I think), 2- cannot check identical actions as I thought (between sessions)

I want to suggest that add context to log and change cheat guard to work with database not to sessions (or maybe an option to select between database and session for performance issues).

This way, we can also have an option to define how many times an event can be rewarded by XPs in a course.

If you prefer, I will add new issues for my suggestions for: 1- cheat guard debugging, 2- option to filter zeroes from log, 3- option to disable default rules individually (to prevent log zeros!) 4- add/replace session to database for cheat guard

And I do not understand this part:

b) The plugin should implement a new event rule (On activity/module completed) which will ensure that it is only triggered when required. Though I find it a bit out of scope and very specific to your case. That said, it should be pretty easy to write.

Thank you very much for great plugin.

FMCorz commented 8 years ago

Hi,

Last night, as a student, I tried to re-access Page activities which were already completed and I did not get any event for completion_updated, I only had course_module_viewed event which accounted for 0. Maybe the completion events are triggered more often than required in some circumstances.

Indeed, the cheat guard is session based. That was intended in order to be as efficient as possible whilst allowing this feature. As block XP is likely to be adding more DB reads (and sometimes writes) on each page request within a course I ruled out the option of adding a complex cheat guard using the session. However, as some reports suggest that it may be useful, I am OK to consider adding an option to the cheat guard for using the logs. Sadly that would force us to store more and more information in the logs, which is basically a duplication of the log stores... I'd have to think about that more thoroughly.

In the past I have argued against changing the default rules. If the only reason is not to log the zeros then maybe we can just add a general option to the logs "Only log non-zero events" kind of thing. That said, maybe I should have a second look at making the default rules editable.

For now I'm OK with the issues 1, 2, and 4. The 3rd one could be "Allow default rules to be editable", though it should mention why we need it as it could become "Option not to log 0 events".

I'm trying to keep the amount of settings as low as possible as it is the best way to confuse users. I want new users to feel comfortable around the plugin, not limiting the audience to whoever has used previous versions of block XP.

I'm really glad you are enjoying the plugin. If you are into gamification, you should definition checkout https://github.com/FMCorz/moodle-block_stash which @abgreeve and I still need to publish on Moodle.org.

FMCorz commented 7 years ago

Linking #52.

FMCorz commented 7 years ago

Editing the default rules is now possible in v3.0, for both admin and teachers.

A more resilient cheat guard and activity/course completion support is available through the add-on Level up! Plus.