-
-
Notifications
You must be signed in to change notification settings - Fork 66
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
fix: Don't overwrite current world name when entering housing [skip ci] #2477
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is the only place to have a special case to prevent the contribution message from appearing each time you entered housing but there may be other places I'm unaware of
if (m.find()) { | ||
String worldName = m.group(1); | ||
String worldName = housing ? currentWorldName : m.group(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assumes that even when joining other worlds by joining houses, you get to see the updated world name for a small bit. I only assume this is correct because of the duplicate messages this PR aims to fix, which I assume is triggered by first, the world change, then by the the world changing to "housing".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this doesn't fix the changing worlds housing issue, only it changing the world name to the name of the housing, which then makes features that use current world name not work correctly. Server uptime overlay, bomb bell command/overlay, discord rich presence etc.
So since setState
checks the world names aren't equal before posting the world change event, it won't incorrectly post the event as the newWorldName
is still set to currentWorldName
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, so it's not a full fix, but at least it patches the issue. Sure.
Should fix the minor issue from #2388 but doesn't fix the main issue.