MantisBT - Zandronum
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0002426Zandronum[All Projects] Bugpublic2015-08-29 09:132018-09-30 21:57
ReporterZalewa 
Assigned ToTorr Samaho 
PrioritynormalSeveritymajorReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version3.0-beta 
Target Version3.0Fixed in Version3.0 
Summary0002426: DECORATE related division by 0 in multiplayer in plasmaplant.wad (clusterfiend)
DescriptionDECORATE related division by 0 in plasmaplant.wad (clusterfiend)
Steps To Reproduce1. Download clusterfiend.wad provided as attachment.
2. Run this WAD on a server.
3. Join this server.
4. Wake up clusterfiend monsters.
5. Give them time.
6. "DIVISON BY 0" in console.
Additional InformationZandronum build:'https://zandronum.com/downloads/testing/3.0/ZandroDev3.0-150819-2351windows.zip [^]'
TagsNo tags attached.
Relationships
Attached Files? clusterfiend.wad (156,781) 2015-08-29 09:13
/tracker/file_download.php?file_id=1635&type=bug
Issue History
Date ModifiedUsernameFieldChange
2015-08-29 09:13ZalewaNew Issue
2015-08-29 09:13ZalewaFile Added: clusterfiend.wad
2015-08-29 11:19Edward-sanNote Added: 0013307
2015-08-29 11:23ZalewaNote Added: 0013308
2015-09-01 14:52DuskNote Added: 0013350
2015-09-01 15:11ZalewaNote Added: 0013351
2015-09-01 15:40ZalewaNote Edited: 0013307bug_revision_view_page.php?bugnote_id=13307#r7991
2015-09-01 16:41DuskNote Added: 0013354
2015-09-01 16:41DuskStatusnew => closed
2015-09-01 16:41DuskResolutionopen => not fixable
2015-09-01 18:46Torr SamahoNote Added: 0013368
2015-09-01 18:47Torr SamahoNote Edited: 0013368bug_revision_view_page.php?bugnote_id=13368#r7997
2015-09-01 18:47Torr SamahoAssigned To => Torr Samaho
2015-09-01 18:47Torr SamahoStatusclosed => needs testing
2015-09-01 19:20cobaltTarget Version => 3.0
2015-09-01 19:20cobaltSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=7999#r7999
2015-09-01 19:20cobaltNote Added: 0013369
2015-10-05 20:53Ru5tK1ngNote Added: 0013617
2015-10-06 16:22DuskStatusneeds testing => resolved
2015-10-06 16:22DuskFixed in Version => 3.0
2015-10-06 16:22DuskResolutionnot fixable => fixed
2016-11-20 21:30Edward-sanProduct Version3.0 => 3.0-beta
2016-11-20 21:30Edward-sanSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=9872#r9872
2018-09-30 21:57Blzut3Statusresolved => closed

Notes
(0013307)
Edward-san   
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((velX*velX)+(velY*velY))==0.0,"VerticalAngle")
      CAL2 E 0 Bright A_SetUserVar("user_pitch",(180.0/3.14159)*
          ((velZ/sqrt((velX*velX)+(velY*velY)))-(((velZ/sqrt((velX*velX)+
          (velY*velY)))*(velZ/sqrt((velX*velX)+(velY*velY)))*
          (velZ/sqrt((velX*velX)+(velY*velY))))/3)+
          (((velZ/sqrt((velX*velX)+(velY*velY)))*(velZ/sqrt((velX*velX)+
          (velY*velY)))*(velZ/sqrt((velX*velX)+(velY*velY)))*
          (velZ/sqrt((velX*velX)+(velY*velY)))*(velZ/sqrt((velX*velX)+
          (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   
2015-08-29 11:23   
IMO if it works in single player then it also should work in multiplayer. No exceptions.
(0013350)
Dusk   
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   
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   
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   
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   
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   
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.