0002426: DECORATE related division by 0 in multiplayer in plasmaplant.wad (clusterfiend)

Notes
(0013307) Edward-san (developer) 2015-08-29 11:19 edited on: 2015-09-01 15:40	That's because of this code in decorate: ACTOR SingleSplitBall : CacodemonBall // ... Death: CAL2 E 0 Bright A_JumpIf(sqrt((velXvelX)+(velYvelY))==0.0,"VerticalAngle") CAL2 E 0 Bright A_SetUserVar("user_pitch",(180.0/3.14159)* ((velZ/sqrt((velXvelX)+(velYvelY)))-(((velZ/sqrt((velXvelX)+ (velYvelY)))(velZ/sqrt((velXvelX)+(velYvelY))) (velZ/sqrt((velXvelX)+(velYvelY))))/3)+ (((velZ/sqrt((velXvelX)+(velYvelY)))(velZ/sqrt((velXvelX)+ (velYvelY)))(velZ/sqrt((velXvelX)+(velYvelY)))* (velZ/sqrt((velXvelX)+(velYvelY)))(velZ/sqrt((velXvelX)+ (velY*velY))))/5))) The A_JumpIf code is ignored by the client, hence the A_SetUserVar is executed, leading the expression evaluator to attempt to divide by zero.

(0013308) Zalewa (developer) 2015-08-29 11:23	IMO if it works in single player then it also should work in multiplayer. No exceptions.

(0013350) Dusk (developer) 2015-09-01 14:52	We could make A_SetUserVar be ignored by clients, but that'll likely open another can of worms. Other than that this is impossible to fix.

(0013351) Zalewa (developer) 2015-09-01 15:11	Quote from Dusk this is impossible to fix. There's no such thing as impossible in software development. However, if fixing client code to synchronize properly with what server is doing is difficult, it might be advisable to sacrifice some client-side visual behavior in exchange for the game not crashing for everyone. While disabling "A_SetUserVar" client-side is definitely not a good idea, it might be instead viable to not crash on DIVISION BY ZERO error. Instead, a simple console warning will suffice and a default result of zero from such division could be returned. I also want to refer to issue 1200 as it describes similar problem.

(0013354) Dusk (developer) 2015-09-01 16:41	I agree that disabling A_SetUserVar isn't a very good idea. Making divide by zero not an error is also not a solution. While we should try to support everything that works in singleplayer in multiplayer as well, the cold hard truth is that the new, advanced DECORATE is too much for our netcode to handle. Clients cannot predict the result of A_JumpIf, so they'll inevitably go in the wrong direction. And as long as that happens, no amount of bandage fixing is going to provide an universal solution. So if you want to completely reimplement actor state flow handling, be my guest but until then there's nothing we can do here. The WAD needs to be modified so that its friendlier to Zandronum's netcode.

(0013368) Torr Samaho (administrator) 2015-09-01 18:46 edited on: 2015-09-01 18:47	Dusk, I fully agree that our jump handling simply cannot handle this kind of DECORATE code properly, but I think that the current client behavior in this case, i.e. to completely error out, is the worst possible thing the client can do. So I took a similar approach to 0001200: Since we already accept that the client does nonsense by ignoring jumps, it should also ignore the consequences of doing nonsense. The client now accepts division by zero and prints a one time warning on the first occasion. In single player and on the server, division by zero in a DECORATE expression is still a fatal error.

(0013369) cobalt (updater) 2015-09-01 19:20	Issue addressed by commit 9ecc6864e781: Clients don't error out anymore when encountering a division by zero in a DECORATE expression. Instead, a one time warning is printed on the first occasion and the result is assumed to be zero. This is necessary since clients can encounter this in valid DECORATE code due to Zandronum's jump handling (addresses 2426). Committed by Benjamin Berkels [Torr Samaho] on Tuesday 01 September 2015 20:35:32 Changes in files: docs/zandronum-history.txt \| 1 + src/thingdef/thingdef_expression.cpp \| 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 0 deletions(-)

(0013617) Ru5tK1ng (updater) 2015-10-05 20:53	I loaded the test wad on a local server with alpha-151004-2012. The division by zero messaged was printed and the server did not crash.

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
2015-08-29 09:13	Zalewa	New Issue
2015-08-29 09:13	Zalewa	File Added: clusterfiend.wad
2015-08-29 11:19	Edward-san	Note Added: 0013307
2015-08-29 11:23	Zalewa	Note Added: 0013308
2015-09-01 14:52	Dusk	Note Added: 0013350
2015-09-01 15:11	Zalewa	Note Added: 0013351
2015-09-01 15:40	Zalewa	Note Edited: 0013307	View Revisions
2015-09-01 16:41	Dusk	Note Added: 0013354
2015-09-01 16:41	Dusk	Status	new => closed
2015-09-01 16:41	Dusk	Resolution	open => not fixable
2015-09-01 18:46	Torr Samaho	Note Added: 0013368
2015-09-01 18:47	Torr Samaho	Note Edited: 0013368	View Revisions
2015-09-01 18:47	Torr Samaho	Assigned To	=> Torr Samaho
2015-09-01 18:47	Torr Samaho	Status	closed => needs testing
2015-09-01 19:20	cobalt	Target Version	=> 3.0
2015-09-01 19:20	cobalt	Steps to Reproduce Updated	View Revisions
2015-09-01 19:20	cobalt	Note Added: 0013369
2015-10-05 20:53	Ru5tK1ng	Note Added: 0013617
2015-10-06 16:22	Dusk	Status	needs testing => resolved
2015-10-06 16:22	Dusk	Fixed in Version	=> 3.0
2015-10-06 16:22	Dusk	Resolution	not fixable => fixed
2016-11-20 21:30	Edward-san	Product Version	3.0 => 3.0-beta
2016-11-20 21:30	Edward-san	Steps to Reproduce Updated	View Revisions
2018-09-30 21:57	Blzut3	Status	resolved => closed

Relationships