MantisBT - Zandronum
View Issue Details
0001956Zandronum[All Projects] Bugpublic2014-10-11 20:202018-09-30 23:36
Ijon Tichy 
Dusk 
normalminoralways
closedfixed 
1.3 
1.41.4 
0001956: Clientside ACS calls are bulkier than one would expect bandwidth-wise
For whatever reason, two clientside ACS calls per tic sends 10KB/s worth of bandwidth. For tracer scripts, which must have the tracer drawing clientsided if they don't want to disconnect everyone online, this is not desirable for any large amount of pellets (like, say, a minigun with realistic fire rates).

If there's any way to minimize the size of these calls, it should be pursued so that weaker connections (people still use dial-up!) can handle online play more easily.
1) Start local testing server with scriptlag.pk3, and cheats on.
2) Open up some network monitoring tool. I used iftop.
3) Join local testing server.
4) give "Pistol Beam"
5) Hold down +attack with the Pistol Beam out.
6) Note the sudden 8KB/s jump.
I adapted Metroid: Dreadnought's Chroma Storm for this. Hence the FABULOUSNESS.
No tags attached.
? scriptlag.pk3 (247,126) 2014-10-11 20:20
/tracker/file_download.php?file_id=1307&type=bug
Issue History
2014-10-11 20:20Ijon TichyNew Issue
2014-10-11 20:20Ijon TichyFile Added: scriptlag.pk3
2014-10-11 20:31DuskNote Added: 0010510
2014-10-11 21:14WatermelonNote Added: 0010511
2014-10-12 01:28DuskNote Edited: 0010510bug_revision_view_page.php?bugnote_id=10510#r5708
2014-10-12 01:36DuskNote Added: 0010519
2014-10-12 01:37DuskNote Edited: 0010519bug_revision_view_page.php?bugnote_id=10519#r5710
2014-10-12 01:37DuskNote Edited: 0010519bug_revision_view_page.php?bugnote_id=10519#r5711
2014-10-12 01:47WatermelonNote Edited: 0010511bug_revision_view_page.php?bugnote_id=10511#r5713
2014-10-12 01:48WatermelonNote Edited: 0010511bug_revision_view_page.php?bugnote_id=10511#r5714
2014-10-12 01:49WatermelonNote Edited: 0010511bug_revision_view_page.php?bugnote_id=10511#r5717
2014-10-12 01:49WatermelonNote Edited: 0010511bug_revision_view_page.php?bugnote_id=10511#r5718
2014-10-12 01:55WatermelonNote Added: 0010521
2014-10-12 01:56WatermelonNote Edited: 0010521bug_revision_view_page.php?bugnote_id=10521#r5720
2014-10-12 02:00WatermelonNote Edited: 0010521bug_revision_view_page.php?bugnote_id=10521#r5721
2014-10-12 02:00WatermelonNote Edited: 0010521bug_revision_view_page.php?bugnote_id=10521#r5722
2014-10-12 02:01WatermelonNote Edited: 0010521bug_revision_view_page.php?bugnote_id=10521#r5723
2014-10-12 12:59DuskNote Added: 0010527
2014-10-12 19:54DuskAssigned To => Dusk
2014-10-12 19:54DuskStatusnew => assigned
2014-10-12 23:41DuskNote Added: 0010561
2014-10-12 23:41DuskStatusassigned => needs review
2014-10-13 19:10Torr SamahoNote Added: 0010572
2014-10-13 19:14Torr SamahoStatusneeds review => feedback
2014-10-19 08:42DuskStatusfeedback => assigned
2014-10-19 09:44DuskNote Added: 0010631
2014-10-19 09:44DuskStatusassigned => needs review
2014-10-19 16:07Torr SamahoNote Added: 0010634
2014-10-19 16:14Torr SamahoNote Edited: 0010634bug_revision_view_page.php?bugnote_id=10634#r5806
2014-10-19 19:39cobaltStatusneeds review => needs testing
2014-10-19 19:39cobaltTarget Version => 1.4
2014-10-19 19:39cobaltDescription Updatedbug_revision_view_page.php?rev_id=5810#r5810
2014-10-19 19:39cobaltSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=5812#r5812
2014-10-19 19:39cobaltNote Added: 0010636
2015-03-16 01:28DuskNote Added: 0011835
2015-03-16 01:28DuskStatusneeds testing => resolved
2015-03-16 01:28DuskFixed in Version => 1.4
2015-03-16 01:28DuskResolutionopen => fixed
2018-09-30 23:36Blzut3Statusresolved => closed

Notes
(0010510)
Dusk   
2014-10-11 20:31   
(edited on: 2014-10-12 01:28)
Hmm. The arguments are sent as three longs and the map name is sent as a string. We could probably cut down on those.

(0010511)
Watermelon   
2014-10-11 21:14   
(edited on: 2014-10-12 01:49)
Interesting its sending the map name, that should be at most a single byte I'm guessing.

(0010519)
Dusk   
2014-10-12 01:36   
(edited on: 2014-10-12 01:37)
To cut down on the args, we could send a byte header for them. For each of the three args, let (header >> (2 * arg)) & 8) be 0 if the arg is not sent, 1 if sent as byte, 2 if sent as short and 3 if sent as long.

In the best case this chops off 11 bytes (a common case where args are all 0), at worst it adds one byte if all 3 args are sent and are at least 2^16 (AFAIK a very rare corner case).

(0010521)
Watermelon   
2014-10-12 01:55   
(edited on: 2014-10-12 02:01)
I was thinking about what you said there, and I have an idea:

void SERVERCOMMANDS_ACSScriptExecute( ULONG ulScript, AActor *pActivator, LONG lLineIdx, char *pszMap, bool bBackSide, int iArg0, int iArg1, int iArg2, bool bAlways, ULONG ulPlayerExtra, ULONG ulFlags )


Using what you said, arg1/arg2/arg3 can be cut down to 2 bits. Since it's weird to send a byte/short to represent 24 bits, here's a possible idea:

0x00 = no args (don't read a byte for this argument)
0x01 = arg is a byte
0x02 = arg is a short
0x03 = arg is a long

This would require 6 bits total (2 bits per 3 args).

We can also bundle the two booleans into 1 bit each. This is 2 more bits.

Therefore we can put all the header information in 1 byte.



The map can be a byte and reference the map index number, so that saves probably on average 4-5 bytes.


Net inflate: 2 bytes
Net deflate: approx 8-17 bytes (probably 17 a lot when sending 0/0/0 args).

Sounds like a good gain! This should lead to most commands being 8-9 bytes in size, rather than 26-27 that they are now.

(0010527)
Dusk   
2014-10-12 12:59   
Quote

0x00 = no args (don't read a byte for this argument)
0x01 = arg is a byte
0x02 = arg is a short
0x03 = arg is a long

This would require 6 bits total (2 bits per 3 args).


This is exactly what I said in my post. Here's what I have in mind:'http://pastebin.com/kKBspCh8 [^]'

In the best case this would be 9 bytes total should the map be sent as a byte index.
(0010561)
Dusk   
2014-10-12 23:41   
'https://bitbucket.org/Torr_Samaho/zandronum-stable/pull-request/78 [^]'

Note that this will cause a merge conflict when merging to 2.0. Just overwrite SERVERCOMMANDS_ACSScriptExecute with this new one.
(0010572)
Torr Samaho   
2014-10-13 19:10   
The "Beam Pistol" example from'https://zandronum.com/tracker/view.php?id=1956 [^]' doesn't work properly anymore if I apply your patch. For instance, if I use the Beam Pistol on Doom2 MAP01 from the player 1 start the beam doesn't originate from the player body. Did you test this example?
(0010631)
Dusk   
2014-10-19 09:44   
'https://bitbucket.org/Torr_Samaho/zandronum-stable/pull-request/86 [^]'

Yeah I forgot to test the beam pistol with this. Problem was that the code didn't take negative numbers into account.
(0010634)
Torr Samaho   
2014-10-19 16:07   
(edited on: 2014-10-19 16:14)
The updated patch seems to work with the example wad now, but it doesn't solve the problem: The net traffic is still very high when firing the pistol. I'll have a closer look at what's going on.

EDIT: The influence of SVC_ACSSCRIPTEXECUTE is negligible for the example wad. SVC_GIVEINVENTORY is eating all the bandwidth.

(0010636)
cobalt   
2014-10-19 19:39   
Issue addressed by commit ce30242c5baf: - significantly optimized the SVC_ACSSCRIPTEXECUTE packet, making server-to-client ACS script calls not use up as much bandwidth (fixes 1956)
Committed by Teemu Piippo [Dusk] on Sunday 19 October 2014 12:41:49

Changes in files:
 docs/zandronum-history.txt | 1 +
 src/cl_main.cpp | 74 +++++++++++++++++++++++++-----------
 src/p_acs.cpp | 2 +-
 src/p_lnspec.cpp | 50 ++++++++----------------
 src/sv_commands.cpp | 92 +++++++++++++++++++++++++++++----------------
 src/sv_commands.h | 2 +-
 6 files changed, 130 insertions(+), 91 deletions(-)
(0011835)
Dusk   
2015-03-16 01:28   
Since this modified a rather integral CLIENTSIDE feature, if there were any problems, surely we would've heard of them by now..