Skip to content

Modify offline mode warning to include Velocity line#8812

Merged
Owen1212055 merged 3 commits into
PaperMC:masterfrom
mja00:feat/velocity-in-offline-mode-msg
Feb 10, 2023
Merged

Modify offline mode warning to include Velocity line#8812
Owen1212055 merged 3 commits into
PaperMC:masterfrom
mja00:feat/velocity-in-offline-mode-msg

Conversation

@mja00
Copy link
Copy Markdown
Contributor

@mja00 mja00 commented Jan 24, 2023

This adds a check to the offline-mode warning to see if Velocity is enabled. If it is enabled then it warns the user to make sure to secure their server and links to the Velocity documentation on that.

Attempt 2 since I had no idea about the branch name requirement.

@mja00 mja00 requested a review from a team as a code owner January 24, 2023 22:48
Copy link
Copy Markdown
Member

@NoahvdAa NoahvdAa left a comment

Choose a reason for hiding this comment

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

This should go into the Velocity support patch

@mja00
Copy link
Copy Markdown
Contributor Author

mja00 commented Jan 24, 2023

Time to figure out how to modify patches now. 👍🏻

@mja00 mja00 force-pushed the feat/velocity-in-offline-mode-msg branch from 7298928 to 7ddca71 Compare January 24, 2023 23:13
@mja00
Copy link
Copy Markdown
Contributor Author

mja00 commented Jan 24, 2023

I think I did it correctly lol

Copy link
Copy Markdown
Member

@MiniDigger MiniDigger left a comment

Choose a reason for hiding this comment

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

that dos indeed look correct.
the branch requirement is so that the team can properly modify your branch, github prohibits that on master.

one thought I had is extracting the two variables (link and software) out so that the text isn't duplicated, but prolly not worth it so approving as it

Copy link
Copy Markdown
Member

@NoahvdAa NoahvdAa left a comment

Choose a reason for hiding this comment

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

lgtm

@Astralchroma
Copy link
Copy Markdown
Contributor

I think if velocity forwarding + velocity online mode is enabled the warning should be hidden entirely

@electronicboy
Copy link
Copy Markdown
Member

This message feels weird given that velocity forwarding is much more secure than bungees; I also wouldn't mind if velocity online mode was an info message instead of a warn

@Astralchroma
Copy link
Copy Markdown
Contributor

tbh the existing message for bungeecord is kinda bad too, might be a good idea to clean that up a bit as well
i dont know if anyone wants suggestions for the wording or not though

@mja00
Copy link
Copy Markdown
Contributor Author

mja00 commented Jan 25, 2023

If someone else figures out the wording I'd gladly swap it to that (I am not a wordsmith). Otherwise I can change up the if statement so we're not duplicating that one line and swap it to INFO if wanted.

@mja00
Copy link
Copy Markdown
Contributor Author

mja00 commented Jan 25, 2023

This latest change should be what Mini was suggesting I believe. Definitely makes it cleaner. I'm not too sure how to go about swapping to using INFO if online-mode for velocity is enabled. It seems like we'd be duplicating a lot of lines for that. Unless we just change that message entirely.

@Owen1212055 Owen1212055 force-pushed the feat/velocity-in-offline-mode-msg branch from b312862 to a17eb57 Compare February 10, 2023 22:56
@Owen1212055 Owen1212055 force-pushed the feat/velocity-in-offline-mode-msg branch from 2007721 to aec9aa4 Compare February 10, 2023 23:04
@Owen1212055 Owen1212055 merged commit afa16e6 into PaperMC:master Feb 10, 2023
maestro-denery pushed a commit to maestro-denery/Paper that referenced this pull request Feb 19, 2023
This adds a check to the offline-mode warning to see if Velocity is enabled. If it is enabled then it warns the user to make sure to secure their server and links to the Velocity documentation on that.
LeonTG pushed a commit to LeonTG/Paper that referenced this pull request May 17, 2026
This adds a check to the offline-mode warning to see if Velocity is enabled. If it is enabled then it warns the user to make sure to secure their server and links to the Velocity documentation on that.
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