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.

Sign up for GitHub

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

Jump to bottom

Add BadPackets1 #1485

Open
wants to merge 3 commits into
base: 2.0
Choose a base branch
from
Open

Add BadPackets1 #1485

wants to merge 3 commits into from

Conversation

ManInMyVan
Copy link
Contributor

check for moving too little

@SamB440
Copy link
Collaborator

SamB440 commented May 25, 2024 •
edited
Loading

Duplicate packets can be sent within 0.03 from what I remember, that's why it checks within 0.03 for duplicate packet, I'd double check

Oh, but you're checking !player.skippedTickInActualMovement, that seems weird though as doesn't it depend on the previous movement?

@ghost
Copy link

ghost commented May 25, 2024

BadPacketsAA looks cleaner then BadPackets1

@ManInMyVan
Copy link
Contributor Author

Oh, but you're checking !player.skippedTickInActualMovement, that seems weird though as doesn't it depend on the previous movement?

I'm checking that because players who are standing still don't send flying packets on 1.9+.

@ManInMyVan
Copy link
Contributor Author

BadPacketsAA looks cleaner then BadPackets1

AA looks like it could be a subcheck of A

@ManInMyVan
Copy link
Contributor Author

Duplicate packets can be sent within 0.03 from what I remember, that's why it checks within 0.03 for duplicate packet, I'd double check

We don't update the player position if it's a duplicate

@Symmettry
Copy link

BadPacketsAA looks cleaner then BadPackets1

AA looks like it could be a subcheck of A

No I think "A1" would be a subcheck of A.
"AA" doesn't sound like that, so I think AA sounds better.

BadPackets1 just looks weird.

@ManInMyVan
Copy link
Contributor Author

"AA" doesn't sound like that, so I think AA sounds better.

Some people might think it does

@Symmettry
Copy link

"AA" doesn't sound like that, so I think AA sounds better.

Some people might think it does

Okay? What's the problem. Even if read as "subclass" it's still its own check, so this argument is flawed.

@MachineBreaker
Copy link
Collaborator

Move the discussion of the check name to the Discord server

@Symmettry
Copy link

Symmettry commented May 26, 2024 •
edited
Loading

Move the discussion of the check name to the Discord server

I'm banned I think lol

Symmettry
Symmettry reviewed May 26, 2024
View reviewed changes
src/main/java/ac/grim/grimac/checks/impl/badpackets/BadPackets1.java Outdated
if (event.getPacketType() == PacketType.Play.Client.PLAYER_POSITION || event.getPacketType() == PacketType.Play.Client.PLAYER_POSITION_AND_ROTATION) {
final double delta = Math.sqrt((player.x - player.lastX) * (player.x - player.lastX) + (player.y - player.lastY) * (player.y - player.lastY) + (player.z - player.lastZ) * (player.z - player.lastZ));
// This seems to be valid for 1.17 duplicates, don't exempt
if (!player.packetStateData.lastPacketWasTeleport && !lastLastPacketWasTeleport && player.checkManager.getPacketCheck(BadPacketsE.class).noReminderTicks < 20 && !player.skippedTickInActualMovement) {
Copy link

Choose a reason for hiding this comment

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

wouldn't getting BadPacketsE every single time cause a bit of lag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have any other solution that doesn't involve storing it twice?

@ManInMyVan
update
ea88d4f 
- use packet pos
- exempt 1.17 duplicates
- cancel the packet
- exempt on teleport less
@overkidding
Copy link
Contributor

Still falses with teleports, boat leaving, etc

@Symmettry
Copy link

I still think you should rename to BadPacketsAA
"BadPackets1" just looks odd

@Anthony01M
Copy link
Contributor

False Positive; Note: BadPacketsH for me is this
Server Information: 1.8.8 git-PandaSpigot-122
Client Information: Lunar 1.8.8
How To Reproduce:

  • Method 1:

    • Tab out with high ping, notice that it will start spamming (I was in console trying to get the second SS so maybe also try uploading too)
      image

  • Method 2

    • 500+ms (HIGH PING) then try to move, notice how you will get setback a few times (didn't alert at first)
    • after a few setbacks when moving / jumping / etc. you will get an alert

image

@ManInMyVan
Copy link
Contributor Author

Still falses with teleports, boat leaving, etc

Should be fixed now, lmk if you can still false

@ManInMyVan
Copy link
Contributor Author

False Positive; Note: BadPacketsH for me is this
Server Information: 1.8.8 git-PandaSpigot-122
Client Information: Lunar 1.8.8
How To Reproduce:

  • Method 1:

    • Tab out with high ping, notice that it will start spamming (I was in console trying to get the second SS so maybe also try uploading too)
      image

  • Method 2

    • 500+ms (HIGH PING) then try to move, notice how you will get setback a few times (didn't alert at first)
    • after a few setbacks when moving / jumping / etc. you will get an alert

image

Can you still replicate?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Symmettry Symmettry Symmettry left review comments

@jonesdevelopment jonesdevelopment jonesdevelopment left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ManInMyVan @SamB440 @Symmettry @MachineBreaker @overkidding @Anthony01M @jonesdevelopment