MantisBT - Doomseeker
View Issue Details
0003992Doomseeker[All Projects] Bugpublic2022-04-12 21:182024-11-03 19:12
Zalewa 
Zalewa 
normalminorrandom
closedfixed 
1.3.3 
1.4.01.4.0 
0003992: Zandronum plugin: awkward ping calculation for servers can bug out
Zandronum plugins rely on the server launcher protocol to calculate the ping. The current time is retrieved in a buggy way from the ZandronumServer::millisecondTime() method, sent to the server in the query challenge, then retrieved back from the server's query reply, then compared again to ZandronumServer::millisecondTime() to determine the server's ping.

This can bug out when:

1. The computer or the user adjusts the time.
2. The time on the computer changes from 23:59:59 to 00:00:00 of the next day.
3. The server decides to play a trick on us and send back a random value.

Moreover, the ZandronumServer::millisecondTime() is bugged itself because there's one 0 missing when multiplying the hour by the amount of milliseconds.
Doomseeker already has a generic way to set the server's ping. There's no need to rely on the Zandronum's Launcher protocol mechanism.

Zandronum server launcher protocol offers a Long field to pass the time around:

> Long Time Current time, this will be sent back to you so you can determine ping.

'https://wiki.zandronum.com/Launcher_protocol [^]'
No tags attached.
png ds_zandronum_negative_ping.png (4,149) 2022-04-12 21:20
https://zandronum.com/tracker/file_download.php?file_id=2731&type=bug
png
Issue History
2022-04-12 21:18ZalewaNew Issue
2022-04-12 21:19ZalewaNote Added: 0022190
2022-04-12 21:20ZalewaFile Added: ds_zandronum_negative_ping.png
2022-05-02 17:00ZalewaAssigned To => Zalewa
2022-05-02 17:00ZalewaStatusnew => assigned
2022-05-02 17:23ZalewaNote Added: 0022203
2022-05-02 17:23ZalewaStatusassigned => needs testing
2022-05-03 22:51WubTheCaptainNote Added: 0022205
2022-05-03 22:57WubTheCaptainNote Edited: 0022205bug_revision_view_page.php?bugnote_id=22205#r13594
2022-05-03 22:58WubTheCaptainStatusneeds testing => needs review
2022-05-04 15:18ZalewaNote Added: 0022211
2022-09-22 23:46ZalewaNote Added: 0022406
2022-09-22 23:46ZalewaStatusneeds review => needs testing
2023-02-19 14:17ZalewaFixed in Version => 1.4.0
2023-02-19 14:17ZalewaTarget Version => 1.4.0
2023-02-19 14:17ZalewaStatusneeds testing => resolved
2023-02-19 14:17ZalewaResolutionopen => fixed
2024-11-03 19:12ZalewaStatusresolved => closed

Notes
(0022190)
Zalewa   
2022-04-12 21:19   
Let's just remove the custom Zandronum code, send all zeros in the "Long Time" query field, and just rely on Doomseeker's built-in method of determining ping, which is based on the monotonic clock of QElapsedTimer.
(0022203)
Zalewa   
2022-05-02 17:23   
The awkard feature got ctrl+x'd

'https://bitbucket.org/Doomseeker/doomseeker/commits/a97b6c37965f4dadcc88da80705ca5d98b94442d [^]'
(0022205)
WubTheCaptain   
2022-05-03 22:51   
(edited on: 2022-05-03 22:57)
The core codebase no longer has calls to setPing and setPingIsSet functions, after this change. Some things change d->ping and d->bPingIsSet directly, without the function callback. There are code comments like in Server::refreshStops referring to plugins potentially setting the ping. Are these functions going to be deprecated?

(0022211)
Zalewa   
2022-05-04 15:18   
Quote
Are these functions going to be deprecated?

No.
(0022406)
Zalewa   
2022-09-22 23:46   
The plugin with this fix is out on the beta channel for some time (and soon will be on stable too). Needs to be tested if improvement was achieved.