0001838: Clientside puffs

(0009508)
Watermelon (developer)
2014-06-16 03:39

Requesting some kind of input, if I hear nothing in a week I will assign it to myself

(0009512)
Dusk (developer)
2014-06-16 08:54

Userinfo is sent very infrequently (when the player connects and when it is changed) and has a rather strict flood limit so I don't think it matters to tack another byte there for clientside puffs. Though maybe it'd make sense to combine all these userinfo bools to a flagset and use a short? Either way the gain is insignificant.

Can you post the patch that implements clientside puffs? I cannot comment further before I see this for myself.

(0009912)
StrikerMan780 (reporter)
2014-07-09 01:57
edited on: 2014-07-09 02:06

I think it should just apply if one is using cl_unlagged, just like with the railgun's trail. (For consistency's sake.)

(0009923)
Torr Samaho (administrator)
2014-07-09 20:40

Extending the unlagged byte into a bit field is a good idea.

(0010068)
Watermelon (developer)
2014-07-29 03:27

I have completed clientsiide puffs for 2.0.
I need to figure out though why sometimes puff appear incorrectly on window ledges. Other than that, It works perfectly.

IDK why they don't appear properly on window ledges, when it comes from the server it's fine, but when the client emulates it, it seems like it guesses the Z wrong and then raises it to the next level. I will debug this and try to find out why. Interestingly this never happened in my 1.2 fork.

(0010115)
Watermelon (developer)
2014-08-03 15:52

Found out why some puffs appear incorrectly, was in the wrong block.

(0010497)
Watermelon (developer)
2014-10-11 16:25

Requesting feedback:

Is it safe to assume that we can handle puffs clientside with no netID? This would be a convenient way if we're not supposed to track them, and would make quite a nice deterministic way of knowing when to predict and not predict.

(0010505)
Torr Samaho (administrator)
2014-10-11 17:57

Quote from Watermelon
Is it safe to assume that we can handle puffs clientside with no netID?

You mean to allow a client to spawn a puff client side if the puff actor has the NONETID flag? Does this mean you never want to spawn puffs without NONETID on the client directly? This sounds ok to me.

(0010507)
Watermelon (developer)
2014-10-11 18:17
edited on: 2014-10-11 18:21

The NONETID flag sounds like a good idea.

I was looking at the function SERVERCOMMANDS_SpawnThingNoNetID and noticed that there is a condition where the puff that is spawned which uses this function. My solution would be to allow clients only to the code that has puff = SpawnPuff(...) and terminate right after so it could check if the code eventually goes to SERVERCOMMANDS_SpawnThingNoNetID.

To be more clear:
- Let clients further in the code (past where we stop for the decal hack if they want clientside puffs)
- Let the clients go into the SpawnPuff functions, and exit right after
- While inside, only proceed to the block of code where SERVERCOMMANDS_SpawnThingNoNetID is, and predict that.

Quote
Does this mean you never want to spawn puffs without NONETID on the client directly?

Only if cl_predictpuffs was set to true.

Can I get your thoughts on how we handle vanilla puffs like ssg and such shots which we know are safe to clientside? We can't throw on the +NONETID to it since it might break some wads, but checking for it not having a net ID and letting the client predict it would work around this.
Plus then it should theoretically work for +NONETID actors.

Also: Should we predict decals and puffs together? Or should we allow people to separately choose if they want decals and not puffs, or puffs and not decal prediction?
There is one person I know who does not want puff prediction (but the rest do). Most people don't really focus too much on the decal prediction anyways.

(0010517)
ZzZombo (reporter)
2014-10-12 01:06

The BulletPuff actor is already marked as +NONETID.

(0010598)
Watermelon (developer)
2014-10-15 20:22

Needs code review:

'https://bitbucket.org/ChrisKOmg/zandronum_stable/commits/8f237cd7d74b77a2b770b159088505953e4fc3a3 [^]'

Not submitted for pull yet because
1) I want a code review
2) Needs more testing for sure

This works great so far in the tests I've done.

INTERESTINGLY due to a small code error while I was making it, I predicted all bulletpuffs from actors shooting a NONETID... I wonder if this could possibly be another setting later on to save even more bandwidth?

Anyways: I only made it work for the actor who is firing since that was the scope of this ticket. It can always be expanded if this gets approved.

(0010624)
Torr Samaho (administrator)
2014-10-18 16:35

I left numerous comments on bitbucket.

(0010628)
Watermelon (developer)
2014-10-18 18:26

Will update to the comments provided. Left one comment:
'https://bitbucket.org/ChrisKOmg/zandronum_stable/commits/8f237cd7d74b77a2b770b159088505953e4fc3a3#chg-src/p_map.cpp [^]'

(0010854)
Watermelon (developer)
2014-11-11 02:59

'https://bitbucket.org/Torr_Samaho/zandronum-stable/pull-request/101/added-clientside-puff-prediction-fixes/diff [^]'

I left some comments for you so hopefully you can provide some feedback.

I understand that the LineAttack function is a complete mess (and it was quite irritating working with it and finding all the weird bugs that pop up), I've tested this a lot (so far so good!) and hopefully can get more testing done with a public beta.

(0010878)
Watermelon (developer)
2014-11-13 03:58

Fully updated and tested!

(0010881)
Watermelon (developer)
2014-11-13 16:41

Updated again

(0010894)
cobalt (updater)
2014-11-17 20:25

Issue addressed by commit 344d29b95344: Added CVAR cl_clientsidepuffs which allows clients to predict puffs, which makes bullet puffs from hitscan weapons appear instantly.
Committed by WChrisK [Watermelon] on Sunday 16 November 2014 10:32:54

Changes in files:
docs/zandronum-history.txt | 1 +
src/d_netinf.h | 8 ++++++++
src/d_netinfo.cpp | 15 ++++++++++++++-
src/m_options.cpp | 4 +++-
src/p_map.cpp | 58 ++++++++++++++++++++++++++++++++++++++++++++--------------
src/p_mobj.cpp | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
src/thingdef/thingdef_codeptr.cpp | 8 +++++---
7 files changed, 131 insertions(+), 20 deletions(-)

(0010916)
Arco (updater)
2014-11-21 12:42

Works correctly in r141117-2018.

This issue is already marked as resolved. If you feel that is not the case, please reopen it and explain why.
Supporters:	Karakurt
Opponents:	No one explicitly opposes this issue yet.

Issue History
Date Modified	Username	Field	Change
2014-06-16 01:43	Watermelon	New Issue
2014-06-16 01:43	Watermelon	Status	new => assigned
2014-06-16 01:43	Watermelon	Assigned To	=> Watermelon
2014-06-16 01:43	Watermelon	Status	assigned => needs review
2014-06-16 03:38	Watermelon	Assigned To	Watermelon =>
2014-06-16 03:38	Watermelon	Status	needs review => new
2014-06-16 03:39	Watermelon	Note Added: 0009508
2014-06-16 08:54	Dusk	Note Added: 0009512
2014-07-09 01:57	StrikerMan780	Note Added: 0009912
2014-07-09 02:06	StrikerMan780	Note Edited: 0009912	View Revisions
2014-07-09 20:40	Torr Samaho	Note Added: 0009923
2014-07-29 03:26	Watermelon	Assigned To	=> Watermelon
2014-07-29 03:26	Watermelon	Status	new => assigned
2014-07-29 03:27	Watermelon	Note Added: 0010068
2014-08-03 15:52	Watermelon	Note Added: 0010115
2014-10-11 16:25	Watermelon	Note Added: 0010497
2014-10-11 16:25	Watermelon	Status	assigned => needs review
2014-10-11 17:57	Torr Samaho	Note Added: 0010505
2014-10-11 17:57	Torr Samaho	Status	needs review => feedback
2014-10-11 18:17	Watermelon	Note Added: 0010507
2014-10-11 18:17	Watermelon	Status	feedback => assigned
2014-10-11 18:18	Watermelon	Status	assigned => needs review
2014-10-11 18:19	Watermelon	Note Edited: 0010507	View Revisions
2014-10-11 18:21	Watermelon	Note Edited: 0010507	View Revisions
2014-10-12 01:06	ZzZombo	Note Added: 0010517
2014-10-12 21:20	Watermelon	Status	needs review => assigned
2014-10-15 20:22	Watermelon	Note Added: 0010598
2014-10-15 20:22	Watermelon	Status	assigned => needs review
2014-10-18 16:35	Torr Samaho	Note Added: 0010624
2014-10-18 16:35	Torr Samaho	Status	needs review => feedback
2014-10-18 18:26	Watermelon	Note Added: 0010628
2014-10-18 18:26	Watermelon	Status	feedback => assigned
2014-11-11 02:59	Watermelon	Note Added: 0010854
2014-11-11 02:59	Watermelon	Status	assigned => needs review
2014-11-13 03:56	Watermelon	Relationship added	related to 0000599
2014-11-13 03:58	Watermelon	Note Added: 0010878
2014-11-13 16:41	Watermelon	Note Added: 0010881
2014-11-17 20:25	cobalt	Status	needs review => needs testing
2014-11-17 20:25	cobalt	Target Version	2.0 => 1.4
2014-11-17 20:25	cobalt	Description Updated	View Revisions
2014-11-17 20:25	cobalt	Note Added: 0010894
2014-11-21 12:42	Arco	Note Added: 0010916
2014-11-21 12:42	Arco	Status	needs testing => resolved
2014-11-21 12:42	Arco	Resolution	open => fixed
2014-11-21 12:42	Arco	Product Version	=> 1.3
2014-11-21 12:42	Arco	Fixed in Version	=> 2.0
2014-11-21 12:42	Arco	Description Updated	View Revisions
2018-09-30 23:12	Blzut3	Status	resolved => closed

Notes
(0009508) Watermelon (developer) 2014-06-16 03:39	Requesting some kind of input, if I hear nothing in a week I will assign it to myself

(0009512) Dusk (developer) 2014-06-16 08:54	Userinfo is sent very infrequently (when the player connects and when it is changed) and has a rather strict flood limit so I don't think it matters to tack another byte there for clientside puffs. Though maybe it'd make sense to combine all these userinfo bools to a flagset and use a short? Either way the gain is insignificant. Can you post the patch that implements clientside puffs? I cannot comment further before I see this for myself.

(0009912) StrikerMan780 (reporter) 2014-07-09 01:57 edited on: 2014-07-09 02:06	I think it should just apply if one is using cl_unlagged, just like with the railgun's trail. (For consistency's sake.)

(0009923) Torr Samaho (administrator) 2014-07-09 20:40	Extending the unlagged byte into a bit field is a good idea.

(0010068) Watermelon (developer) 2014-07-29 03:27	I have completed clientsiide puffs for 2.0. I need to figure out though why sometimes puff appear incorrectly on window ledges. Other than that, It works perfectly. IDK why they don't appear properly on window ledges, when it comes from the server it's fine, but when the client emulates it, it seems like it guesses the Z wrong and then raises it to the next level. I will debug this and try to find out why. Interestingly this never happened in my 1.2 fork.

(0010115) Watermelon (developer) 2014-08-03 15:52	Found out why some puffs appear incorrectly, was in the wrong block.

(0010497) Watermelon (developer) 2014-10-11 16:25	Requesting feedback: Is it safe to assume that we can handle puffs clientside with no netID? This would be a convenient way if we're not supposed to track them, and would make quite a nice deterministic way of knowing when to predict and not predict.

(0010505) Torr Samaho (administrator) 2014-10-11 17:57	Quote from Watermelon Is it safe to assume that we can handle puffs clientside with no netID? You mean to allow a client to spawn a puff client side if the puff actor has the NONETID flag? Does this mean you never want to spawn puffs without NONETID on the client directly? This sounds ok to me.

(0010507) Watermelon (developer) 2014-10-11 18:17 edited on: 2014-10-11 18:21	The NONETID flag sounds like a good idea. I was looking at the function SERVERCOMMANDS_SpawnThingNoNetID and noticed that there is a condition where the puff that is spawned which uses this function. My solution would be to allow clients only to the code that has puff = SpawnPuff(...) and terminate right after so it could check if the code eventually goes to SERVERCOMMANDS_SpawnThingNoNetID. To be more clear: - Let clients further in the code (past where we stop for the decal hack if they want clientside puffs) - Let the clients go into the SpawnPuff functions, and exit right after - While inside, only proceed to the block of code where SERVERCOMMANDS_SpawnThingNoNetID is, and predict that. Quote Does this mean you never want to spawn puffs without NONETID on the client directly? Only if cl_predictpuffs was set to true. Can I get your thoughts on how we handle vanilla puffs like ssg and such shots which we know are safe to clientside? We can't throw on the +NONETID to it since it might break some wads, but checking for it not having a net ID and letting the client predict it would work around this. Plus then it should theoretically work for +NONETID actors. Also: Should we predict decals and puffs together? Or should we allow people to separately choose if they want decals and not puffs, or puffs and not decal prediction? There is one person I know who does not want puff prediction (but the rest do). Most people don't really focus too much on the decal prediction anyways.

(0010517) ZzZombo (reporter) 2014-10-12 01:06	The BulletPuff actor is already marked as +NONETID.

(0010598) Watermelon (developer) 2014-10-15 20:22	Needs code review: 'https://bitbucket.org/ChrisKOmg/zandronum_stable/commits/8f237cd7d74b77a2b770b159088505953e4fc3a3 [^]' Not submitted for pull yet because 1) I want a code review 2) Needs more testing for sure This works great so far in the tests I've done. INTERESTINGLY due to a small code error while I was making it, I predicted all bulletpuffs from actors shooting a NONETID... I wonder if this could possibly be another setting later on to save even more bandwidth? Anyways: I only made it work for the actor who is firing since that was the scope of this ticket. It can always be expanded if this gets approved.

(0010624) Torr Samaho (administrator) 2014-10-18 16:35	I left numerous comments on bitbucket.

(0010628) Watermelon (developer) 2014-10-18 18:26	Will update to the comments provided. Left one comment: 'https://bitbucket.org/ChrisKOmg/zandronum_stable/commits/8f237cd7d74b77a2b770b159088505953e4fc3a3#chg-src/p_map.cpp [^]'

(0010854) Watermelon (developer) 2014-11-11 02:59	'https://bitbucket.org/Torr_Samaho/zandronum-stable/pull-request/101/added-clientside-puff-prediction-fixes/diff [^]' I left some comments for you so hopefully you can provide some feedback. I understand that the LineAttack function is a complete mess (and it was quite irritating working with it and finding all the weird bugs that pop up), I've tested this a lot (so far so good!) and hopefully can get more testing done with a public beta.

(0010878) Watermelon (developer) 2014-11-13 03:58	Fully updated and tested!

(0010881) Watermelon (developer) 2014-11-13 16:41	Updated again

(0010894) cobalt (updater) 2014-11-17 20:25	Issue addressed by commit 344d29b95344: Added CVAR cl_clientsidepuffs which allows clients to predict puffs, which makes bullet puffs from hitscan weapons appear instantly. Committed by WChrisK [Watermelon] on Sunday 16 November 2014 10:32:54 Changes in files: docs/zandronum-history.txt \| 1 + src/d_netinf.h \| 8 ++++++++ src/d_netinfo.cpp \| 15 ++++++++++++++- src/m_options.cpp \| 4 +++- src/p_map.cpp \| 58 ++++++++++++++++++++++++++++++++++++++++++++-------------- src/p_mobj.cpp \| 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- src/thingdef/thingdef_codeptr.cpp \| 8 +++++--- 7 files changed, 131 insertions(+), 20 deletions(-)

(0010916) Arco (updater) 2014-11-21 12:42	Works correctly in r141117-2018.