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

Refactor and improvements #62

Merged
merged 29 commits into from
Nov 1, 2024
Merged

Refactor and improvements #62

merged 29 commits into from
Nov 1, 2024

Conversation

Diyagi
Copy link
Contributor

@Diyagi Diyagi commented Oct 10, 2024

  • Added custom PUID and PGID
    • These ENVs default to 1000 and can be changed by setting both to other IDs
    • The container will always enforce these IDs upon initialization via chown
    • Should avoid issues when mounting save folders inside the container (figured the hard way that the server does not exit when it cannot write to the save file :c )
  • Fix X11 Sockets folder permissions
    • X11 Sockets folder should always be owned by root, and have its permission set to 777 (rwxrwxrwx)
  • Dockerfile Refactor
    • Moved COPY command to be executed after apt-get RUN command so docker can use build cache for test builds
    • Removed some dependencies that were being added via apt-get that seemed unnecessary (More about this at the end)
    • Added tzdata, this adds the possibility to change the container timezone by using the ENV TZ= ( Will come in handy with Updated launch.sh with Discord integration #60 )
  • entry.sh Rewritten
    • This file was repurposed to handle the root part of the initialization like usermod groupmod chown
    • This will also check if the user is trying to run the image as root, in this case the container will exit.
    • Also checks for write permission to STEAMAPPDIR and STEAMAPPDATADIR
    • After everything that needs root is done it will proceed to execute the other scripts as steam user
  • setup.sh Is basically what the old entry.sh did but refactored, mostly to make it cleaner.
  • launch.sh Refactor
    • Removed a lot of code that seemed unnecessary (More about this at the end)
    • Moved Core Keeper Server arguments compilation/build to its own file
    • Added tail for the log file
    • Removed the DISCORD ENV, now it only checks if DISCORD_HOOK ENV is not empty to send the webhook push
    • Silenced curl output, it will still output in case of an error, but for success it will not output anything to stdout
  • compile-parameters.sh script now handles the argument compilation/build and should be ran with source
    • Logs files are now saved to the logs folder
    • Each server start should now generate its own file
    • Parameters are available in the array variable params
    • The path to the current log file is also available in the variable logfile
    • Modifying arguments now should be a lot easier and straight forward
  • Discord integration by @ArrowPrint

Now, i know some code that i removed from launch.sh and some dependencies i removed from Dockerfile were there to fix an issue with green obsidian wall around spawn, but after removing those i generated and tested 10+ different maps and never found any issue with those.
In my tests i lowered the outer wall and walked until i reached the end of the map and didn't find a single issue, as for hardware, i was using an GCP VM without an GPU.
It is possible that this issue got fixed in some patch, and if its the case simply merging this PR would not be a good idea.
Please let me know what you think.

Fixes #42, Fixes #57, Fixes #53

@Diyagi Diyagi changed the title Fix permission for x11-unix file Fix permission for x11-unix folder Oct 10, 2024
@Diyagi Diyagi marked this pull request as draft October 11, 2024 04:45
@Diyagi Diyagi changed the title Fix permission for x11-unix folder Refactor and improvements Oct 11, 2024
@Diyagi
Copy link
Contributor Author

Diyagi commented Oct 18, 2024

For now i think im happy with this stage, ive been using this branch for my own server that i play with friends and i did not found many issues, the ones i did were fixed.

Im planning on adding mod support (automatic download and update) in the future, but that would be better in its own PR. Keeping this as an draft until #60 is merged.

Reorder Dockerfile and remove unnecessary dependencies

Move bash scripts to their own folder

Add environment variables for PUID and PGID

Refactor and move parameter compilation to its own script

Refactor server launch script and remove unnecessary code
Using su causes issues with tini, because su creates a new process hierarchy tini cannot properly forward signals to its childs, causing the game server to not shutdown properly in some cases, using gosu fully fixes those issues
It being wrong didnt cause any issues but better to fix it anyway
@Renari
Copy link

Renari commented Oct 21, 2024

Tried your branch and it fixed all the issues I was having trying to deploy on TrueNAS.

@Diyagi
Copy link
Contributor Author

Diyagi commented Oct 27, 2024

I made the choice of not printing the logs lines with timestamps because it seemed unnecessary, it is already possible to get logs from docker with timestamps by using docker logs -t.

@Diyagi
Copy link
Contributor Author

Diyagi commented Oct 28, 2024

Changed the build action to trigger on an new release in github.
Before merging this, i do recommend cherry-picking just the action build commits and creating an tag release of the current state of main branch as 1.0.0, this will make sure that anyone wanting to run an older version of core keeper can do so by using an older version of the image where issues with world gen are fixed.
After that this branch can be merged and another tag release can be made.

@Diyagi Diyagi marked this pull request as ready for review October 28, 2024 07:28
@Micke90s
Copy link
Contributor

I just saw that the new line \n in the SEASON description does not work as suspected. I think <br/> should work. Could you please check this?
https://github.com/Diyagi/core-keeper-dedicated/blob/e86e15d6c7dea18abf2c4bf6443fe3b3e6af758a/README.md?plain=1#L101

@Diyagi
Copy link
Contributor Author

Diyagi commented Oct 30, 2024

I just saw that the new line \n in the SEASON description does not work as suspected. I think <br/> should work. Could you please check this? https://github.com/Diyagi/core-keeper-dedicated/blob/e86e15d6c7dea18abf2c4bf6443fe3b3e6af758a/README.md?plain=1#L101

Oh, thats true, sorry
I dont usually deal with readme, i throw it to a friend who likes to do this e.e

@arguser arguser merged commit 4feea26 into escapingnetwork:main Nov 1, 2024
@Diyagi Diyagi deleted the patch-1 branch November 3, 2024 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants