MantisBT - Zandronum
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0002707Zandronum[All Projects] Suggestionpublic2016-04-23 10:062017-05-14 18:22
ReporterLeonard 
Assigned ToTorr Samaho 
PrioritynormalSeverityminorReproducibilityalways
StatusfeedbackResolutionopen 
PlatformMicrosoftOSWindowsOS VersionXP/Vista/7
Product Version2.1 
Target VersionFixed in Version 
Summary0002707: A way to force clients to execute decorate jumps
DescriptionTo give you an example:
I was cleaning up some old decorate code and wanted to get rid of inventory item hacks and use user variables instead but now the problem is that the client never predicts any A_JumpIf jumps as opposed to what was used previously (A_JumpIfInventory with the player as owner).
I made sure the user variables are also changed at the same time on the client's end so there should be no problem with it predicting this specific jump.

As a second example:
I prefer to use "A_JumpIf(true, ...)" instead of "A_Jump(256, ...)" because it obviously looks much better but unfortunately only the latter is predicted by clients.
If you're wondering why someone would need to use any of those instead of just goto, this is because the goto keyword cannot perform virtual jumps (as stated on the wiki)

This is why we need a way to be able to specify a jump's predictability decorate-wise.
An optional third parameter to the A_JumpIf function would solve this.
Steps To Reproduce
Additional Information
TagsNo tags attached.
Relationships
Attached Files? jumppred.pk3 (508) 2016-04-24 15:26
/tracker/file_download.php?file_id=1793&type=bug
? acspred.pk3 (1,340) 2016-05-22 22:30
/tracker/file_download.php?file_id=1813&type=bug
Issue History
Date ModifiedUsernameFieldChange
2016-04-23 10:06LeonardNew Issue
2016-04-23 15:47Edward-sanStatusnew => feedback
2016-04-23 15:48Edward-sanNote Added: 0014744
2016-04-24 15:26LeonardFile Added: jumppred.pk3
2016-04-24 15:29LeonardNote Added: 0014748
2016-04-24 15:29LeonardStatusfeedback => new
2016-04-24 16:37Edward-sanNote Added: 0014749
2016-04-24 16:37Edward-sanAssigned To => Edward-san
2016-04-24 16:37Edward-sanStatusnew => assigned
2016-04-24 16:54Edward-sanNote Added: 0014750
2016-04-24 16:54Edward-sanStatusassigned => needs review
2016-04-24 17:14Edward-sanNote Edited: 0014750bug_revision_view_page.php?bugnote_id=14750#r8927
2016-04-24 17:16Edward-sanNote Edited: 0014750bug_revision_view_page.php?bugnote_id=14750#r8928
2016-04-24 18:26LeonardNote Added: 0014751
2016-04-24 19:57Edward-sanNote Added: 0014752
2016-04-25 10:42LeonardNote Added: 0014754
2016-05-14 14:59LeonardNote Added: 0014904
2016-05-14 15:01LeonardNote Edited: 0014904bug_revision_view_page.php?bugnote_id=14904#r9011
2016-05-15 07:48Torr SamahoNote Added: 0014925
2016-05-15 14:53LeonardNote Added: 0014932
2016-05-15 15:36ZzZomboNote Added: 0014933
2016-05-15 16:29Torr SamahoNote Added: 0014934
2016-05-15 18:36LeonardNote Added: 0014935
2016-05-16 13:44LeonardNote Added: 0014936
2016-05-17 05:00ZzZomboNote Added: 0014939
2016-05-17 18:58Torr SamahoNote Added: 0014943
2016-05-17 21:28LeonardNote Added: 0014949
2016-05-18 21:22Torr SamahoNote Added: 0014953
2016-05-22 22:30LeonardNote Added: 0014971
2016-05-22 22:30LeonardFile Added: acspred.pk3
2016-06-05 19:12Edward-sanAssigned ToEdward-san =>
2016-06-06 04:52StrikerMan780Note Added: 0015033
2016-06-06 04:53StrikerMan780Note Edited: 0015033bug_revision_view_page.php?bugnote_id=15033#r9073
2016-06-06 04:57StrikerMan780Note Edited: 0015033bug_revision_view_page.php?bugnote_id=15033#r9074
2016-06-06 04:59StrikerMan780Note Edited: 0015033bug_revision_view_page.php?bugnote_id=15033#r9075
2016-06-06 05:04StrikerMan780Note Edited: 0015033bug_revision_view_page.php?bugnote_id=15033#r9076
2016-06-06 05:22StrikerMan780Note Edited: 0015033bug_revision_view_page.php?bugnote_id=15033#r9077
2016-06-06 05:53StrikerMan780Note Edited: 0015033bug_revision_view_page.php?bugnote_id=15033#r9078
2016-06-06 05:56StrikerMan780Note Edited: 0015033bug_revision_view_page.php?bugnote_id=15033#r9079
2016-06-06 05:58StrikerMan780Note Edited: 0015033bug_revision_view_page.php?bugnote_id=15033#r9080
2016-06-06 06:11StrikerMan780Note Edited: 0015033bug_revision_view_page.php?bugnote_id=15033#r9081
2016-06-06 06:12StrikerMan780Note Edited: 0015033bug_revision_view_page.php?bugnote_id=15033#r9082
2016-06-06 06:14StrikerMan780Note Edited: 0015033bug_revision_view_page.php?bugnote_id=15033#r9083
2016-06-06 06:14StrikerMan780Note Edited: 0015033bug_revision_view_page.php?bugnote_id=15033#r9084
2016-06-06 06:19StrikerMan780Note Edited: 0015033bug_revision_view_page.php?bugnote_id=15033#r9085
2016-06-06 06:25StrikerMan780Note Edited: 0015033bug_revision_view_page.php?bugnote_id=15033#r9088
2016-06-06 06:27StrikerMan780Note Edited: 0015033bug_revision_view_page.php?bugnote_id=15033#r9089
2016-06-07 18:30IvanNote Added: 0015048
2016-06-26 13:54Torr SamahoNote Added: 0015127
2016-08-30 08:11FusedNote Added: 0015544
2016-08-30 18:17Torr SamahoNote Added: 0015546
2016-09-04 15:41Torr SamahoAssigned To => Torr Samaho
2016-09-04 15:41Torr SamahoStatusneeds review => feedback
2016-09-04 23:39DuskNote Added: 0015589
2016-09-06 07:15FusedNote Added: 0015603
2016-09-10 11:51Torr SamahoNote Added: 0015607
2016-11-20 21:50Edward-sanProduct Version2.2-beta => 2.1

Notes
(0014744)
Edward-san   
2016-04-23 15:48   
Can you provide some example wads for these problems?
(0014748)
Leonard   
2016-04-24 15:29   
I attached a small and simple example wad (jumppred.pk3)
When used online, the client doesn't show the pistol being fired but when used offline the jump is performed and you see the pistol being fired as intended.
(0014749)
Edward-san   
2016-04-24 16:37   
I've got an idea about this.
(0014750)
Edward-san   
2016-04-24 16:54   
(edited on: 2016-04-24 17:16)
'https://bitbucket.org/Torr_Samaho/zandronum/pull-requests/90/added-client-prediction-for-a_jumpif-true/diff [^]'

Build containing the change:'https://www.dropbox.com/s/29e5rrt8ji630na/zandronum-3.0-r160424-1647-075b823-windows.zip?dl=0 [^]'

BTW: this doesn't handle the other cases yet!
(0014751)
Leonard   
2016-04-24 18:26   
The binary does work with the "A_JumpIf(true, ..)" case.

As for the other cases, I was thinking: if adding a third parameter to A_JumpIf is too "invasive" or out of place, another solution could be to have a new zandronum-specific state keyword (like NoDelay) called "predict" or something similar which would indicate the possibility for clients to predict the action function for this state.
By the way do you need another example wad for the other cases?
(0014752)
Edward-san   
2016-04-24 19:57   
Yes, another example wad would be helpful, with a list of all possible and valid expressions you think it should be okay to predict, if possible.
(0014754)
Leonard   
2016-04-25 10:42   
Quote
another example wad would be helpful

I don't know if you saw already but here:
<TerminusEst13>: Reposting here for posterity.
<TerminusEst13>: Here's a couple mods that might be good for testing out the A_JumpIf tweaks:
<TerminusEst13>:'http://puu.sh/oudWD/a8c660f0b9.wad [^]' - Here's a wad that uses A_JumpIf(true) to have a shotgun replacement fire five times in a row.
<TerminusEst13>:'http://puu.sh/oudZ3/0fc04844e7.pk3 [^]' - Here's a more utilitarian test that uses A_JumpIf with an ACS check to check armor value on a chainsaw replacement pickup, giving armor if you don't have it and no armor if you don't.
<TerminusEst13>:'http://puu.sh/ouecF/8f4adfee2a.pk3 [^]' - Here's a more utilitarian test that uses A_JumpIf with an ACS check that swaps out the shotgun spawn depending on difficulty.


Quote
with a list of all possible and valid expressions you think it should be okay to predict

I honestly don't think the way to solve this is to deduce if the jump is predictable or not because it actually depends heavily on context which is why I suggested it to be controlled by modders.
Just take TerminusEst's examples: he's using ACS inside his expressions and you might think that predicting those ACS calls would be a bad idea but in the difficulty example it is predictable because the clients know about the skill level. In the second example however it could be predictable but only if it's the consoleplayer picking it up.
In my own example described in the ticket, I use user-variables: they could be synced with clients or not depending on how I changed them and if I made sure they were so it normally wouldn't be predictable but in my case it is as I synced them to clients using ACS.
You can't solve this by deducing a jump's predictability unless you let the author specify it.
(0014904)
Leonard   
2016-05-14 14:59   
(edited on: 2016-05-14 15:01)
Did a pull request.
As discussed on IRC with edward this only applies to a few action functions which I thought could make use of this in some cases because applying this to every action functions could lead to very big changes.
Though this can be easily extended to other action functions in the future if needed.

EDIT: I should mention that I did test my own changes with my own example and it seems to be working fine as it solved my problem.
(0014925)
Torr Samaho   
2016-05-15 07:48   
I agree that we probably have to let the author specify the predictability, but so far I'd say your original suggestion to add an optional third parameter to A_JumpIf is preferable to a new keyword. With the former it's clear to which function the predictability applies to. Not to mention that the latter will be much more problematic to merge with ZDoom's recent scripting code overhaul.
(0014932)
Leonard   
2016-05-15 14:53   
Well so far the latest zdoom still parses and stores state keywords the same way and can still be accessed the same way in action functions so as of right now there doesn't seem to be any problem with merging unless you're referring to possible future changes?
I would say that if we take in account the possibility of future changes, adding a third parameter seems like a big risk because zdoom could decide to add an actual third parameter and that would make a big mess for mod compatibility.

On another note, as I said this method is easily extensible to other action functions and I noticed this is also very useful for ACS_Execute stuff because it saves me a lot of time while trying to sync information on client/serverside and I'm not sure adding new parameters to such functions is a good idea since the ACS_Execute functions for example are internally converted to "A_CallSpecial" calls (which was covered in my pr).
(0014933)
ZzZombo   
2016-05-15 15:36   
There wouldn't be any big changes to Decorate at all. 1) It's going to be superseded and deprecated by Doom Script, so why bother? 2) Decorate needs to be in a backwards-compatible state, since the support for it is gonna be truncated due to the point #1.
(0014934)
Torr Samaho   
2016-05-15 16:29   
Quote from Leonard
Well so far the latest zdoom still parses and stores state keywords the same way and can still be accessed the same way in action functions so as of right now there doesn't seem to be any problem with merging unless you're referring to possible future changes?

I'm referring to the fact that CallingState is not available in the implementation of many of the code pointers anymore. I had to change our internal jump handling twice in the zdoom-sync branch because of different changes ZDoom did under the hood recently.

Any delta we have to ZDoom code that relies on internal structures not exposed to mods is bound to get in the way in the future. Considering how much I had to fight with all our existing delta to ZDoom when merging post ZDoom 2.8 changes, I'd really like to avoid additional delta relying on such internal structures.

Quote from Leonard
I would say that if we take in account the possibility of future changes, adding a third parameter seems like a big risk because zdoom could decide to add an actual third parameter and that would make a big mess for mod compatibility.

ZDoom already has numerous DECORATE placeholders (e.g. A_FaceConsoleplayer) to stay compatible with Zandronum, so I'd say it's possible that they also this argument as placeholder.

Quote from ZzZombo
There wouldn't be any big changes to Decorate at all.
I'm not talking about problems with the DECORATE interface mods use, I'm talking about the internal implementation of the DECORATE functions. ZDoom may overhaul this at any time.
(0014935)
Leonard   
2016-05-15 18:36   
What about the other action functions I mentioned like ACS_Execute stuff?
Do you suggest we should use an extra parameter for those too?
(0014936)
Leonard   
2016-05-16 13:44   
I thought about this a bit more and I think I came up with a new solution:
what about using an extra zandronum-specific flag like "CLIENTSIDEONLY" and allow modders to toggle it on clients without restriction with an action function like "A_SetPredict" or a similar name that isn't executed by the server?
This would then be accessed the same way any other network flags are accessed within action functions (with self->ulNetworkFlags).
Would that be acceptable?
(0014939)
ZzZombo   
2016-05-17 05:00   
I some time ago have already suggested to allow separate client/server actor logic. On an opt-in basis, so currently existing mods won't change. But if one goes through the process of rewriting code he gets all the benefits/headaches of the system.
(0014943)
Torr Samaho   
2016-05-17 18:58   
Quote from Leonard

I thought about this a bit more and I think I came up with a new solution:
what about using an extra zandronum-specific flag like "CLIENTSIDEONLY" and allow modders to toggle it on clients without restriction with an action function like "A_SetPredict" or a similar name that isn't executed by the server?
This would then be accessed the same way any other network flags are accessed within action functions (with self->ulNetworkFlags).
Would that be acceptable?

So far I fully agree with the second part of what you originally requested, i.e. to give the client some means to perform a goto for a virtual jump. That's definitely an important basic functionality. An additional argument to A_JumpIf would allow this (and a little more).

Can you give a specific example where you absolutely need A_SetPredict (or something that is equivalent)?
(0014949)
Leonard   
2016-05-17 21:28   
A_SetPredict could be used with other jumping functions like A_JumpIfInventory if the author synced the inventory in question with clients and maybe even A_Warp for example. It will also be usable with "A_Jump(256, x)" since apparently that got removed too recently.
Like I said, it could also be used with the ACS_Execute functions which can save a lot of time while trying to sync stuff with clients ACS-wise.
It could also be used with A_GiveInventory and A_TakeInventory.
And of course, it could be extended to any other action functions later on if needed.
(0014953)
Torr Samaho   
2016-05-18 21:22   
That's still a quite general. I'm asking for a really specific example where you would need this kind of functionality to have a convincing usage case.
(0014971)
Leonard   
2016-05-22 22:30   
I attached another example wad, in it you have a weapon called "newweapon" which will transform into a powered-up version when you use altfire with it.
If you host it on a server on which you have at least 100 ping, enable cheats to give yourself "newweapon" and then quickly altfire then fire it, you may notice a small desync: your client will show you the unpowered-up sprites when it's actually the powered-up version being fired which you can see by looking at the puffs appearing.
The server will of course correct that little desync but it will be too late as your client will have already jumped giving you the time to notice the desync.
This could have been avoided if the ACS_Execute call in the weapon was predicted by clients.
(0015033)
StrikerMan780   
2016-06-06 04:52   
(edited on: 2016-06-06 06:27)
Speaking of jumping, there *REALLY* needs to be a way to predict normal random A_Jump commands. I think it can be done, and should be done... the current jumping behavior is absolute buggy trash.

I've been wondering if it's possible to write a special function for random number generation for A_Jump to use exclusively, that isn't tied to the gamesim like most of the RNG is currently (AFAIK). For every A_Jump call, a seed is generated in a predictable fashion, and synchronized between client and server. The server should refresh the seed with the client on occasion, or on connection interruption/packet loss to make sure things remain consistent.

Or, something more universal could be done. (Which would be a bit more work, But it would pay off with much more reliable/predictable behavior around the board for any function that may need it).

On that note, I have some functions of interest from Source that I gathered a while back when trying to figure out the possibility of synchronizing puffs and decals between client/server in Zan, that may provide as a source of inspiration. Of course, the engines are different, but there's some conceptual similarities.

It involves each player and actor storing their own random seeds (Generated using an MD5 PseudoRandom seed generator that's fed the current net/user command number) which should produce the same result on both client/server, which is then retrieved when necessary and the RNG reinitialized with said seed for functions such as bullet spread, random jumps, etc.

EDIT: Here'http://pastebin.com/Nr6cGuZL [^]'
(the RandomSeed function would be the same as the FRandom::Init function in Zandronum, I do believe.)
(0015048)
Ivan   
2016-06-07 18:30   
I'm wondering one thing. We've been having many weapon desyncs with weapons utilizing A_JumpIf. (Not sure about A_JumpIfInventory) Maybe related?
(0015127)
Torr Samaho   
2016-06-26 13:54   
Quote from StrikerMan780
I've been wondering if it's possible to write a special function for random number generation for A_Jump to use exclusively, that isn't tied to the gamesim like most of the RNG is currently (AFAIK).

This is not impossible, but it's incredibly complicated to make it work reliably considering how our c/s architecture works combined with the amount of freedom ZDoom's DECORATE expression evaluator allows for. If anybody wants to give this a try, be my guest, but be prepared for a huge undertaking.
(0015544)
Fused   
2016-08-30 08:11   
Sorry to bump this but there should really be a way to support this. I too need this very badly and I understand the difficulty behind this but leaving such useful feature just limits decorate so much next to what it already is.
(0015546)
Torr Samaho   
2016-08-30 18:17   
Please post a very specific usage example that is impossible to implement without this.
(0015589)
Dusk   
2016-09-04 23:39   
Quote

This is not impossible, but it's incredibly complicated to make it work reliably considering how our c/s architecture works combined with the amount of freedom ZDoom's DECORATE expression evaluator allows for. If anybody wants to give this a try, be my guest, but be prepared for a huge undertaking.

I actually took a shot at this a long time ago. The conclusion was that synchronising like this cannot very feasibly be done.

For instance, suppose the server goes through 1 million A_Jump calls. The client would have to be given the seed value, and roll count of 1 million, which could possibly cause a freeze on the client.

Furthermore, the client never has the exact same game state as the server. Therefore the client will always be behind the server as far as RNG calls go and may desync enough to not even to call A_Jump the same amount of times as the server. So it will go out of sync very quickly.
(0015603)
Fused   
2016-09-06 07:15   
> Please post a very specific usage example that is impossible to implement without this.

Not exactly impossible, just very time consuming. Imagine all the inventory jumps while it can just be a single jump using a variable. There's much usefullness in this, especially in my case.
(0015607)
Torr Samaho   
2016-09-10 11:51   
Then please post a very specific usage example where this would radically simplify the implementation.