Ban Management

Ban Management

193k Downloads

DateUtils bad algorithm

stevmei opened this issue ยท 5 comments

commented

Hello @confuser !

Your DateUtils algorithm has some major bugs. It's calculating the difference for the database correctly, but the back calculation with formatDifference is wrong.

  1. Your month has always 30 days (+/- 1 day difference)
  2. You cannot use week, because it has the same divisor as month (days) (+/- some days in the worst case)

Ban times and database times are absolutly correct, it's just the show value in the chat, that's wrong.

Maybe a new formatDifference function?

If you want to replicate this, try to ban for 8 weeks for example. Remember daylight savings time (this is calculated correctly, an hour is added when the expiring date is after 30.10.16 (germany: daylight saving time ends).

commented

I can also confirm this being an issue, don't know if a seperate ticket is necessary though, I noted the same thing with daylight saving time also in #452.

commented

Yes, you did. This is a similar issue but not exactly the same. The result of 1 hour 59 minutes 59 seconds is a result of an asynchronous calculation of the difference, but I'm telling here about an addionally issue.

I banned a player for 8w and it resulted in banned for 1 month 3 weeks 5 days 1 hour.

1 hour is ok (daylight saving time), but players are confusing about that expression.

13.09. + 1 month = 13.10. (but only because september has 30 days !!!!)
13.10. + 3 weeks = 03.11.
03.11. + 5 days = 08.11.

is the same as

13.09. + 8 weeks = 08.11.

But it would be better when it is showing banned for 8 weeks instead of that confusing expression. Only for the broadcasted message. The "expires in ..." expression is ok with "1 month 3 weeks ..." (but remember, months with <> 30 days are not calculated correctly).

Thats all and yea, you're alright. It's similar to #452 ๐Ÿ‘

commented

I'm aware of the flaws with the format function. It was based on Essentials at the time (around 4 years ago I believe now). Unfortunately as it's just a visual issue, it doesn't affect the database storage itself as you rightly pointed out, I'd rather prioritise the time I have on other issues. However, I'm open to a PR to fix this.

commented

Ok, I guess I'll write a workaround within the next few days and make a PR. Changing the broadcasted message is too much. it would be necessary to rewrite the whole command handling and that's too much. I guess I'll just fix the simple calculation of days of month to show the correct difference in the broadcasted message. That would be enough ๐Ÿ˜„

commented

Should be fixed with #676