0000099: monsters move east online if 'A_Chase' is called when idling

Notes
(0002731) TIHan (reporter) 2012-03-02 08:32 edited on: 2012-03-02 14:20	Using the wad, the zombiemen facing east is normal as it happens in single-player (You can test that right when you open a map through a console and type notarget). This happens the same in latest ZDoom. The real issue here is our spawn state calls A_Chase. If no idle state is provided, spawn will be our idle state. The reason why the zombiemen don't move on the server side is because it checks for a target in A_DoChase that doesn't exist, so it doesn't perform a P_Move; edit: on the client side, it sets that actor's state to idle, which is actually its spawn state with A_Chase - so then, the client executes A_DoChase and skips the target check and performs the P_Move which explains why the actors move forward. Not sure how to handle this properly yet without making new SERVERCOMMANDS.

(0002740) TIHan (reporter) 2012-03-03 00:41	This should fix this problem.'https://bitbucket.org/TIHan/tst/changesets/tip/branch(%22bug_99%22) [^]' I did have to allow transferring network flags over from server to client to get this to work correctly without doing anything crazy.

(0002744) Torr Samaho (administrator) 2012-03-03 17:55	> I did have to allow transferring network flags over from server to client to get this to work correctly without doing anything crazy. FYI, these flags are not supposed to be synchronized between client and server. For instance, actors on the server should never have the NETFL_CLIENTSIDEONLY flag and actors on the client should never have the NETFL_SERVERSIDEONLY flag. I didn't have time to look too closely at what exactly you are trying to achieve, but if you want to (partially) sync the target of actors on clients and server you should base this on SERVERCOMMANDS_SetThingTarget instead of creating another NETFL as workaround. Of course you will need to alter SERVERCOMMANDS_SetThingTarget slightly since it doesn't allow to set the target to NULL at the moment.

(0002746) TIHan (reporter) 2012-03-03 18:37 edited on: 2012-03-03 18:49	> FYI, these flags are not supposed to be synchronized between client and server. For instance, actors on the server should never have the NETFL_CLIENTSIDEONLY flag and actors on the client should never have the NETFL_SERVERSIDEONLY flag. I didn't know this. I'll look into SetThingTarget then. Is it ok if I created something like SetThingTargetNull? Or an extra parameter on SetThingTarget?

(0002747) Torr Samaho (administrator) 2012-03-03 20:08	> Is it ok if I created something like SetThingTargetNull? Or an extra parameter on SetThingTarget? It should be sufficient if you extend SetThingTarget to work in case target==NULL. For this you should get rid of the EnsureActorHasNetID(pActor->target) and simply sent -1 as target netID if pActor->target == NULL or pActor->target doesn't have a valid netID.

(0002748) TIHan (reporter) 2012-03-03 21:20 edited on: 2012-03-03 22:47	> It should be sufficient if you extend SetThingTarget to work in case target==NULL. For this you should get rid of the EnsureActorHasNetID(pActor->target) and simply sent -1 as target netID if pActor->target == NULL or pActor->target doesn't have a valid netID. 'https://bitbucket.org/TIHan/tst/changesets/tip/branch(%22bug_99_second%22) [^]' Wow, this is a much simpler solution than I had before. :( Software development is hard. I really need to stay away from the network flags. I should only use them if there is absolutely no other way to solve the problem. The only thing I see wrong with this right now, is that its going to use more bandwidth. The previous solution I had only sends an extra command to the clients if target changes from null to not null or vice versa. How should I handle this?

(0002749) Torr Samaho (administrator) 2012-03-04 16:37	> Wow, this is a much simpler solution than I had before. :( Software development is hard. It's definitely not trivial ;-) and experience is very helpful. Regarding your new fix, while it's much simpler it still looks weird: You let the server tell the actor's target to the client just to have the client NULL the target on its end? The only thing that seems different to me now is that actor->threshold is handled differently on the client.

(0002750) TIHan (reporter) 2012-03-04 18:02 edited on: 2012-03-04 18:08	> Regarding your new fix, while it's much simpler it still looks weird: You let the server tell the actor's target to the client just to have the client NULL the target on its end? The only thing that seems different to me now is that actor->threshold is handled differently on the client. I'll do my best to explain this. The shortest explanation: The original/BC solution was to set the target to NULL on the client instead of having to do conditional NETWORK_GetState( ) != NETSTATE_CLIENT's everywhere. So, if the client actually has a target, the client will perform its own movement/directions. Setting the client target to NULL it doesn't perform anything. :) This is why I set the target back to NULL so it doesn't do that. While my solution is weird, it does its job by knowing about if the server actor's target is NULL or not. The solution to this bug is this: When the server actor's target is NULL, it doesn't move. The client needs to know that the server actor's target is NULL, and should not move, or effectively return and remove inchase flag. Both my solutions do just that in entirely different ways.

(0002753) Torr Samaho (administrator) 2012-03-05 01:29	The idea behind Carn's monster movement system is that the client relies on the server to determine the direction of movement. Instead of trying to tell the clients that the server doesn't have a target for the actor, you could try to set the movement dir to DI_NODIR and tell that to the clients. PS: I'm basing my suggestion on your description, I didn't check the code if setting the movement dir would work or not.

(0002754) TIHan (reporter) 2012-03-05 03:22 edited on: 2012-03-05 03:40	Ready for a third branch? :D'https://bitbucket.org/TIHan/tst/changesets/tip/branch(%22bug_99_third%22) [^]' Blzut3 mentioned me about DI_NODIR as well. Seems to work fine. The only thing I see something wrong with it, is that there is a possibility that the client actor will face a different direction than on the server. So far, I haven't seen that happen yet.

(0002755) Torr Samaho (administrator) 2012-03-06 00:18	Yeah, that looks pretty good now :). One final thing, we should try to let the server only tell the clients to set movedir to DI_NODIR when the target became NULL, i.e. if the target already was NULL before, there shouldn't be any need for this message. To do so you can create a new bool in the first line of A_DoChase with something like "const bool bStartedWithTarget = ( actor->target != NULL );".

(0002758) TIHan (reporter) 2012-03-06 06:10 edited on: 2012-03-06 13:59	> One final thing, we should try to let the server only tell the clients to set movedir to DI_NODIR when the target became NULL, i.e. if the target already was NULL before, there shouldn't be any need for this message. The problem with this is that the target will be NULL before it even runs A_DoChase when you join the game; therefore, our bug will still occur.

(0002763) Torr Samaho (administrator) 2012-03-07 01:19	> The problem with this is that the target will be NULL before it even runs A_DoChase when you join the game; therefore, our bug will still occur. As a general remark: If newly connecting clients are missing some kind of information they need to be supplied with it when the connect. Sending that information redundantly to clients already connected to fix this kind of connect issues is a waste of nettraffic. Now that you mention this here, I noticed that connecting clients were not informed about the current movedir (which is fixed now). I also did some testing with the "DI_NODIR approach" and noticed that it fixes the movement issue, but it doesn't fix the sound problem. So the fix unfortunately still needs some fine tuning.

(0002766) TIHan (reporter) 2012-03-07 06:24	> Sending that information redundantly to clients already connected to fix this kind of connect issues is a waste of nettraffic. I do understand this concept and reasoning why you wanted something such as bStartedWithTarget, I was just stating that our bug would still occur. If you remember my first attempt at this bug, it didn't waste any net traffic ( though bad implementation ) Also, the DI_NODIR fix there - I managed to connect and some zombiemen were facing the wrong directions (only happened on first connect, reconnecting they were facing the correct direction).'http://i.imgur.com/7KSvl.png [^]' - As with this custom wad, they should be facing east. This is also with your fix for informing clients on connect for movedir. I'll do my best to explain what happened here: The server first hosted. Nothing, such as A_DoChase was ran. When I connected for the first time, then the clock starts tic'ing, it takes a few tics to move the zombiemen in the proper direction (east). The server tells the client the current movedir, then eventually the server will tell the client the movedir is DI_NODIR (target is NULL), probably within the same tic. At this point, the calculations on the client to determine what angle the actor is, is based on its movedir, which now is DI_NODIR - so we get some weird results on first connect. On the reconnect, since the server already ran A_DoChase and actor is at its proper angle, the client will get the angle informed by the server; therefore, it is now and will always be in the correct direction. Thinking about the DI_NODIR way of solving this, I feel a little uneasy about it. To me, we are setting the movedir on the client to DI_NODIR as a way to stop movement if the server's target was NULL, and potentially a way to stop playing the ActiveSound. The desync here between the movedir on the server and client can cause interesting results in very specific cases. Btw, we do not want to forcefully set the actor's movedir on the server to DI_NODIR if the actor's target is NULL, as this would change the effect intended by ZDoom. Eh, I'm sorry for being picky here. :( I just want a solution that we can both agree on and makes the most sense for both of us. I'm thinking about going back to the getting targets in sync, but blocking clients from running certain actions when a target is not NULL. But I'm afraid I will have issues on how to handle the net traffic, unless I create a network flag. Or there is the other crazy option from the first solution - but we do not want that. That's something that hasn't even been done in the source anyway. .. God, software really is hard.'http://www.cioinsight.com/c/a/Expert-Voices/Scott-Rosenberg-What-Makes-Software-So-Hard/ [^]'

(0002773) Torr Samaho (administrator) 2012-03-11 01:04	> Thinking about the DI_NODIR way of solving this, I feel a little uneasy about it. To me, we are setting the movedir on the client to DI_NODIR as a way to stop movement if the server's target was NULL, and potentially a way to stop playing the ActiveSound. Possibly the DI_NODIR approach is not the way to go, I guess I have to look into this in more detail. > The desync here between the movedir on the server and client can cause interesting results in very specific cases. Anything other than the angle problems you mentioned above? > Btw, we do not want to forcefully set the actor's movedir on the server to DI_NODIR if the actor's target is NULL, as this would change the effect intended by ZDoom. I fully agree. > Eh, I'm sorry for being picky here. :( I just want a solution that we can both agree on and makes the most sense for both of us. No need to excuse. I'm interested in a proper fix too. > .. God, software really is hard In particular when you are trying to debug a huge code base other people wrote...

(0002828) TIHan (reporter) 2012-03-18 21:33 edited on: 2012-03-18 21:34	Looking into a proper way to solve this, which I think syncing targets would be appropriate. The problem is trying to figure out when to tell the clients that the target has changed and not let the server continually send the actor's target every tick. 'https://bitbucket.org/TIHan/tst/changesets/tip/branch(%22bug_99_fourth%22) [^]' This branch at the moment doesn't fix the issue, but it will try to aid in syncing targets without using a ton of bandwidth and be extremely easy on maintenance. What do you think of this?

(0002831) Torr Samaho (administrator) 2012-03-18 22:38	I'm very hesitant to syncing the targets. The monster movement code works just fine for the Doom monsters for instance. Syncing the targets would definitely increase net traffic, even for the Doom monsters. And for those it's just a complete waste of bandwidth. > This branch at the moment doesn't fix the issue, but it will try to aid in syncing targets without using a ton of bandwidth and be extremely easy on maintenance. What do you think of this? IMHO the proper way to sync a value would be to use access functions (e.g. add set/getTarget to AActor and make target private. Unfortunately this requires changes to the ZDoom base... Regarding your branch, I can't really see where this is heading yet since the important part, i.e. the actual notification of the client is still missing. I guess you plan to call all the "NOTIFY_CLIENT(AActor, Target)" functions every tic? If so, this could introduce some timing issues since all other notifications are made immediately.

(0002833) TIHan (reporter) 2012-03-18 22:50 edited on: 2012-03-18 23:08	> IMHO the proper way to sync a value would be to use access functions (e.g. add set/getTarget to AActor and make target private. I agree, but like you said it would require changing the ZDoom base - which is why I came up with NOTIFY_CLIENT. > I guess you plan to call all the "NOTIFY_CLIENT(AActor, Target)" functions every tic? If so, this could introduce some timing issues since all other notifications are made immediately. Originally I thought about calling NotifyClientOfTargetChange on every tick (in AActor::Tick) - but you can call it after when A_DoChase executes. Edit:'https://bitbucket.org/TIHan/tst/changeset/a28771366f34 [^]' This shouldn't have any timing issues. > I'm very hesitant to syncing the targets. The monster movement code works just fine for the Doom monsters for instance. Syncing the targets would definitely increase net traffic, even for the Doom monsters. And for those it's just a complete waste of bandwidth. If syncing targets doesn't float, coming up with a boolean for the actor of whether or not he has a target would be another way.

(0002837) Torr Samaho (administrator) 2012-03-18 23:59	> Edit:https://bitbucket.org/TIHan/tst/changeset/a28771366f34 [^] This shouldn't have any timing issues. This implicitly makes the assumption that the only code part that may alter the target of an actor is the code in A_DoChase above the notifyClientOfTargetChange call... > If syncing targets doesn't float, coming up with a boolean for the actor of whether or not he has a target would be another way. Which essentially would bring us back to your very first version of the fix. Perhaps it is the "least bad" way to fix this we have discussed so far. Since this bug is not very critical we probably should just let it rest for a while and return to it with a fresh mind. There are lots of other problems to work on ;).

(0002839) TIHan (reporter) 2012-03-19 00:13	> Since this bug is not very critical we probably should just let it rest for a while and return to it with a fresh mind. There are lots of other problems to work on ;). Cool. Whenever we come back to it, there will be a lot of information pertaining to it in the bug tracker. :)

(0009100) Watermelon (developer) 2014-06-11 21:20	I'm pretty sure 2.0 solved this. Will re-open if not the case

(0012567) unknownna (updater) 2015-06-06 21:52	This issue was never fixed.

(0012573) Edward-san (developer) 2015-06-06 22:26	Does this require any ping emulation?

(0012574) unknownna (updater) 2015-06-06 22:40	No, just host the WAD on a server and connect a client to the server.

Only registered users can voice their support. Click here to register, or here to log in.
Supporters:	Combinebobnt unknownna
Opponents:	No one explicitly opposes this issue yet.

Issue History
Date Modified	Username	Field	Change
2010-10-11 10:22	unknownna	New Issue
2010-10-11 10:22	unknownna	File Added: spawn_chase_bug_test.wad
2012-03-02 08:32	TIHan	Note Added: 0002731
2012-03-02 14:20	TIHan	Note Edited: 0002731	View Revisions
2012-03-03 00:41	TIHan	Note Added: 0002740
2012-03-03 17:55	Torr Samaho	Note Added: 0002744
2012-03-03 18:37	TIHan	Note Added: 0002746
2012-03-03 18:46	TIHan	Note Edited: 0002746	View Revisions
2012-03-03 18:49	TIHan	Note Edited: 0002746	View Revisions
2012-03-03 20:08	Torr Samaho	Note Added: 0002747
2012-03-03 21:20	TIHan	Note Added: 0002748
2012-03-03 22:42	TIHan	Note Edited: 0002748	View Revisions
2012-03-03 22:47	TIHan	Note Edited: 0002748	View Revisions
2012-03-04 16:37	Torr Samaho	Note Added: 0002749
2012-03-04 18:02	TIHan	Note Added: 0002750
2012-03-04 18:08	TIHan	Note Edited: 0002750	View Revisions
2012-03-05 01:29	Torr Samaho	Note Added: 0002753
2012-03-05 03:22	TIHan	Note Added: 0002754
2012-03-05 03:39	TIHan	Note Edited: 0002754	View Revisions
2012-03-05 03:40	TIHan	Note Edited: 0002754	View Revisions
2012-03-06 00:18	Torr Samaho	Note Added: 0002755
2012-03-06 06:10	TIHan	Note Added: 0002758
2012-03-06 06:11	TIHan	Note Edited: 0002758	View Revisions
2012-03-06 13:59	TIHan	Note Edited: 0002758	View Revisions
2012-03-07 01:19	Torr Samaho	Note Added: 0002763
2012-03-07 06:24	TIHan	Note Added: 0002766
2012-03-11 01:04	Torr Samaho	Note Added: 0002773
2012-03-18 21:33	TIHan	Note Added: 0002828
2012-03-18 21:34	TIHan	Note Edited: 0002828	View Revisions
2012-03-18 22:38	Torr Samaho	Note Added: 0002831
2012-03-18 22:50	TIHan	Note Added: 0002833
2012-03-18 23:08	TIHan	Note Edited: 0002833	View Revisions
2012-03-18 23:59	Torr Samaho	Note Added: 0002837
2012-03-19 00:13	TIHan	Note Added: 0002839
2012-06-09 13:22	Torr Samaho	Category	General => Bug
2014-06-11 21:20	Watermelon	Note Added: 0009100
2014-06-11 21:20	Watermelon	Status	new => closed
2014-06-11 21:20	Watermelon	Resolution	open => fixed
2015-06-06 21:52	unknownna	Note Added: 0012567
2015-06-06 21:52	unknownna	Status	closed => feedback
2015-06-06 21:52	unknownna	Resolution	fixed => reopened
2015-06-06 22:26	Edward-san	Note Added: 0012573
2015-06-06 22:40	unknownna	Note Added: 0012574
2015-06-06 22:40	unknownna	Status	feedback => new
2015-08-09 12:58	unknownna	Relationship added	parent of 0002338