0000718: Projectile desync when fired to an edge (see demo for the effect)

(0002895)
Edward-san (developer)
2012-03-25 21:05

To be more precise, make a new server from the command line, join the server and do as the demo shows. It's not easy to spot the right angle, but it happened sometimes that monsters projectiles stopped at a certain edge but later they hurt the player. Pretty annoying.

(0002951)
unknownna (updater)
2012-03-28 03:18

I can confirm this. With 2 clients, both clients agree that the missile visually/sonically explodes in the wrong place. The actual explosion is on the pillar on the left in the screenshot.

(0003330)
unknownna (updater)
2012-04-17 03:38

Quote from Torr Samaho
To save bandwidth the server only sent the player's position at 16 bit accuracy. When at a sector boundary, this could cause a player to be rounded to the neighboring sector and since a player can't be inside the floor, it was then raised to the floor height. I made the server sent the x/y position at full precision, fixing the problems for players. I really don't want to send the puffs at full precision though, since there are potentially a lot of puffs, this would eat a lot of bandwidth.

(0003338)
TIHan (reporter)
2012-04-17 22:13

This does feel like a precision problem. The question is whether we should allow the full precision of projectiles spawning. IMHO, we don't want to have edge cases of projectiles desyncing nor do we want to waste a lot of bandwidth.

If this is indeed a precision issue (which I believe it is), there needs to be a discussion on whether or not all missiles should spawn with full precision. Reading this "monsters projectiles stopped at a certain edge but later they hurt the player." is a big problem.

(0003341)
Torr Samaho (administrator)
2012-04-18 00:58

It should be completely sufficient if the client doesn't execute the explode on hit logic and waits for the server to tell it about the explosion. That's how I fixed the client side explosions mispredictions on actor hits.

(0003342)
TIHan (reporter)
2012-04-18 02:15

Fixed.
Clients will not explode missiles on their own. The server will tell them.
'https://bitbucket.org/TIHan/tst/changeset/9f8deb416f44 [^]'

(0003347)
TIHan (reporter)
2012-04-18 23:43

This apparently isn't complete yet. I'll let you know when I have updated it.

(0003350)
TIHan (reporter)
2012-04-21 00:03
edited on: 2012-04-21 00:03

This should take care of it.
'https://bitbucket.org/TIHan/tst/changeset/4c932e6148fe [^]'

I thought about this for a while and IMHO, I do not think it is a problem if we let projectiles fire on exact positions as they do not occur THAT frequently between seconds. Until we really know setting projectiles to exact firing positions causes some very significant bandwidth issues should we even consider hacky methods.

But anyway, if you disagree, then that fix should take care of your original plan.

(0003353)
Torr Samaho (administrator)
2012-04-21 15:21

Regarding the precision: Deciding to always increase bandwidth usage to fix problems that only occur rarely is a decision that you can probably make a few times, but recently I have seen a tendency that this kind of decisions are proposed to be made on a regular basis.

All this additional bandwidth usage accumulates and may very well get us in trouble. What we could consider is to reintroduce a client side connection setting, where the client tells the server whether he favors less bandwidth usage or more accuracy.

(0003369)
TIHan (reporter)
2012-04-21 20:21

> kind of decisions are proposed to be made on a regular basis.
Because we have desyncs. We do not know for sure how precision in these cases would affect bandwidth typically. But we do know for sure because of no precision we have bugs. I would prefer that we not gold plate anything here. Let's make everything work and if there is a performance/bandwidth problem then we will address it.

> where the client tells the server whether he favors less bandwidth usage or more accuracy.
IMHO, the client should never determine how much bandwidth the server should use. That should be entirely up to the server admin.

(0003373)
Torr Samaho (administrator)
2012-04-21 23:25

> Let's make everything work and if there is a performance/bandwidth problem then we will address it.

The big problem I see with that approach is that by the time the bandwidth usage gets to excessive, we made so many changes that each increased the bandwidth that it's a lot of work to go back to the old bandwidth usage.

> IMHO, the client should never determine how much bandwidth the server should use. That should be entirely up to the server admin.

Well, you may argue that the server should be able to set an upper cap on the bandwidth. That's fine. But I don't see why a client should not be able to say, send me less stuff, I don't care about small desyncs. The client knows best how much download bandwidth he has and it doesn't make sense if the server admin forces the client to receive more than the connection of the client can handle.

(0003375)
TIHan (reporter)
2012-04-21 23:39

> we made so many changes that each increased the bandwidth that it's a lot of work to go back to the old bandwidth usage.

The only changes I see concerning precision are items dropped on actor death and spawned projectiles. If we talk about how we can design such things where we can flip a switch and turn on and off precision would be the best approach to avoid this problem.

> But I don't see why a client should not be able to say, send me less stuff, I don't care about small desyncs. The client knows best how much download bandwidth he has and it doesn't make sense if the server admin forces the client to receive more than the connection of the client can handle.

I sort of agree with this, but then are we wanting to make clients be able to play ST on 512kbit connections? Also, should precision be enabled by default? I wouldn't feel warm and fuzzy about players joining games where their settings having turned off precise spawning of projectiles. That seems like a cheap way to cut bandwidth down on people who don't even know such a setting exists.

(0003500)
Torr Samaho (administrator)
2012-04-30 00:38

Your patch intentionally desyncs the ZDoom flags of the actor between clients and server which IMHO is a bad idea. The other code is written under the assumption that these flags are synced and since all other code handles it this way, the server can easily overwrite the client flag change you added. Note that the special Skulltag flags are intentionally not synced between client and server.

(0003502)
TIHan (reporter)
2012-04-30 01:18
edited on: 2012-04-30 02:50

> Your patch intentionally desyncs the ZDoom flags of the actor between clients and server which IMHO is a bad idea. The other code is written under the assumption that these flags are synced and since all other code handles it this way, the server can easily overwrite the client flag change you added. Note that the special Skulltag flags are intentionally not synced between client and server.

Ok, I understand what you are saying. I agree.

I have noticed another problem with the hit detection on floors/ceiling. Projectiles with bounce just get stuck in floor and ceiling. I understand why.

(0003505)
TIHan (reporter)
2012-04-30 02:45

This should take care of it. Much cleaner.
'https://bitbucket.org/TIHan/tst/changeset/a8c082b2ba20 [^]'

(0003833)
Torr Samaho (administrator)
2012-06-24 09:42

The patch looks very clean now, how well it works is hard to say though just looking at the diff. How did you test how well it works?

(0003841)
TIHan (reporter)
2012-06-24 15:44

Thanks for the review.

I tested by having heavy packet loss and firing non-bouncing missiles to see if they indeed go through actors, walls, ceilings, and floors; which they did when I tested it.

Note: The hit detection for ceiling and floors are separate from actors + walls.

(0003877)
Torr Samaho (administrator)
2012-07-01 13:54

Alright, I added you patch, still can't confirm how well it works though. Thus this should be thoroughly tested in the next beta build.

(0003990)
Torr Samaho (administrator)
2012-07-15 11:46

Unfortunately 0000718:0003505 introduced two serious problems 0000912 and 0000914. Both should be fixed now, but please test this very thoroughly in the next beta build to make sure that we found and fixed all adverse side effects the fix introduced.

(0004045)
Qent (updater)
2012-07-22 17:19

I get "CLIENTDEMO_ProcessDemoHeader: Expected CLD_DEMOSTART" when trying to play the demo in 98d. What version is it in?

(0004048)
Edward-san (developer)
2012-07-22 18:26

Quote from "Description"
Done with r3417.

ie old skulltag 98e beta r3417.

(0004049)
Torr Samaho (administrator)
2012-07-22 18:30

Since you recorded the demo, you should know how to reproduce the problem ;). So can you try if this is still broken in the latest beta build?

(0004053)
Edward-san (developer)
2012-07-22 21:57

Hmmm... I wasn't able to reproduce the problem with the latest beta. It seems like the fix works :) .

(0004058)
Torr Samaho (administrator)
2012-07-23 19:12

Alright, then let's assume that it's fixed. :)

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

Issue History
Date Modified	Username	Field	Change
2012-03-25 21:02	Edward-san	New Issue
2012-03-25 21:02	Edward-san	File Added: projbug.cld
2012-03-25 21:05	Edward-san	Note Added: 0002895
2012-03-28 03:15	unknownna	File Added: Screenshot_Doom_20120328_051153.png
2012-03-28 03:18	unknownna	Note Added: 0002951
2012-03-28 03:18	unknownna	Status	new => confirmed
2012-03-28 03:19	unknownna	Reproducibility	have not tried => always
2012-04-17 02:55	unknownna	Relationship added	related to 0000790
2012-04-17 03:38	unknownna	Note Added: 0003330
2012-04-17 22:09	TIHan	Assigned To	=> TIHan
2012-04-17 22:09	TIHan	Status	confirmed => assigned
2012-04-17 22:13	TIHan	Note Added: 0003338
2012-04-18 00:58	Torr Samaho	Note Added: 0003341
2012-04-18 02:15	TIHan	Note Added: 0003342
2012-04-18 02:16	TIHan	Status	assigned => needs review
2012-04-18 23:43	TIHan	Note Added: 0003347
2012-04-18 23:44	TIHan	Status	needs review => assigned
2012-04-21 00:03	TIHan	Note Added: 0003350
2012-04-21 00:03	TIHan	Note Edited: 0003350	View Revisions
2012-04-21 00:03	TIHan	Status	assigned => needs review
2012-04-21 15:21	Torr Samaho	Note Added: 0003353
2012-04-21 20:21	TIHan	Note Added: 0003369
2012-04-21 23:25	Torr Samaho	Note Added: 0003373
2012-04-21 23:39	TIHan	Note Added: 0003375
2012-04-30 00:38	Torr Samaho	Note Added: 0003500
2012-04-30 00:38	Torr Samaho	Status	needs review => feedback
2012-04-30 01:18	TIHan	Note Added: 0003502
2012-04-30 02:45	TIHan	Note Added: 0003505
2012-04-30 02:45	TIHan	Status	feedback => needs review
2012-04-30 02:49	TIHan	Note Edited: 0003502	View Revisions
2012-04-30 02:50	TIHan	Note Edited: 0003502	View Revisions
2012-06-09 13:22	Torr Samaho	Category	General => Bug
2012-06-24 09:42	Torr Samaho	Note Added: 0003833
2012-06-24 09:42	Torr Samaho	Status	needs review => feedback
2012-06-24 15:44	TIHan	Note Added: 0003841
2012-07-01 13:54	Torr Samaho	Note Added: 0003877
2012-07-01 14:03	Torr Samaho	Status	feedback => needs testing
2012-07-15 11:27	Torr Samaho	Relationship added	related to 0000914
2012-07-15 11:40	Torr Samaho	Relationship added	related to 0000912
2012-07-15 11:46	Torr Samaho	Note Added: 0003990
2012-07-22 17:19	Qent	Note Added: 0004045
2012-07-22 18:26	Edward-san	Note Added: 0004048
2012-07-22 18:30	Torr Samaho	Note Added: 0004049
2012-07-22 21:57	Edward-san	Note Added: 0004053
2012-07-23 19:12	Torr Samaho	Note Added: 0004058
2012-07-23 19:13	Torr Samaho	Status	needs testing => resolved
2012-07-23 19:13	Torr Samaho	Fixed in Version	=> 1.0
2012-07-23 19:13	Torr Samaho	Resolution	open => fixed
2012-07-26 19:20	Torr Samaho	Relationship added	related to 0000936
2014-01-14 13:47	Dusk	Relationship added	related to 0001665
2014-11-04 11:28	Dusk	Relationship added	parent of 0001468
2018-09-30 19:51	Blzut3	Status	resolved => closed

Notes
(0002895) Edward-san (developer) 2012-03-25 21:05	To be more precise, make a new server from the command line, join the server and do as the demo shows. It's not easy to spot the right angle, but it happened sometimes that monsters projectiles stopped at a certain edge but later they hurt the player. Pretty annoying.

(0002951) unknownna (updater) 2012-03-28 03:18	I can confirm this. With 2 clients, both clients agree that the missile visually/sonically explodes in the wrong place. The actual explosion is on the pillar on the left in the screenshot.

(0003330) unknownna (updater) 2012-04-17 03:38	Quote from Torr Samaho To save bandwidth the server only sent the player's position at 16 bit accuracy. When at a sector boundary, this could cause a player to be rounded to the neighboring sector and since a player can't be inside the floor, it was then raised to the floor height. I made the server sent the x/y position at full precision, fixing the problems for players. I really don't want to send the puffs at full precision though, since there are potentially a lot of puffs, this would eat a lot of bandwidth.

(0003338) TIHan (reporter) 2012-04-17 22:13	This does feel like a precision problem. The question is whether we should allow the full precision of projectiles spawning. IMHO, we don't want to have edge cases of projectiles desyncing nor do we want to waste a lot of bandwidth. If this is indeed a precision issue (which I believe it is), there needs to be a discussion on whether or not all missiles should spawn with full precision. Reading this "monsters projectiles stopped at a certain edge but later they hurt the player." is a big problem.

(0003341) Torr Samaho (administrator) 2012-04-18 00:58	It should be completely sufficient if the client doesn't execute the explode on hit logic and waits for the server to tell it about the explosion. That's how I fixed the client side explosions mispredictions on actor hits.

(0003342) TIHan (reporter) 2012-04-18 02:15	Fixed. Clients will not explode missiles on their own. The server will tell them. 'https://bitbucket.org/TIHan/tst/changeset/9f8deb416f44 [^]'

(0003347) TIHan (reporter) 2012-04-18 23:43	This apparently isn't complete yet. I'll let you know when I have updated it.

(0003350) TIHan (reporter) 2012-04-21 00:03 edited on: 2012-04-21 00:03	This should take care of it. 'https://bitbucket.org/TIHan/tst/changeset/4c932e6148fe [^]' I thought about this for a while and IMHO, I do not think it is a problem if we let projectiles fire on exact positions as they do not occur THAT frequently between seconds. Until we really know setting projectiles to exact firing positions causes some very significant bandwidth issues should we even consider hacky methods. But anyway, if you disagree, then that fix should take care of your original plan.

(0003353) Torr Samaho (administrator) 2012-04-21 15:21	Regarding the precision: Deciding to always increase bandwidth usage to fix problems that only occur rarely is a decision that you can probably make a few times, but recently I have seen a tendency that this kind of decisions are proposed to be made on a regular basis. All this additional bandwidth usage accumulates and may very well get us in trouble. What we could consider is to reintroduce a client side connection setting, where the client tells the server whether he favors less bandwidth usage or more accuracy.

(0003369) TIHan (reporter) 2012-04-21 20:21	> kind of decisions are proposed to be made on a regular basis. Because we have desyncs. We do not know for sure how precision in these cases would affect bandwidth typically. But we do know for sure because of no precision we have bugs. I would prefer that we not gold plate anything here. Let's make everything work and if there is a performance/bandwidth problem then we will address it. > where the client tells the server whether he favors less bandwidth usage or more accuracy. IMHO, the client should never determine how much bandwidth the server should use. That should be entirely up to the server admin.

(0003373) Torr Samaho (administrator) 2012-04-21 23:25	> Let's make everything work and if there is a performance/bandwidth problem then we will address it. The big problem I see with that approach is that by the time the bandwidth usage gets to excessive, we made so many changes that each increased the bandwidth that it's a lot of work to go back to the old bandwidth usage. > IMHO, the client should never determine how much bandwidth the server should use. That should be entirely up to the server admin. Well, you may argue that the server should be able to set an upper cap on the bandwidth. That's fine. But I don't see why a client should not be able to say, send me less stuff, I don't care about small desyncs. The client knows best how much download bandwidth he has and it doesn't make sense if the server admin forces the client to receive more than the connection of the client can handle.

(0003375) TIHan (reporter) 2012-04-21 23:39	> we made so many changes that each increased the bandwidth that it's a lot of work to go back to the old bandwidth usage. The only changes I see concerning precision are items dropped on actor death and spawned projectiles. If we talk about how we can design such things where we can flip a switch and turn on and off precision would be the best approach to avoid this problem. > But I don't see why a client should not be able to say, send me less stuff, I don't care about small desyncs. The client knows best how much download bandwidth he has and it doesn't make sense if the server admin forces the client to receive more than the connection of the client can handle. I sort of agree with this, but then are we wanting to make clients be able to play ST on 512kbit connections? Also, should precision be enabled by default? I wouldn't feel warm and fuzzy about players joining games where their settings having turned off precise spawning of projectiles. That seems like a cheap way to cut bandwidth down on people who don't even know such a setting exists.

(0003500) Torr Samaho (administrator) 2012-04-30 00:38	Your patch intentionally desyncs the ZDoom flags of the actor between clients and server which IMHO is a bad idea. The other code is written under the assumption that these flags are synced and since all other code handles it this way, the server can easily overwrite the client flag change you added. Note that the special Skulltag flags are intentionally not synced between client and server.

(0003502) TIHan (reporter) 2012-04-30 01:18 edited on: 2012-04-30 02:50	> Your patch intentionally desyncs the ZDoom flags of the actor between clients and server which IMHO is a bad idea. The other code is written under the assumption that these flags are synced and since all other code handles it this way, the server can easily overwrite the client flag change you added. Note that the special Skulltag flags are intentionally not synced between client and server. Ok, I understand what you are saying. I agree. I have noticed another problem with the hit detection on floors/ceiling. Projectiles with bounce just get stuck in floor and ceiling. I understand why.

(0003505) TIHan (reporter) 2012-04-30 02:45	This should take care of it. Much cleaner. 'https://bitbucket.org/TIHan/tst/changeset/a8c082b2ba20 [^]'

(0003833) Torr Samaho (administrator) 2012-06-24 09:42	The patch looks very clean now, how well it works is hard to say though just looking at the diff. How did you test how well it works?

(0003841) TIHan (reporter) 2012-06-24 15:44	Thanks for the review. I tested by having heavy packet loss and firing non-bouncing missiles to see if they indeed go through actors, walls, ceilings, and floors; which they did when I tested it. Note: The hit detection for ceiling and floors are separate from actors + walls.

(0003877) Torr Samaho (administrator) 2012-07-01 13:54	Alright, I added you patch, still can't confirm how well it works though. Thus this should be thoroughly tested in the next beta build.

(0003990) Torr Samaho (administrator) 2012-07-15 11:46	Unfortunately 0000718:0003505 introduced two serious problems 0000912 and 0000914. Both should be fixed now, but please test this very thoroughly in the next beta build to make sure that we found and fixed all adverse side effects the fix introduced.

(0004045) Qent (updater) 2012-07-22 17:19	I get "CLIENTDEMO_ProcessDemoHeader: Expected CLD_DEMOSTART" when trying to play the demo in 98d. What version is it in?

(0004048) Edward-san (developer) 2012-07-22 18:26	Quote from "Description" Done with r3417. ie old skulltag 98e beta r3417.

(0004049) Torr Samaho (administrator) 2012-07-22 18:30	Since you recorded the demo, you should know how to reproduce the problem ;). So can you try if this is still broken in the latest beta build?

(0004053) Edward-san (developer) 2012-07-22 21:57	Hmmm... I wasn't able to reproduce the problem with the latest beta. It seems like the fix works :) .

(0004058) Torr Samaho (administrator) 2012-07-23 19:12	Alright, then let's assume that it's fixed. :)