Skip to content

/ AuthMeReloaded

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

Jump to bottom

Fix unregistered players teleport to spawn with unforced registration #1913

Open
wants to merge 1 commit into
base: master
from
Open

Fix unregistered players teleport to spawn with unforced registration #1913

wants to merge 1 commit into from

Conversation

@Kongolan
Copy link

@Kongolan Kongolan commented Sep 17, 2019

Considering that I posted the same bug 4 years back (which was fixed on an old version), you should really consider that there are some servers, which do not force registration for players... since the whole test class is written only with forced registration in mind.

If you set teleportUnAuthToSpawn to true and also forceRegistration to false, unregistered players always end up at spawn, while they should not be altered by AuthMe and login like vanilla. Once you register that player it works perfectly fine.
@Kongolan
Fix unregistered players teleport to spawn with unforced registration
Loading status checks…
c389105
@sgdc3 sgdc3 requested review from sgdc3, Hex3l and ljacqu Sep 18, 2019
@sgdc3
sgdc3 approved these changes Sep 18, 2019
View changes
@ljacqu
Copy link
Member

@ljacqu ljacqu commented Sep 21, 2019

Looks fine to me.
@sgdc3 – I remember in the past you were worried about checking if an auth is available too frequently. Maybe we should think about issuing a warning if registration is optional but the data source is not cached?
@Kongolan – thanks for the contribution. You mention that there are no tests that test for optional registration. Feel free to add some if you like.
@ljacqu
ljacqu approved these changes Sep 21, 2019
View changes
@Kongolan
Copy link
Author

@Kongolan Kongolan commented Sep 22, 2019

So far I only added the tests for optional registration to make it compile. So I did not actually write a solid test code. But it's really something which is missed out all the time.
@Kongolan
Copy link
Author

@Kongolan Kongolan commented Dec 15, 2019 •
edited

Looking into this again since i was going to check for the 1.15 update... What is the codeclimate issue which needs to be fixed? Is that the reason why the pull request was not applied yet?
@sgdc3
Copy link
Member

@sgdc3 sgdc3 commented Dec 16, 2019

Hmm dataSource.isAuthAvailable is a blocking call, are we shure that the teleport method is never called from the main thread?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ljacqu ljacqu

@sgdc3 sgdc3

@Hex3l Hex3l

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@Kongolan @ljacqu @sgdc3
You can’t perform that action at this time.