0001654: Compatflag zd jump physics is broken

(0007946)
AlexMax (developer)
2014-01-11 03:57

Found the issue. For some reason, gravity is applied twice for a single tic when running off a ledge, and this behavior did not exist in old versions of ZDoom.

I've opened a pull request to fix the issue and also to name the compatflag something sane.

'https://bitbucket.org/Torr_Samaho/zandronum/pull-request/30 [^]'

(0007948)
Torr Samaho (administrator)
2014-01-11 11:22

The behavior patch itself looks good, but I think we still need to discuss about the name change. I'm pretty sure Dusk will say that "old" in the name is redundant since it's a compat flag ;-). Also it's not clear what kind of old behavior the flag is emulating. The goal of this flag was to be consistent with ZDoom 1.23B33 (no matter which other ZDoom versions also had the same behavior), so I'd say keeping the version in the name instead of old would be good. The change from "jumpphysics" to "zmovement" sounds reasonable though.

(0007951)
Dusk (developer)
2014-01-11 14:33

Why not just compat_zmovement? We don't have compat_vanilla19hitscan or compat_vanilla1666plasmabump or other stupidity like that either.

(0007952)
AlexMax (developer)
2014-01-11 16:37

I dig that. The version number is what bugged me, but removing 'old' is fine too.

(0007953)
Torr Samaho (administrator)
2014-01-11 16:45

Alright, then let's call it compat_zmovement and hope nobody will ever want the z-movement of any other version to be restored.

(0007954)
AlexMax (developer)
2014-01-11 17:03

Once I figure out how to modify pull requests (or just open a new one), I'll make that change.

For what it's worth, that behavior of applying gravity twice is consistent with Vanilla. It's only Old ZDoom that is the odd one out here.

(0007955)
Torr Samaho (administrator)
2014-01-11 17:14

Now that we are explicitly breaking with Vanilla Doom to cater to old school fans, hopefully they become aware that it's not Vanilla Doom they want but a certain, ancient ZDoom version ZDaemon is based on. In this light, it would definitely make sense to have a ZDoom reference in the name of the compat flag, since it's not about restoring vanilla behavior anymore.

(0007961)
AlexMax (developer)
2014-01-11 20:01
edited on: 2014-01-11 20:02

compat_oldzdoomzmovement ?

If you want to name-drop zdoom, it's probably best to indicate that it's aiming to be compatible with OLD ZDoom, not modern ZDoom, which is why I'm not a fan of just `compat_zdoomzmovement`.

(0007963)
Torr Samaho (administrator)
2014-01-11 20:04

I'd be fine with compat_oldzdoomzmovement. Just to remind the users every time the use the flag, that they don't want Vanilla but old ZDoom :).

(0007964)
Konar6 (reporter)
2014-01-11 20:13

I don't like dropping the word "old" from compatflags that restore "old" behavior, for consistency with existing dmflags, and also not all the compatflags(2) restore "old" behavior (eg. "NET scripts are clientside").

(0007974)
Torr Samaho (administrator)
2014-01-12 10:38

Ok, please change the name to compat_oldzdoomzmovement and add a history entry. Since I'd like to keep the history linear, please apply the name change to c48879d0395af766ea438fd4d0ff63080bd359be instead of 15a7d1375de1e2ff718e9713762024842a96cc44. Then I can transplant your changes.

(0007981)
AlexMax (developer)
2014-01-12 18:05

I'm not sure I follow.

15a7d13 is the 'namechange' commit, c48879d is the 'fix' commit. If I commit against c48879d, that creates a branch and I need to create a new pull request. If I commit against 15a7d13, that allows me to keep the pull request and also keeps the history linear.

Did you mix up the two commit hashes, or am I missing something?

(0007986)
Torr Samaho (administrator)
2014-01-12 20:19

I'd like to keep the history not only linear, but also clean in the sense that changes that are not intended to be used at all, don't end up in our main repository. So I don't intend to pull the namechange commit 15a7d13, that's why I'd like you to commit your new name change against c48879d.

To keep the history linear, I won't be able to use bitbucket's pull mechanism anyway but need to either rebase or transplant your changesets since they are not based on Zandronum's current head. I may be working against some of the DVCS principles here, but since we have a master repository I think it's no problem to do so and the linear and clean history is worth the extra effort. The linearization via transplanting won't take me more than a minute of work anyway and keeps the commit meta data intact.

(0007989)
AlexMax (developer)
2014-01-12 21:19

I dig. Just wanted to make sure that you and I were on the same page.

'https://bitbucket.org/Torr_Samaho/zandronum/pull-request/31 [^]'

(0007991)
Torr Samaho (administrator)
2014-01-12 22:14

Thanks! I transplanted your changeset to Zandronum's head.

(0007999)
Dusk (developer)
2014-01-14 08:25

Shouldn't this be also committed to 1.3?

(0008016)
AlexMax (developer)
2014-01-15 22:57

I'm fine with the change being in 1.3 - I thought that 2.0 was the priority and 1.3 was a contingency plan just in case the 2.0 rollout didn't go as planned.

(0008022)
Torr Samaho (administrator)
2014-01-16 07:13

See 0001110:0008021. Here, we additionally have the problem that the internal flag names have been changed and shuffled in 2.0, which makes the porting more cumbersome.

(0009796)
Arco (updater)
2014-06-29 20:51

Tested with v1.3 140610-2242. The jump behavior appears to be the same as 1.2 with both of the flags on. I was not able to move from ledge to ledge the majority of the time in the wad that was provided in the ticket.

(0011897)
Ru5tK1ng (updater)
2015-03-26 03:04

I can confirm that gravity no longer is added twice when you run off a ledge as of 1.3. I verified the wad example in the description to be working properly as intended.

(0012958)
unknownna (updater)
2015-07-17 12:58

Yes, this issue is now fixed indeed. The EonDM MAP03 ledge jump works fine and the IDL MAP26 reverse window SR50 jump still doesn't work, so it works as intended.

This issue is already marked as resolved. If you feel that is not the case, please reopen it and explain why.
Supporters:	Ru5tK1ng
Opponents:	No one explicitly opposes this issue yet.

Notes
(0007946) AlexMax (developer) 2014-01-11 03:57	Found the issue. For some reason, gravity is applied twice for a single tic when running off a ledge, and this behavior did not exist in old versions of ZDoom. I've opened a pull request to fix the issue and also to name the compatflag something sane. 'https://bitbucket.org/Torr_Samaho/zandronum/pull-request/30 [^]'

(0007948) Torr Samaho (administrator) 2014-01-11 11:22	The behavior patch itself looks good, but I think we still need to discuss about the name change. I'm pretty sure Dusk will say that "old" in the name is redundant since it's a compat flag ;-). Also it's not clear what kind of old behavior the flag is emulating. The goal of this flag was to be consistent with ZDoom 1.23B33 (no matter which other ZDoom versions also had the same behavior), so I'd say keeping the version in the name instead of old would be good. The change from "jumpphysics" to "zmovement" sounds reasonable though.

(0007951) Dusk (developer) 2014-01-11 14:33	Why not just compat_zmovement? We don't have compat_vanilla19hitscan or compat_vanilla1666plasmabump or other stupidity like that either.

(0007952) AlexMax (developer) 2014-01-11 16:37	I dig that. The version number is what bugged me, but removing 'old' is fine too.

(0007953) Torr Samaho (administrator) 2014-01-11 16:45	Alright, then let's call it compat_zmovement and hope nobody will ever want the z-movement of any other version to be restored.

(0007954) AlexMax (developer) 2014-01-11 17:03	Once I figure out how to modify pull requests (or just open a new one), I'll make that change. For what it's worth, that behavior of applying gravity twice is consistent with Vanilla. It's only Old ZDoom that is the odd one out here.

(0007955) Torr Samaho (administrator) 2014-01-11 17:14	Now that we are explicitly breaking with Vanilla Doom to cater to old school fans, hopefully they become aware that it's not Vanilla Doom they want but a certain, ancient ZDoom version ZDaemon is based on. In this light, it would definitely make sense to have a ZDoom reference in the name of the compat flag, since it's not about restoring vanilla behavior anymore.

(0007961) AlexMax (developer) 2014-01-11 20:01 edited on: 2014-01-11 20:02	compat_oldzdoomzmovement ? If you want to name-drop zdoom, it's probably best to indicate that it's aiming to be compatible with OLD ZDoom, not modern ZDoom, which is why I'm not a fan of just `compat_zdoomzmovement`.

(0007963) Torr Samaho (administrator) 2014-01-11 20:04	I'd be fine with compat_oldzdoomzmovement. Just to remind the users every time the use the flag, that they don't want Vanilla but old ZDoom :).

(0007964) Konar6 (reporter) 2014-01-11 20:13	I don't like dropping the word "old" from compatflags that restore "old" behavior, for consistency with existing dmflags, and also not all the compatflags(2) restore "old" behavior (eg. "NET scripts are clientside").

(0007974) Torr Samaho (administrator) 2014-01-12 10:38	Ok, please change the name to compat_oldzdoomzmovement and add a history entry. Since I'd like to keep the history linear, please apply the name change to c48879d0395af766ea438fd4d0ff63080bd359be instead of 15a7d1375de1e2ff718e9713762024842a96cc44. Then I can transplant your changes.

(0007981) AlexMax (developer) 2014-01-12 18:05	I'm not sure I follow. 15a7d13 is the 'namechange' commit, c48879d is the 'fix' commit. If I commit against c48879d, that creates a branch and I need to create a new pull request. If I commit against 15a7d13, that allows me to keep the pull request and also keeps the history linear. Did you mix up the two commit hashes, or am I missing something?

(0007986) Torr Samaho (administrator) 2014-01-12 20:19	I'd like to keep the history not only linear, but also clean in the sense that changes that are not intended to be used at all, don't end up in our main repository. So I don't intend to pull the namechange commit 15a7d13, that's why I'd like you to commit your new name change against c48879d. To keep the history linear, I won't be able to use bitbucket's pull mechanism anyway but need to either rebase or transplant your changesets since they are not based on Zandronum's current head. I may be working against some of the DVCS principles here, but since we have a master repository I think it's no problem to do so and the linear and clean history is worth the extra effort. The linearization via transplanting won't take me more than a minute of work anyway and keeps the commit meta data intact.

(0007989) AlexMax (developer) 2014-01-12 21:19	I dig. Just wanted to make sure that you and I were on the same page. 'https://bitbucket.org/Torr_Samaho/zandronum/pull-request/31 [^]'

(0007991) Torr Samaho (administrator) 2014-01-12 22:14	Thanks! I transplanted your changeset to Zandronum's head.

(0007999) Dusk (developer) 2014-01-14 08:25	Shouldn't this be also committed to 1.3?

(0008016) AlexMax (developer) 2014-01-15 22:57	I'm fine with the change being in 1.3 - I thought that 2.0 was the priority and 1.3 was a contingency plan just in case the 2.0 rollout didn't go as planned.

(0008022) Torr Samaho (administrator) 2014-01-16 07:13	See 0001110:0008021. Here, we additionally have the problem that the internal flag names have been changed and shuffled in 2.0, which makes the porting more cumbersome.

(0009796) Arco (updater) 2014-06-29 20:51	Tested with v1.3 140610-2242. The jump behavior appears to be the same as 1.2 with both of the flags on. I was not able to move from ledge to ledge the majority of the time in the wad that was provided in the ticket.

(0011897) Ru5tK1ng (updater) 2015-03-26 03:04	I can confirm that gravity no longer is added twice when you run off a ledge as of 1.3. I verified the wad example in the description to be working properly as intended.

(0012958) unknownna (updater) 2015-07-17 12:58	Yes, this issue is now fixed indeed. The EonDM MAP03 ledge jump works fine and the IDL MAP26 reverse window SR50 jump still doesn't work, so it works as intended.

Issue History
Date Modified	Username	Field	Change
2014-01-10 21:29	Decay	New Issue
2014-01-11 03:57	AlexMax	Note Added: 0007946
2014-01-11 11:22	Torr Samaho	Note Added: 0007948
2014-01-11 11:22	Torr Samaho	Product Version	=> 1.2
2014-01-11 11:23	Torr Samaho	Assigned To	=> Torr Samaho
2014-01-11 11:23	Torr Samaho	Status	new => feedback
2014-01-11 14:33	Dusk	Note Added: 0007951
2014-01-11 16:37	AlexMax	Note Added: 0007952
2014-01-11 16:45	Torr Samaho	Note Added: 0007953
2014-01-11 17:03	AlexMax	Note Added: 0007954
2014-01-11 17:14	Torr Samaho	Note Added: 0007955
2014-01-11 20:01	AlexMax	Note Added: 0007961
2014-01-11 20:02	AlexMax	Note Edited: 0007961	View Revisions
2014-01-11 20:04	Torr Samaho	Note Added: 0007963
2014-01-11 20:13	Konar6	Note Added: 0007964
2014-01-12 10:38	Torr Samaho	Note Added: 0007974
2014-01-12 18:05	AlexMax	Note Added: 0007981
2014-01-12 20:19	Torr Samaho	Note Added: 0007986
2014-01-12 21:19	AlexMax	Note Added: 0007989
2014-01-12 22:14	Torr Samaho	Note Added: 0007991
2014-01-12 22:15	Torr Samaho	Status	feedback => needs testing
2014-01-14 08:25	Dusk	Note Added: 0007999
2014-01-14 08:25	Dusk	Target Version	=> 2.0
2014-01-15 22:57	AlexMax	Note Added: 0008016
2014-01-16 07:13	Torr Samaho	Note Added: 0008022
2014-01-16 07:13	Torr Samaho	Target Version	2.0 => 1.3
2014-06-29 20:51	Arco	Note Added: 0009796
2014-06-29 20:51	Arco	Status	needs testing => feedback
2015-03-26 03:04	Ru5tK1ng	Note Added: 0011897
2015-07-17 12:58	unknownna	Note Added: 0012958
2015-07-17 12:58	unknownna	Assigned To	Torr Samaho => AlexMax
2015-07-17 12:58	unknownna	Status	feedback => resolved
2015-07-17 12:58	unknownna	Resolution	open => fixed
2015-07-17 12:58	unknownna	Fixed in Version	=> 2.0
2018-09-30 22:54	Blzut3	Status	resolved => closed

Relationships