legacyclonk / LegacyClonk

The LegacyClonk engine and the c4group command line tool.
https://clonkspot.org/lc-en
Other
83 stars 18 forks source link

Possible Bug: Pressing Dig while climbing drops item in addition to letting go, with jump and run control #119

Open fritzw opened 7 months ago

fritzw commented 7 months ago

Description says it all. Should there be a break at the end of this line?https://github.com/legacyclonk/LegacyClonk/blob/a08f92720b055f29e1786b829c04ed4e837e3f4b/src/C4Object.cpp#L3599

I noticed this because my COM_Jump implementation copied that line and also dropped items when letting go. It seems unhelpful to have a key that does "let go" and "drop" at once, because you can collide with the dropped object, or fall into the explosion of the flint you just dropped. If you wanted to do both, just press both keys at once.

Interestingly, the DFA_Scale case in C4Object::DirectCom (line 3421) does not mention COM_Dig at all. It's only in C4Object::AutoStopDirectCom.

maxmitti commented 7 months ago

Yes, it looks like a bug (and it also annoyed me very much when trying JnR). However, there are players who like it as a feature for letting go and dropping the object very soon …

fritzw commented 7 months ago

Well for my gamepad patch it does not matter much, because COM_Jump is a separate button, so you can just press A (Jump) to let go without dropping items or press B (Dig) to let go with dropping items.

For keyboard control I don't really care much. So if the bug has grown into a requested feature, I'd be fine with closing this issue.

However, players who want to do this could still press "Throw then immediately Dig" to achieve the same effect. It's not as if fast button combos are unheard of in Clonk. Just saying. In the end it's your call, I don't care either way.

maxmitti commented 7 months ago

However, players who want to do this could still press "Throw then immediately Dig" to achieve the same effect. It's not as if fast button combos are unheard of in Clonk. Just saying. In the end it's your call, I don't care either way.

If I remember correctly, the horizontal drop position is different when using Dig.

I once had an idea of creating an improved JnR scheme which would be the new default, but opt-in for existing players. The idea included fixing this. I don’t recall the other changes. Perhaps I can still find the discussion somehow.

Danghorx commented 7 months ago

Results of the poll from years ago. Noone liked the dropping of the items while scaling: Screenshot_20240328_121018_Discord Screenshot_20240328_121012_Discord

fritzw commented 7 months ago

I don't quite understand the first point in the survey. Neither the JnR controls nor the classic controls drop items on Dig or Down in the DFA_HANGLE case. Or was that different in the past and got fixed?

Quote @maxmitti I once had an idea of creating an improved JnR scheme which would be the new default, but opt-in for existing players. The idea included fixing this. I don’t recall the other changes. Perhaps I can still find the discussion somehow.

I also considered implementing my Xinput gamepad controls, with an explicit jump button, as player->ControlStyle == 2, but then decided against it, because then I would have to change all those if (player->ControlStyle) throughout the code. In the end I decided against it, because there is only one place where it would have made a difference (in ObjectComUp), and all other places just work the same as before if COM_Jump is never sent.

On the other hand, I might still do it, because it would allow a bit more freedom to change the controls to be more gamepad/analog-stick optimized without impacting keyboard controls. But I don't think it would be worth the effort for just this one break;.

Danghorx commented 7 months ago

You drop items after falling down when you hangled, because it still has the JnR input for Down active for half a second. This sometimes leads to you dropping your weapon (like a spear or gun) or item on yourself that you would have wanted to throw or shoot. With classic this doesn't happen because you need a second down press for dropping an item.

fritzw commented 7 months ago

Ah yes, that makes sense. Luckily I didn't encounter that behavior yet in my tests. But I see how it would be seriously annoying in a fight, or when dropping a flint on a slope. Fixing that probably requires more than a break; though.

I'll definitely fix that in my gamepad patch. If you think it's a good idea, I can create a PR for the main branch with just those changes.

maxmitti commented 7 months ago

Thanks to @Danghorx ’s poll results, I think it is reasonable to fix it unconditionally. If you want, you can create that PR, otherwise I can do it too, since it is a simple change. :)

maxmitti commented 7 months ago

Regarding dropping of weapons only, this would need to be fixed via Clonk and individual object scripts. I wonder if people would have a problem if it’s fixed in general. Especially, because the exactly same thing could still be achieved by holding down [Down] until [Throw] has been pressed.

Danghorx commented 7 months ago

If it is easily implemented, you could implement both JnR "Bug-fixes" and let it be tested by the players. And if it is accepted and doesn't break anything, it can stay this way. Otherwise you can revert the changes. Funni proposed more DefCore settings but as the discussion below shows, Fulgen doesn't want it. It is more a problem of the engine than of the packs. Whole discussion in discord: JnR Bug Controls.txt

Danghorx commented 7 months ago

Is there a reason in the code why pressing dig while scaling or hanging lets the clonk go of the wall or ceiling? This happens only with JnR.

fritzw commented 7 months ago

It is there since the first commit to this repository, so I guess someone must have thought that the dig key is underused when first inventing the JnR mode.

fritzw commented 7 months ago

I've found another weird behavior connected to this: When hangling in a very low corridor and pressing Dig to let go, the clonk would start digging immediately after landing. But if the fall takes longer than C4DoubleClick, then C4Player::ExecuteControl will clear LastCom and the Clonk will not start to dig after landing...

This is clearly a bug, so I fixed it in the same commit.

maxmitti commented 7 months ago

I once had an idea of creating an improved JnR scheme which would be the new default, but opt-in for existing players. The idea included fixing this. I don’t recall the other changes. Perhaps I can still find the discussion somehow.

I couldn’t find the discussion, but Danghor’s and your insights reminded me of a few things. My personal opinions were and still are to change (if there was no consideration about players used to the current JnR controls) behavior during Climbing and Hangling:

However, especially the last point is strongly biased due to my experiences. I started playing Clonk with Clonk Planet, which only had classic controls, so I always have been very used to pressing [Down] for stopping. Whenever I would use JnR controls, I would still press [Down] subconsciously many times in order to stop. So I often dropped stuff unintentionally. But anyway, I think having this kind of “memory” for having pressed [Down] recently for alternative actions is rather unintuitive, especially when holding down keys is already the norm for controls. Together with it being defined as “has been pressed in the recent X frames” it makes a very unpredictable behavior during fluctuating FPS.

I also considered implementing my Xinput gamepad controls, with an explicit jump button, as player->ControlStyle == 2, but then decided against it, because then I would have to change all those if (player->ControlStyle) throughout the code.

For this reason, I was considering to (internally) use a separate flag, or change ControlStyle to a bitfield. This way, everything would default to the already established JnR behavior, except when explicitly checked otherwise.

For the sake of having a distinction between [Up] and [Jump] a similar idea could apply: Set a new flag when a dedicated [Jump] binding shall be used and only add the check at the point where you want the change.

fritzw commented 7 months ago
  • No engine behavior for [Dig] during climbing and hangling. This means, neither let go, nor drop inventory contents.
  • Pressing [Down] to let go doesn’t set the flag which is responsible for dropping things.
  • I would even consider to require holding [Down], [Left] or [Right] to drop things mid-flight.

The second point is implemented in #120, but I'm not so sure about 1 and 3.

Having [Dig] as a "Let go" key is quite useful, because it removes the need to decide whether to press [Left], [Right], or [Down] to let go, depending on your current climbing/hangling mode. And since [Dig] has no other useful meaning during climbing that I can think of (except a hypothetical item that you could trigger with [DoubleDig] while climbing/hangling) I see no harm in keeping it.

The third point makes sense from a design perspective, but that should surely be default only for new players, and opt-in for existing players. Otherwise it would trip up existing JnR players, similar to how you are still used to classic controls. (PS: I started with Clonk 1, but Planet was the start of my programming career :D)

For the sake of having a distinction between [Up] and [Jump] a similar idea could apply: Set a new flag when a dedicated [Jump] binding shall be used and only add the check at the point where you want the change.

Yes, that's what I did in my Xinput gamepad PR: pPlr->HasExplicitJumpKey is set if a Jump button was configured for that player, and is then checked in ObjectComUp to determine whether to jump or not. But it could just as well be a flag in ControlStyle, if we convert that to a bit field, which I think would make sense.

maxmitti commented 7 months ago

Having [Dig] as a "Let go" key is quite useful, because it removes the need to decide whether to press [Left], [Right], or [Down] to let go, depending on your current climbing/hangling mode.

Good point.

And since [Dig] has no other useful meaning during climbing that I can think of (except a hypothetical item that you could trigger with [DoubleDig] while climbing/hangling) I see no harm in keeping it.

Activating items is heavily used in Eke Reloaded, we disabled the engine behavior because of that for Eke Reloaded CE. I think in Hazard they already fixed it in the Clonk, at least for weapons. In general, engine behavior for [Double Dig] doesn’t activate items during climbing or hangling, but the SFT (Clonk) from Eke Reloaded does. On the other hand, activating is possible any time with [Double Special 2], except when that is bound to something else by script (in case of Eke Reloaded), so it is indeed not that essential to disable.

The third point makes sense from a design perspective, but that should surely be default only for new players, and opt-in for existing players. Otherwise it would trip up existing JnR players, similar to how you are still used to classic controls. (PS: I started with Clonk 1, but Planet was the start of my programming career :D)

Yes, keeping it the same for existing players is a precondition for these proposed changes.

PS: My programming career started with Clonk Rage. :D

Danghorx commented 7 months ago

The JnR behaviour with dig and letting go of the wall or ceiling is not worth it. I prefer a direct action like down for dropping myself or left and right to let go of a wall. It could lead to accidents when you dig in sky ilands or you need to dig to move more precise. JnR to Classic is here also different: JnR scales on a right press on a right wall, but classic stops the clonk.

Activating items is heavily used in Eke Reloaded, we disabled the engine behavior because of that for Eke Reloaded CE. I think in Hazard they already fixed it in the Clonk, at least for weapons. In general, engine behavior for [Double Dig] doesn’t activate items during climbing or hangling, but the SFT (Clonk) from Eke Reloaded does. On the other hand, activating is possible any time with [Double Special 2], except when that is bound to something else by script (in case of Eke Reloaded), so it is indeed not that essential to disable.

I always wondered why it is not allowed to use items in the air/scaling/hanging with double dig. Its hidden for newer players. And it is mentioned nowhere that you can do it with special 2 in the air or on walls. Also this special 2 desaster on walls with normal EKE is just uhh...