Skip to content

Conversation

Nxllpointer
Copy link
Contributor

Fixes top helper anticheat not counting visible charachters instead of invisible ones. This was caused by me using a capital P for matching invisible charachters which selects all characters not being invisible. Everything now works as intended.

This also includes Zabuzard's suggestion of using a Pattern directly to replace the invisible chars which avoids String#replaceAll compiling a new pattern every time.

Sped up uncounted char matching by compiling the pattern once and using the pattern to replace chars instead of passing the RegEx string to String#replaceAll which would compile the pattern every time
@Nxllpointer Nxllpointer requested review from a team as code owners October 24, 2022 21:16
@Nxllpointer Nxllpointer added bug Something isn't working priority: normal labels Oct 24, 2022
@Zabuzard
Copy link
Member

You know, thats why one should write unit tests 🙂

Copy link
Member

@Zabuzard Zabuzard left a comment

Choose a reason for hiding this comment

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

tested locally. works better lol

@Zabuzard
Copy link
Member

considering this critical, since we must not release until this is merged. otherwise the top helper system is dead until the next release

@Nxllpointer
Copy link
Contributor Author

You know, thats why one should write unit tests slightly_smiling_face

Should I add one? It would make sense considering how important this working is.

@Zabuzard
Copy link
Member

if you feel confident enough to add one, that would be cool. might require reworking a bit of the code/methods though.

but please do it in another PR. this PR must not be delayed, it has to be deployed asap.

@marko-radosavljevic marko-radosavljevic merged commit bb788aa into Together-Java:develop Oct 25, 2022
@Zabuzard Zabuzard mentioned this pull request Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority: critical

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants