DateUtils bad algorithm
stevmei opened this issue ยท 5 comments
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.
- Your month has always 30 days (+/- 1 day difference)
- You cannot use
week
, because it has the same divisor asmonth
(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).
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.
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 ๐
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.
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 ๐
Should be fixed with #676