Timer implementation changed?

[SlimeDog]

Spigot 1.12.2 CommandSigns 1.6.3

Timer implementation seems to have changed sometime in the last few versions (this is a test server, so I didn't notice immediately; apologies).

This code used to work:

{
  "id": 13,
  "name": "Reset-MS-board-0",
  "world": "world_minigames",
  "x": 287,
  "y": 8,
  "z": 15,
  "commands": [
    "^particle hugeexplosion 287 10 15 1 1 1 1 500",
    "^setblock 287 10 15 tnt",
    "/sweeper reset 0"
  ],
  "needed_permissions": [],
  "temporary_permissions": [],
  "price": 0.0,
  "time_before_execution": 4,
  "move_cancel_timer": false,
  "move_reset_timer": false,
  "global_time_between_usages": 5,
  "player_time_between_usages": 5
}

Note the timer settings. In CS 1.6.3 I get two in-game messages, one expected:

This command sign execution is delayed by a timer. Please wait 4 seconds.

and one not:

This command block has been used 3.9 seconds ago. You must wait 0.1 more seconds

Changing

...
  "global_time_between_usages": 3,
  "player_time_between_usages": 3
...

makes everything work, but this doesn't seem right, somehow. The _betweenusages timers are blocking the current execution.

[Nokorbis]

Weird stuff, looking into when I come back from work

[SlimeDog]

Yeah. The between timers should be started after completion of the current execution.

[Nokorbis]

Okay, I see where the problem stands. But it has probably been there for a very long time (before 1.6.3). I'm still not sure if I try to fix this tonight or tomorrow. I've had a very long day and I am not sure if my brain is properly working x)

[SlimeDog]

There is no hurry (for me); I config'd around it for now. As I said, I have no idea when it was introduced, and I didn't think it was worth the effort to isolate that, unless you couldn't find the problem. It looks like you did. So... sleep and fix it when brain is working properly. :)

[SlimeDog]

:sleeping:

[Nokorbis]

Yeah sorry, working on it but it has more implication that it first looked to. Got delayed because got ill last week.

[SlimeDog]

No worries. Just a gentle nudge. Hope you're feeling better. :)

[Nokorbis]

I think the update is done. I put it on my own server to make sure it is stable. If it is stable, I'll upload it on spigot.

[SlimeDog]

Looking forward to it. Let me know if you want me to test.

[Nokorbis]

I uploaded it here https://github.com/Nokorbis/ar-command-signs/releases/tag/v1.6.5 If you want to test it. If anything's wrong, tell me it here and put the v1.6.3 version back on your server and it should be fine.

[SlimeDog]

Tested 1.6.5. It seems stable, but the scenario described above still fails.

[Nokorbis]

Then I'm not sure what you want, this is actually the way I want it to be. In that example, You see that I define a command and put it with a wait time of 5 seconds, because I want the player to wait five 5 seconds once he clicked on the sign before the command is executed. Then, there is a between usage ("cooldown") of 4 seconds because I want the player to be unable to use that sign right away. (and thus, 4 seconds between each execution) Wasn't it what you wanted ?

[SlimeDog]

My previous configuration:

  "time_before_execution": 4,
  "move_cancel_timer": false,
  "move_reset_timer": false,
  "global_time_between_usages": 5,
  "player_time_between_usages": 5

I want command execution to be delayed 4 seconds (time_before_execution). I want the player to be unable to execute it for 5 seconds (player_time_between_usages), and I want no one else to be able to execute it for 5 seconds (global_time_between_usages). That configuration did what I wanted/expected until 1.6.x (I didn't test every new version, but it certainly failed to do so at 1.6.3). The problem appears to be that the delay prevents the command from executing at all. If I change the "between_usages" timers to 3, everything works, so that's how I configure it now. But it appears that the "between_usages" timers are started when the sign is executed, rather than after the (delayed) command execution.

I can live with it the way it is now, but that isn't what I expected, and it changed at some point.

[Nokorbis]

Okay, there was something strange with what you told me because it's seemed weird, so I read the full process again, and I might have understood what's wrong. I've also seen a part that I forgot about (2.vi in the following list). (Actually, while writing what's following, I did see what's wrong, which is of course linked to what I forgot about at point 2.vi)(I still write the full process since it might interest someone of it works someday)
So, here is the full process of an execution :

1. The player clicks on the sign (or the button, or walk on a plate)
2. Checking requirements (first time)
   1. Checking player's availability (offline/dead)
   2. Checking needed/required permissions
   3. Checking global cooldown (global time between usage)
   4. Checking player cooldown (player time between usage)
   5. (If vault, check if player has enough money)
   6. Refresh global timer
3. Start "time_before_execution" timer
4. If the player moves, cancel/reset the time_before_execution timer according to the config'
5. time_before_execution later, Checking requirements (second time, in case somebody lost a needed permission for example)
6. (If vault, withdraw the player)
7. Checking time between player usage (again, huh ? I'm currently wondering why I'm doing it again here if it's in the check requirement step)
8. Grant temporary permissions
9. Execute commands
10. Start player cooldown timer
11. Remove temporary granted permission

Funny how I now see how my code has evolded with the years :D

[SlimeDog]

Thanks for sharing the algorithm.

Perhaps (7) is the step that is causing the issue.

I would also suggest that (2) is unnecessary, since it does't matter if the player has the permissions at that point, only after the timer has run. (And since the timer is usually 0, (2) and (5) usually occur in immediate sequence, so (2) is even less necessary.)

[Nokorbis]

Sorry for the long wait. I've been moving in to a new place and was difficult to find time even for simple things. This version should fix your problem : https://github.com/Nokorbis/ar-command-signs/releases/download/v1.6.6/Spigot-CommandSigns.jar

[SlimeDog]

Thank you. 1.6.6 works as expected. Much appreciated.