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

Add 45 snap client setting #9465

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add 45 snap client setting #9465

wants to merge 1 commit into from

Conversation

l-ouis
Copy link
Contributor

@l-ouis l-ouis commented Jan 3, 2025

😬

removes need for janky 45 deg mouse distance bind, allows this to be bound easily in settings

makes snapping to 45 degrees very easy and more convenient, can make certain parts (lasers, shotguns, gores saves) easier than they have been before

needs discussion

Screencast.From.2025-01-03.14-57-28.trimmed.mp4

Checklist

  • Tested the change ingame
  • Provided screenshots if it is a visual change
  • Tested in combination with possibly related configuration options
  • Written a unit test (especially base/) or added coverage to integration test
  • Considered possible null pointers and out of bounds array indexing
  • Changed no physics that affect existing maps
  • Tested the change with ASan+UBSan or valgrind's memcheck (optional)

@@ -232,9 +239,9 @@ void CPlayers::RenderHookCollLine(

if(Local && !m_pClient->m_Snap.m_SpecInfo.m_Active && Client()->State() != IClient::STATE_DEMOPLAYBACK)
{
ExDirection = normalize(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if this is important so I just commented it out for now if someone can review

@Kaffeine
Copy link
Contributor

Kaffeine commented Jan 3, 2025

New cheats in DDNet 🗿

@heinrich5991
Copy link
Member

First, I think this should be discussed as a gameplay feature, if we want this at all or not.

If we want it, we'll need to see whether we should exclude non-DDNet gametypes from it. We should probably also add proper graphics for this "aim assist", so that it's clear that your hook can only go into 8 directions, e.g. by having some sort of overlay.

@heinrich5991
Copy link
Member

Possible reasons for wanting it:

  • There's currently a janky way to do this.
  • It makes the game easier.

Possible reasons for not wanting it:

  • Just because there's currently a janky way to do it, doesn't mean we should encourage it. We might as well just patch it out entirely.

We should probably set a line what kind of "aim assist" features we want to allow in DDNet. Should we stop at 45°? What about other angles? Aim assist for edge hooks? Should we remove the dummy self-laser "feature"?

We could remove such features by removing them the next time we add a quality of life feature so that people will have to decide whether they want the new QoL or the old "features".

@l-ouis
Copy link
Contributor Author

l-ouis commented Jan 6, 2025

We could remove such features by removing them the next time we add a quality of life feature so that people will have to decide whether they want the new QoL or the old "features".

Not sure exactly what that means @heinrich5991

But I agree with the discussion part. I think even the janky way of aim snapping is too powerful, and being able to naturally aim at these angles is a skill that most players can build up without a bind.

One alternative I had in mind is to have the bind act as a "focus" mechanic: instead of x immediately toggling the snap, you have to hold x for 0.5 seconds before you get the aim snap. (this can be complimented with a little animation that shows blurred cardinal direction axes overlayed onto your tee that become unblurred after 0.5 secs)

This still acts as QoL for players who want to do double nades without trouble but also raises the skill ceiling for speedrunners as they won't be able to instantly pop vertical nades- instead, they'll have to either properly aim them or tank a 0.5 second delay (it will also make some shotgun and laser parts less cheesable by the bind)

This alternative raises the skill ceiling and will probably feel more intentional than the current bind but might be unpopular w people who heavily rely on the current bind

@sjrc6
Copy link
Contributor

sjrc6 commented Jan 8, 2025

Just because there's currently a janky way to do it, doesn't mean we should encourage it. We might as well just patch it out entirely.

aren't there maps that expect you to use the bind?

@heinrich5991
Copy link
Member

I don't know. Which would that be?

@l-ouis
Copy link
Contributor Author

l-ouis commented Jan 8, 2025

There are some corner hooks on maps (like the second-to-last part in Flow Fever) where the bind can help, but in maps like these you can also just manually aim to get the 45 degree angle, you aren't required to do them under a time constraint

@KebsCS
Copy link
Contributor

KebsCS commented Jan 8, 2025

Just because there's currently a janky way to do it, doesn't mean we should encourage it. We might as well just patch it out entirely.

Doesn't make sense. What's going to stop anyone from just using an older version of the client?

@def-
Copy link
Member

def- commented Jan 8, 2025

I agree, we should keep everything that is currently supported via binds supported, and not add any further ways to make the game easier. Even vanilla Teeworlds allowed these binds.

@Learath2
Copy link
Member

Learath2 commented Jan 9, 2025

I think removing the janky way is just a no-go without introducing a one to one replacement. It would be so unfair to everyone making new runs and no one would upgrade their client. So no nerfing this QoL to make it less "overpowered" IMO. At least not without clearing the entire ladder.

Given that, I think it's a nice idea to add a "shortcut" like this so people don't have to look up a weird janky bind. It does probably need some sort of graphical indication though.

Other discussion about "aim-assist" like features in DDNet, while useful, are probably not exactly relevant to this specific case because this case just recreates something that is already possible with better UX.

@l-ouis
Copy link
Contributor Author

l-ouis commented Jan 9, 2025

What would be a good visual indicator? Overlaying a .png would be pretty but also overkill, should we just draw 8 lines coming out of the tee of some sort?

@heinrich5991
Copy link
Member

We could remove such features by removing them the next time we add a quality of life feature so that people will have to decide whether they want the new QoL or the old "features".

To everyone saying removing stuff is not possible, I'd like to ask you to at least acknowledge that you have read the above part of my comment. This includes @KebsCS, @def- and @Learath2.

I'm proposing that we can remove janky (potentially unwanted) quality of life features the next time we introduce a new quality of life feature. People can continue using an old client (or we can potentially even include both in the new client, with a config that can be set once per connection to choose whether you want to have the old or the new QoL). This way, we don't have to block new QoL features forever, but can instead phase out annoying old ones and give people new ones.

I think the current state where we can never introduce a new dummy feature again, for example, is bad. In my proposal, we could, for example, remove the dummy self-aim "feature" and allow people more control over the dummy, but not giving people the option to do both at once.

@Learath2
Copy link
Member

Learath2 commented Jan 9, 2025

I'm not saying removing something is not possible. I'm saying offering a nerfed version of this feature that includes any penalty is a bad idea because it would pretty much push every serious player to an old version which is much more undesirable.

IMO there is nothing blocking this feature as-is. So I think there might be some sort of miscommunication here. Maybe as to the scope of the discussion as I was trying to limit it rather than expand it to a broader discussion of any and all "aim-assist"-like features.

Rest of this comment is, in my opinion, unrelated to this PR:

Now if we do look at it in a broader sense, I don't think our stance is "no new dummy features". We haven't been allowing new dummy features because we didn't (or at least I didn't) think dummy is in the spirit of the game and the stronger it is the more this game becomes a solo affair. It's not "no new dummy features" it's "don't make dummy stronger".

It might make sense to try "balance" out a QoL change by taking away something else. But there is a more practical moderation issue with your proposal. We technically won't ship it, and won't allow it but people will definitely ship clients that have all dummy features enabled. This is pretty much impossible for mods to keep track of or enforce. So it will lead to all serious dummy players to play on "illegal" clients with no way to prevent them from doing so.

@def-
Copy link
Member

def- commented Jan 9, 2025

Agreed, makes it waaay too easy for a modification to combine all old and new functionality.

The setting in question here doesn't allow anything new, so no problem.

@heinrich5991
Copy link
Member

The setting in question here doesn't allow anything new, so no problem.

I don't know where you get this from. How do you guarantee this? How do you roll back the changes once it becomes clear that it does allow something new?

I can already, from just looking at the description, think of something new it enables. How do you deal with the unknown unknowns?

@Learath2
Copy link
Member

Learath2 commented Jan 9, 2025

I don't know where you get this from. How do you guarantee this? How do you roll back the changes once it becomes clear that it does allow something new?

We get it from the fact that it's supposed to implement whatever the old bind implemented. We guarantee it by ensuring that it mimics the old bind thus not introducing anything new. We don't have to roll it back because it won't allow something new

I can already, from just looking at the description, think of something new it enables.

I think you should just elaborate on what this is. There is no need to be secretive.

How do you deal with the unknown unknowns?

What do you propose is the "solution" here? You can't deal with unknown unknowns a priori, you can only make them easier to handle. That usually involves building flexibility into things, but I genuinely do not see what a feature like this can be flexible in. If it's anymore flexible (e.g. in angle) it'll only introduce more known unknowns (e.g. finer grain snap aiming was not possible before, thus there is a risk that now some parts are much easier). Also why should it be more flexible in angle?


It seems you stumbled upon something all of us are completely missing here, everyone else seems to be under the impression that this feature implements the bind bind x "+toggle cl_mouse_max_distance 2 400; +toggle inp_mousesens 1 200; +showhookcoll" as a native feature. So I'm guessing there is something about this feature being easier to use that creates "something new". Perhaps something to do with the fact that it's one setting instead of two?

@heinrich5991
Copy link
Member

I can already, from just looking at the description, think of something new it enables.

I think you should just elaborate on what this is. There is no need to be secretive.

It allows you to go back and forth between a carefully selected location and a 45° rounding that is close to it.

I didn't go into this because I thought it's instructive to see that even the most trivial things can have unintended further "features".

@Learath2
Copy link
Member

Learath2 commented Jan 9, 2025

So it doesn't behave as the old bind. This is something that was already in my mental model for things that can go wrong. There was nothing in the description to suggest that it didn't, so I just didn't notice that was the case. It can be fixed to just not revert to the old location.

Theoretically, I agree that we can just remove this. Practically, it will be extremely hard on the moderation, it will be extremely annoying to the people who actually play the game and use the bind to line up shots. Moreover, I don't think I've ever heard a single person tell me that they want angle snaps removed. So I don't quite see the point in discussing the removal of this.

Furthermore, I don't really see how your suggestion accounts for the unknown unknowns either. You can't know how much of a nerf on the other side of the scale is needed to account for a truly unforeseeable interaction.

@heinrich5991
Copy link
Member

Furthermore, I don't really see how your suggestion accounts for the unknown unknowns either. You can't know how much of a nerf on the other side of the scale is needed to account for a truly unforeseeable interaction.

Fair, unknown unknowns are hard to account for. I agree that "the new thing doesn't do exactly the thing that the old thing does" is pretty obvious, although it might need to be figure out how that is the case.

Moreover, I don't think I've ever heard a single person tell me that they want angle snaps removed.

I'd think you'd find an overwhelming support (in number of players, not in loudest voices) for giving aim support for edge hooks as well — it's just that it's not the status quo and some people will find that it makes the game "too easy". In that sense, making something like "45° snap" a feature is also a game design question. Yes, it's currently possible, but also, this makes it an official feature, not a consequence of unrelated features.

@sjrc6
Copy link
Contributor

sjrc6 commented Jan 9, 2025

I think the current state where we can never introduce a new dummy feature again, for example, is bad. In my proposal, we could, for example, remove the dummy self-aim "feature" and allow people more control over the dummy, but not giving people the option to do both at once.

People (not me) will make custom clients that combine all historical capabilities into one client, the downstream forks of tclient are already stretching the rules with stuff like hitboxs and auto kill, so I have absolutely no doubt this would happen.

@l-ouis l-ouis closed this Jan 10, 2025
@l-ouis l-ouis reopened this Jan 10, 2025
@l-ouis l-ouis force-pushed the 45snap branch 2 times, most recently from 3e645e1 to 5362092 Compare January 10, 2025 04:25
@l-ouis
Copy link
Contributor Author

l-ouis commented Jan 10, 2025

Screencast.From.2025-01-10.13-50-52.trimmed.mp4

Cursor now snaps to the angle sent when an input (fire, hook) is done, this way the behavior is more similar to the previous bind

players.cpp now renders m_aRenderedLocalTeeAngle from controls.h. This might be a bad change, not sure how else to implement it. This vector is either set to mousepos/snapped mousepos depending on whether the bind is toggled or not. It has the effect of making the player tee's weapon/hook snapped to 45 degree angles without the mouse cursor necessarily being snapped.

It sort of serves as a visual indicator, and I think it might be a better indicator than an overlay -- some people are bound to find an overlay ugly and wish to disable it, so the overlay either has to be perfect or it needs another config setting. If we just change the rendered local tee angle, players can immediately see that their tee is snapping to angles and it won't be as intrusive as an overlay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants