MantisBT - Zandronum
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0001838Zandronum[All Projects] Suggestionpublic2014-06-16 01:432018-09-30 23:12
ReporterWatermelon 
Assigned ToWatermelon 
PrioritynormalSeverityfeatureReproducibilityN/A
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version1.3 
Target Version1.4Fixed in Version2.0 
Summary0001838: Clientside puffs
DescriptionI made my clientside puffs for my custom binary, certain people liked it a lot. Others turned it off. So it's important to be 'toggleable'.

I need input though on if this really should be bound to connection type. This seems like a feature that people will want on without sacrificing connection type. People like me with good connections like seeing the shot come out instantly.

This will further add to our obnoxious spaghetti zdoom code. The additions are relatively straight forward, but it requires syncing up with the server and client. Therefore this most likely should go in the user info.

Maybe I can turn that unlagged byte field that is !!'d into a bitfield where 0x02 indicates clientside puffs? Save on an extra byte we don't need to send... unless you feel its worth sending it as a byte on it's own.
Steps To Reproduce
Additional Information
TagsNo tags attached.
Relationships
related to 0000599closed Watermelon Blood decals only show under certain circumstances online 
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2014-06-16 01:43WatermelonNew Issue
2014-06-16 01:43WatermelonStatusnew => assigned
2014-06-16 01:43WatermelonAssigned To => Watermelon
2014-06-16 01:43WatermelonStatusassigned => needs review
2014-06-16 03:38WatermelonAssigned ToWatermelon =>
2014-06-16 03:38WatermelonStatusneeds review => new
2014-06-16 03:39WatermelonNote Added: 0009508
2014-06-16 08:54DuskNote Added: 0009512
2014-07-09 01:57StrikerMan780Note Added: 0009912
2014-07-09 02:06StrikerMan780Note Edited: 0009912bug_revision_view_page.php?bugnote_id=9912#r5301
2014-07-09 20:40Torr SamahoNote Added: 0009923
2014-07-29 03:26WatermelonAssigned To => Watermelon
2014-07-29 03:26WatermelonStatusnew => assigned
2014-07-29 03:27WatermelonNote Added: 0010068
2014-08-03 15:52WatermelonNote Added: 0010115
2014-10-11 16:25WatermelonNote Added: 0010497
2014-10-11 16:25WatermelonStatusassigned => needs review
2014-10-11 17:57Torr SamahoNote Added: 0010505
2014-10-11 17:57Torr SamahoStatusneeds review => feedback
2014-10-11 18:17WatermelonNote Added: 0010507
2014-10-11 18:17WatermelonStatusfeedback => assigned
2014-10-11 18:18WatermelonStatusassigned => needs review
2014-10-11 18:19WatermelonNote Edited: 0010507bug_revision_view_page.php?bugnote_id=10507#r5700
2014-10-11 18:21WatermelonNote Edited: 0010507bug_revision_view_page.php?bugnote_id=10507#r5701
2014-10-12 01:06ZzZomboNote Added: 0010517
2014-10-12 21:20WatermelonStatusneeds review => assigned
2014-10-15 20:22WatermelonNote Added: 0010598
2014-10-15 20:22WatermelonStatusassigned => needs review
2014-10-18 16:35Torr SamahoNote Added: 0010624
2014-10-18 16:35Torr SamahoStatusneeds review => feedback
2014-10-18 18:26WatermelonNote Added: 0010628
2014-10-18 18:26WatermelonStatusfeedback => assigned
2014-11-11 02:59WatermelonNote Added: 0010854
2014-11-11 02:59WatermelonStatusassigned => needs review
2014-11-13 03:56WatermelonRelationship addedrelated to 0000599
2014-11-13 03:58WatermelonNote Added: 0010878
2014-11-13 16:41WatermelonNote Added: 0010881
2014-11-17 20:25cobaltStatusneeds review => needs testing
2014-11-17 20:25cobaltTarget Version2.0 => 1.4
2014-11-17 20:25cobaltDescription Updatedbug_revision_view_page.php?rev_id=5981#r5981
2014-11-17 20:25cobaltNote Added: 0010894
2014-11-21 12:42ArcoNote Added: 0010916
2014-11-21 12:42ArcoStatusneeds testing => resolved
2014-11-21 12:42ArcoResolutionopen => fixed
2014-11-21 12:42ArcoProduct Version => 1.3
2014-11-21 12:42ArcoFixed in Version => 2.0
2014-11-21 12:42ArcoDescription Updatedbug_revision_view_page.php?rev_id=5992#r5992
2018-09-30 23:12Blzut3Statusresolved => closed

Notes
(0009508)
Watermelon   
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   
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   
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   
2014-07-09 20:40   
Extending the unlagged byte into a bit field is a good idea.
(0010068)
Watermelon   
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   
2014-08-03 15:52   
Found out why some puffs appear incorrectly, was in the wrong block.
(0010497)
Watermelon   
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   
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   
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   
2014-10-12 01:06   
The BulletPuff actor is already marked as +NONETID.
(0010598)
Watermelon   
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   
2014-10-18 16:35   
I left numerous comments on bitbucket.
(0010628)
Watermelon   
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   
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   
2014-11-13 03:58   
Fully updated and tested!
(0010881)
Watermelon   
2014-11-13 16:41   
Updated again
(0010894)
cobalt   
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   
2014-11-21 12:42   
Works correctly in r141117-2018.