Skip to content
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 incorrect environment variable handling #222

Merged
merged 2 commits into from
Jan 30, 2024

Conversation

beomseok-park
Copy link
Contributor

Context

  • Fix incorrect environment variable handling

Choices

  • In PalWorldSettings.ini, ServerName, ServerDescription, AdminPassword, ServerPassword, PublicIP, Region, and BanListURL should be enclosed in double quotes.
  • Fix the incorrect handling of double quotes in start.sh.

Test instructions

  1. Stop the Palworld server.
  2. Build the container.
  3. Delete <mount_folder>/Pal/Saved/Config/LinuxServer/PalWorldSettings.ini.
  4. Set a SERVER_NAME with spaces in the .env. e.g. SERVER_NAME="My Palworld Server"
  5. Start the Palworld server.
  6. Check the server name in the game.

Checklist before requesting a review

  • I have performed a self-review of my code
  • I've added documentation about this change to the README.
  • I've not introduced breaking changes.

Copy link
Owner

@thijsvanloef thijsvanloef left a comment

Choose a reason for hiding this comment

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

If the strings are already quoted, like in the examples:

         - ADMIN_PASSWORD="adminPasswordHere"
         - SERVER_NAME="World of Pals"
         - SERVER_DESCRIPTION=""

Then the whole PalWorldSettings.ini gets really wierd with quotes:

ServerName=""World of Pals"",ServerDescription="""",AdminPassword=""adminPasswordHere"",ServerPassword=""worldofpals"",PublicPort=8211,PublicIP="",RCONEnabled=true,RCONPort=25575,Region=""

A check needs to be implemented to see if the vars already have quotes, and if they do not already have quotes then they should be added.

@llnut
Copy link

llnut commented Jan 30, 2024

Change the way environment variables to use the following syntax can fix it:

environment:
  ADMIN_PASSWORD: "adminPasswordHere"
  SERVER_NAME: "World of Pals"
  SERVER_DESCRIPTION: ""

Although this may not be good enough, alternatively, can use sed for additional processing like this.

Copy link
Contributor Author

@beomseok-park beomseok-park left a comment

Choose a reason for hiding this comment

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

I have pushed a new commit that resolves the issue.

@thijsvanloef
Copy link
Owner

thijsvanloef commented Jan 30, 2024

Will take a look tonight, unable to do so now

@Dashboy1998
Copy link
Contributor

If the strings are already quoted, like in the examples:

         - ADMIN_PASSWORD="adminPasswordHere"
         - SERVER_NAME="World of Pals"
         - SERVER_DESCRIPTION=""

Then the whole PalWorldSettings.ini gets really wierd with quotes:

ServerName=""World of Pals"",ServerDescription="""",AdminPassword=""adminPasswordHere"",ServerPassword=""worldofpals"",PublicPort=8211,PublicIP="",RCONEnabled=true,RCONPort=25575,Region=""

A check needs to be implemented to see if the vars already have quotes, and if they do not already have quotes then they should be added.

Docker Documentation doesn't support var="value" but rather:

  • var: "value"
  • var: value
  • var=value
    It is worth noting that you can do "var=value" but it's not mentioned

Copy link
Owner

@thijsvanloef thijsvanloef left a comment

Choose a reason for hiding this comment

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

thank you for your work. I've only updated the examples.

@thijsvanloef thijsvanloef merged commit 8c68de7 into thijsvanloef:main Jan 30, 2024
4 checks passed
@JustusPan
Copy link

does this mean we should change our docker compose file manually to remove all double quote of values when update to 0.19.1?

@thijsvanloef
Copy link
Owner

@JustusPan Yes, but a check has been put in place that it should work either way

MusclePr pushed a commit to MusclePr/palworld-server-docker that referenced this pull request Jun 19, 2024
…ndling

Fix incorrect environment variable handling
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.

5 participants