Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjusting how deadzones are calculated #3079

Merged
merged 6 commits into from
Feb 16, 2022
Merged

Conversation

skrekhere
Copy link
Contributor

@skrekhere skrekhere commented Feb 3, 2022

Previously, the way that deadzones were calculated was that if the x axis is smaller than the set value of the deadzone, then X would be 0. same goes for Y. This works for a select few games, but say for example, in some game you're moving forward with the stick pressed all the way forward. if you wanted to turn say 5 degrees to the right, then it wouldn't be possible, as the deadzone would entirely prevent small adjustments on the edge of another axis. With this new implementation, it creates what is essentially a dead circle in the centre, and then smooths the edges from there. This both allows for small adjustments at the extreme ends of a specific axis, but also prevents the problem of a sudden burst of movement once you're past the deadzone, because of there being no "sharp" edges.

Also added .idea/* to the gitignore because I'm sure more than just me uses rider for development, and it makes life easier if it's just there.

Should fix #3074

@MutantAura
Copy link
Collaborator

I'd suggest a more descriptive PR title and description here.
Something like "Adjustments to deadzone calculation and clamping" and a brief explanation of what you're changing and how it is different/better than the old method. Nothing crazy long but it helps the reviewers and also anyone reading :)

@skrekhere skrekhere changed the title Making deadzones feel nice and smooth Adjusting how deadzones are calculated Feb 4, 2022
@Vegita2
Copy link

Vegita2 commented Feb 4, 2022

I made a deadzone visualizer some time ago and added the current implementation and the one from this pr to it.
https://vigilant-beaver-7e2f51.netlify.app/
(there is a select input at the bottom left)

color ranges from black to white depending on stick input value
black = no input
white = fully pressed
red circle = deadzone

Basically the difference is this:

Current implementation

Pull request implementation

@MutantAura MutantAura added fix Fix something input Related to Ryujinx.Input or Ryujinx.Input.SDL2 labels Feb 5, 2022
@MutantAura
Copy link
Collaborator

MutantAura commented Feb 6, 2022

Some quick tests performed at deadzone of 0.5 in BoTW

Master:

Master-1.mp4

PR:

Pr-1.mp4

There is much more fine control in the new calculation and removes the almost 8-axis effect a high deadzone would previously apply in the past (See master video). No other abnormalities noticed. Looking good from my perspective.

Ryujinx.Input/HLE/NpadController.cs Outdated Show resolved Hide resolved
Ryujinx.Input/HLE/NpadController.cs Outdated Show resolved Hide resolved
Ryujinx.Input/HLE/NpadController.cs Outdated Show resolved Hide resolved
Ryujinx.Input/HLE/NpadController.cs Show resolved Hide resolved
Copy link
Member

@AcK77 AcK77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm!

{
Dx = ClampAxis(MathF.Abs(x) > deadzone ? x : 0.0f),
Dy = ClampAxis(MathF.Abs(y) > deadzone ? y : 0.0f)
Dx = ClampAxis((x / magnitudeClamped) * ((magnitudeClamped - deadzone) / (1 - deadzone))),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really a fan of the potential divide by zero. ClampAxis seems like it is working by a fluke, as float.NaN casts to 0 as short. If the behaviour of that function changes, this might break things in an unexpected way.

Copy link
Contributor Author

@skrekhere skrekhere Feb 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no more potential of a divide by zero. that changed when I changed the < to a <= in the first check, before the ClampAxis call entirely. The only potential of a divide by zero before the change was when the magnitude is zero, with the set deadzone also being zero, but now a divide by zero is only possible with negative deadzone values, which is a whole other problem if that's allowed to happen.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If deadzone is 1 you also have a divide by zero

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nevermind we can't never reach this situation since magnitude is already clamped to 1. Another thing to take in mind is that you don't need to clamp negative values. Since (xx + yy) is guaranteed to be positive

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Speaking about that, ClampAxis appears to be slightly wrong. It would be nice to take a look too, since you're working on this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I didn't see the condition above that returns early. It should also trigger if deadzone is >= 1, so no concerns.

And yeah, ClampAxis is incorrect right now, it doesn't clamp positive and the negative clamp is wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest commit should fix everything up :)

else
{
return (short)(value * short.MaxValue);
return (short) Math.Max(value * -short.MinValue, short.MinValue);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return (short) Math.Max(value * -short.MinValue, short.MinValue);
return (short)Math.Max(value * -short.MinValue, short.MinValue);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kinda wonder if -1.0 is supposed to map to -32768. That would make the range uneven sinnice short.MaxValue is 32767.

}

return (short) Math.Min(value * short.MaxValue, short.MaxValue);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return (short) Math.Min(value * short.MaxValue, short.MaxValue);
return (short)Math.Min(value * short.MaxValue, short.MaxValue);

@skrekhere
Copy link
Contributor Author

fixed up that spacing

Copy link
Member

@gdkchan gdkchan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks.

@AcK77 AcK77 merged commit 8cc2479 into Ryujinx:master Feb 16, 2022
Copy link
Member

@riperiperi riperiperi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, good to see this weird behaviour sorted.

else
{
return (short)(value * short.MaxValue);
return (short)Math.Max(value * -short.MinValue, short.MinValue);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit weird, but it seems to work as intended?
-short.MinValue doesn't fit in a short, it will just equal -32768 again. But here, it seems to negate as int32, then multiply the float, so it's 32768f.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Fix something input Related to Ryujinx.Input or Ryujinx.Input.SDL2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

controller deadzone feels wrong
7 participants