Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Clarify environment/config variables #39

Merged
merged 2 commits into from
Oct 17, 2022
Merged

Clarify environment/config variables #39

merged 2 commits into from
Oct 17, 2022

Conversation

Rene-Sackers
Copy link
Contributor

I found the readme a little confusing regarding the environment variables. Especially the one labeled PUBLIC_SERVER.

There's a .ini variable called Public=true, but this environment variable modifies Open=true. I figured I'm not going to change the working of this, as it'll break any existing servers using it if they update.

I've added some extra description to the readme explaining that some .ini variables are overwritten at the start, and which environment variables affect which .ini variables.

Also the following was explained inversely:

PUBLIC_SERVER | If set to true only Pre-Approved/Allowed players can join the server

Setting PUBLIC_SERVER to true sets Open=true in ini, an in the ini it explains:

Clients may join without already having an account in the whitelist. If set to false, administrators must manually create username/password combos.

Related issue: #23

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@Rene-Sackers
Copy link
Contributor Author

@Renegade-Master can I get a review on this so we can maybe merge it? :)

@Renegade-Master
Copy link
Owner

Hey!

Thanks for the contribution! The only reason I have not really merged this yet, is that I am not sure I completely agree with some of the wording.

Also, I intend to rewrite the whole process I have here of configuring the server because it tends to overwrite what people do unexpectedly.


But, it could take me a while to get around to that, and your wording is good enough for now.

@Renegade-Master Renegade-Master self-requested a review October 17, 2022 11:06
@Renegade-Master Renegade-Master merged commit 02783a5 into Renegade-Master:main Oct 17, 2022
@Rene-Sackers
Copy link
Contributor Author

Sorry for the late reply, my notification settings were messed up apparently 😟

Yeah I saw you were working on a different way of doing it, I know my changes aren't perfect, but hoped it'd help people in the meantime to clarify some things and prevent the pitfalls I encountered, until then.

Thx for the merge!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants