MantisBT - Zandronum
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0001200Zandronum[All Projects] Bugpublic2012-12-02 23:542018-09-30 20:16
ReporterZalewa 
Assigned ToTorr Samaho 
PrioritynormalSeveritycrashReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version1.0 
Target Version1.1Fixed in Version1.1 
Summary0001200: "randomguy" causes clients to freeze in online games
Description"randomguy" is a monster that is used in this WAD:
'http://www.doomworld.com/idgames/?id=16974 [^]'

In offline single player this monster works correctly, but in online cooperative it causes all clients to freeze when woken up.
Steps To Reproduce1. Start a cooperative server with attached randomguy.wad. Don't forget '+sv_cheats 1'.
2. Join the server.
3. summon randomguy
4. The game should freeze now and require process kill from task manager to be closed.
Additional Information
TagsNo tags attached.
Relationships
has duplicate 0001252closed  Zero length Looping state crashes in multiplayer 
related to 0001162new Dusk A_Jump causes nasty desyncs to occur in Futur War online 
related to 0001832closed Torr Samaho Online Turns Finite 0 Tic Loops into Infinite Loops 
Attached Files? randomguy.wad (2,685) 2012-12-02 23:54
https://zandronum.com/tracker/file_download.php?file_id=896&type=bug
? randomguy_and_randomguy2.wad (5,323) 2013-01-26 13:51
https://zandronum.com/tracker/file_download.php?file_id=926&type=bug
Issue History
Date ModifiedUsernameFieldChange
2012-12-02 23:54ZalewaNew Issue
2012-12-02 23:54ZalewaFile Added: randomguy.wad
2012-12-03 14:07DuskStatusnew => confirmed
2012-12-03 14:10DuskNote Added: 0005465
2012-12-03 15:04ZzZomboNote Added: 0005466
2012-12-03 16:29ZalewaNote Added: 0005467
2012-12-03 16:43DuskNote Added: 0005468
2012-12-03 16:59ZzZomboNote Added: 0005469
2012-12-07 21:31Dark-AssassinNote Added: 0005481
2013-01-13 10:47Torr SamahoNote Added: 0005759
2013-01-13 11:00ZalewaNote Added: 0005760
2013-01-13 11:07ZalewaNote Edited: 0005760bug_revision_view_page.php?bugnote_id=5760#r3165
2013-01-13 11:18Torr SamahoNote Added: 0005761
2013-01-13 11:19Torr SamahoNote Edited: 0005761bug_revision_view_page.php?bugnote_id=5761#r3167
2013-01-13 11:20Torr SamahoAssigned To => Torr Samaho
2013-01-13 11:20Torr SamahoStatusconfirmed => needs testing
2013-01-13 11:20Torr SamahoTarget Version => 1.1
2013-01-13 11:28ZalewaNote Added: 0005762
2013-01-13 11:31Torr SamahoNote Added: 0005763
2013-01-16 17:56DuskRelationship addedhas duplicate 0001252
2013-01-17 01:01Blzut3Note Added: 0005795
2013-01-17 01:06Blzut3Note Edited: 0005795bug_revision_view_page.php?bugnote_id=5795#r3176
2013-01-17 01:11DuskNote Added: 0005796
2013-01-26 13:51ZalewaNote Added: 0005825
2013-01-26 13:51ZalewaFile Added: randomguy_and_randomguy2.wad
2013-01-26 19:56Torr SamahoNote Added: 0005826
2013-01-26 20:15ZalewaNote Added: 0005827
2013-01-26 20:16ZalewaNote Edited: 0005827bug_revision_view_page.php?bugnote_id=5827#r3185
2013-01-27 08:30Torr SamahoNote Added: 0005829
2013-01-27 08:31ZalewaNote Added: 0005830
2013-04-03 19:44ArcoNote Added: 0006204
2013-04-05 19:52DuskStatusneeds testing => resolved
2013-04-05 19:52DuskFixed in Version => 1.1
2013-04-05 19:52DuskResolutionopen => fixed
2013-08-02 10:41DuskRelationship addedrelated to 0001162
2014-06-14 11:01DuskRelationship addedrelated to 0001832
2018-09-30 20:16Blzut3Statusresolved => closed

Notes
(0005465)
Dusk   
2012-12-03 14:10   
This is the see state:

  See:
    TROO A 0 A_Jump(256,"vile","head","skel","boss","imp","spid","fat")
    goto see


The client does not perform the jump until the server instructs it to, so it keeps looping. The stack is overflown before the client gets the order to jump.
(0005466)
ZzZombo   
2012-12-03 15:04   
Maybe use "wait" instead of "goto See"?
(0005467)
Zalewa   
2012-12-03 16:29   
ZzZombo: In my opinion if something works correctly offline then it should also work correctly online. No excuses about "WAD being bad".
(0005468)
Dusk   
2012-12-03 16:43   
It's a crash and should be addressed...
(0005469)
ZzZombo   
2012-12-03 16:59   
I just provide a possible workaround to the problem that can bu used unless you will make a patch, that's the point.
(0005481)
Dark-Assassin   
2012-12-07 21:31   
For online use, at least 1 tic is needed for A_Jump loops.
This code would have to be modified to compensate.
(0005759)
Torr Samaho   
2013-01-13 10:47   
As Dusk pointed out, the see state is not compatible with Zandronum's jump handling and I don't see any proper way to fix this without completely throwing away Carn's jump handling.

As workaround we could simply try to break infinite loops on the clients (always risking to break valid but very long state combinations of zero duration).
(0005760)
Zalewa   
2013-01-13 11:00   
(edited on: 2013-01-13 11:07)
Quote from "Torr Samaho"
As workaround we could simply try to break infinite loops on the clients (always risking to break valid but very long state combinations of zero duration).


This looks reasonable enough. After all, the clients should synchronize with server eventually (I hope they will) so the player would only see a desynch comparable to what happens with Lost Souls now. This is definitely better than a crash that freezes some computers so strongly that the only option left is to press the reset button. Furthermore a warning could be printed to console when this happens, preferably only once per single game/map, so the modder knows that something is wrong with his decorate.
(0005761)
Torr Samaho   
2013-01-13 11:18   
(edited on: 2013-01-13 11:19)
Since anything is better than a crash, clients now try to break infinite state loops of zero duration by not allowing any actor to go through more than 10000 states in one tic. This should fix the problems with randomguy. Since this is a very ugly workaround, a warning is printed every time the workaround kicks in.

EDIT: Posted this without seeing your latest response. Printing the warning only once could be a good compromise.
(0005762)
Zalewa   
2013-01-13 11:28   
I've tested your build and it appears to work correctly now.
(0005763)
Torr Samaho   
2013-01-13 11:31   
Thanks for checking!

The client side infinite loop workaround warning is now only printed once on each Zandronum instance. Would be nice if somebody confirms this change in the upcoming official 1.1 beta build.
(0005795)
Blzut3   
2013-01-17 01:01   
(edited on: 2013-01-17 01:06)
One thing to consider is it's probably fairly common to use A_Jump(256, ...) for jumping to some random state. Any states following these calls will never be run, so perhaps it might be wise to change the client version of A_Jump to set the state duration to -1 if maxchance >= 256 so it waits for the server information?

Edit: While on the topic of knowns in A_Jump. A_Jump(256, "SomeState") is sometimes used instead of 'goto SomeState' since it is resolved at runtime instead of "compile time." This would be another fairly simple condition to check if(maxchance >= 256 && count == 1) and the client would accurately predict this case.
(0005796)
Dusk   
2013-01-17 01:11   
I like that, though I think that should be taken to a new ticket.
(0005825)
Zalewa   
2013-01-26 13:51   
Tested with build "1.1-alpha-r130120-0905".

Two things I've noticed:
1. Reloading map with 'map' command doesn't reset the 'message was displayed' flag. In other words: to see the message again you have to close the process of the server and start a new one. This might be the intended behaviour, although.

2. When there are multiple offending actor types the message is displayed only for the first offender. Subsequent errors, even for other actors, are ignored silently. As an example I copied entire "RandomGuy" code and pasted it into "RandomGuy2". Summoning RandomGuy displays the message, but if RandomGuy2 is summoned next then the message is not displayed. Reverse summoning order yields the same result. I'm attaching WAD with these actors.
(0005826)
Torr Samaho   
2013-01-26 19:56   
That's the behavior I intended and what I meant with "is only printed once on each Zandronum instance" in the log. Printing it once per instance and per actor type would probably be nicer, but also is more complicated to implement. I decided to go with the simple implementation to have more time to work on other issues.
(0005827)
Zalewa   
2013-01-26 20:15   
(edited on: 2013-01-26 20:16)
Quote from "Torr Samaho"
That's the behavior I intended and what I meant with "is only printed once on each Zandronum instance" in the log.

Roger that.

Quote from "Torr Samaho"
Printing it once per instance and per actor type would probably be nicer, but also is more complicated to implement. I decided to go with the simple implementation to have more time to work on other issues.

I disagree on the complicated part.

std::set<std::string> actors;
if (actors.find(actorType) == actors.end())
{
    actors.insert(actorType);
    printWarning();
}

I don't consider this fixed until it treats each actor type individually.
(0005829)
Torr Samaho   
2013-01-27 08:30   
Using an std:set is more complicated than using a bool. Nevertheless, I agree that it's still simple enough and changed the warning code accordingly. It's just using the ActorNetworkIndex instead of the actor's name.
(0005830)
Zalewa   
2013-01-27 08:31   
Thanks, I'll test it once the next build comes up.
(0006204)
Arco   
2013-04-03 19:44   
It appears that this bug is fixed. I tested the wad with the latest version on my own server and it’s not causing any conflict that’s otherwise major. The only thing that I did noticed was a DECORATE offset at 14.