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

Feature/environment configuration #116

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

thmsndk
Copy link
Contributor

@thmsndk thmsndk commented Feb 24, 2024

The following PR aims to move more configurations out into configuration files that are not comitted to git, this way you have more control on the specific server installation.

  • Fixes an issue where api/characters_and_servers would return a hardcoded 192.168.1.125 ip when running in SDK mode
    • This allows headless (caracAL, ALClient) / custom clients that rely on that api response to connect to the websocket to connect properly
  • Added new environment.py
    • you can configure game_name, appengine_id, live_domain, sdk_domain
    • The hardcoded IP_TO_SUBDOMAIN has also been moved here.
  • DISCORD
    • Moved some hardcoded links to #welcome into environment.py
    • You can now customize your discord bot token and channels in variables.js
      image
    • You can now enable discord in sdk mode
      • discord integration is disabled by default in template.variables.js
  • You can now override values in server.js mode
    • this allows you to disable the drm_check that causes the authfail debuff
    • you can also disable / enable the notverified_debuff
    • any property inside mode can be changed / adjusted

closes #94

-WARNING: This PR will require the live servers to update their config file(s)
  • secrets.py the discord url for the welcome invite needs to be supplied
  • variables.js make sure to configure the discord section to be ENABLED, the bot TOKEN and the EVENT_CHANNELS
    • discord_url has been replaced in the config with a discord section
  • add the new environment.py the IP_TO_SUBDOMAIN should be untouched, if running in SDK mode, make sure to map REQUEST_IP_TO_HOSTNAME properly

@thmsndk
Copy link
Contributor Author

thmsndk commented Feb 24, 2024

I currently can't think of other things I want to add to this PR, reviews will be much appreciated. I have tested it on my local dev server, and everything works as expected as far as I can tell.

@thmsndk thmsndk marked this pull request as draft February 25, 2024 13:45
@thmsndk thmsndk marked this pull request as ready for review February 25, 2024 13:49
Copy link
Collaborator

@Telokis Telokis 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 this PR!
I left a few comments and questions.
I feel like some auto-formatting happened in the HTML files, that was probably not on purpose.

More important, this is a very big and deep change. I think it's very important that you also provide an extensive migration guide to make sure @kaansoral has a reference to follow.
I can't ever merge this myself due to the big deployment implications, it should probably be at a time where you are both available to live debug/migrate as it is being put in place and deployed.

api.py Outdated Show resolved Hide resolved
htmls/contents/guide.html Outdated Show resolved Hide resolved
htmls/contents/section_buttons.html Outdated Show resolved Hide resolved
htmls/page.html Outdated Show resolved Hide resolved
var query=args.secret&&"desktop="+(!is_comm&&1||"")+"&secret="+args.secret||undefined;
if(location.protocol=="https:") window.socket=io('wss://'+server_addr+':'+server_port,{secure:true,transports:['websocket'],query:query});
else window.socket=io(server_addr+':'+server_port,{transports:['websocket'],query:query});
add_log("Connecting to the server.");
else window.socket=io('ws://'+server_addr+':'+server_port,{transports:['websocket'],query:query});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does it now explicitely need the protocol but didn't before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be able to connect via caracAL to the server on http, the protocol was required for a correct websocket / socket.io connection to be established.

# Discord
DISCORD = {
"URL": {
"WELCOME": "https://discord.gg/44yUVeU"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably be named DISCORD_INVITE_LINK instead of WELCOME, for clarity.

Copy link
Contributor Author

@thmsndk thmsndk Apr 12, 2024

Choose a reason for hiding this comment

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

I agree, was just an easy name to represent it as the welcome link i think the welcome name made sense in context of where it was used, I'd have to verify it again though

fast_sdk: 0,
DISCORD: {
ENABLED: false,
TOKEN: "NDXXXXXXXXXXX", // Your discord applications bot token
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this go into secrets instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Secrets is for python.
This is for the node servers

Comment on lines +10 to +13
apple_token: "acXXXXXXXX...",
steam_key: "8aXXXXXXXXX...",
steam_web_key: "B4XXXXXXX...",
steam_partner_key: "F9XXXXXXX...",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same, those tokens can probably move to secrets, is there a reason not to?

character_limit: 3,
DISCORD: {
ENABLED: false,
TOKEN: "NDXXXXXXXXXXX",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Duplicated token? Why is this here as well? (Maybe you don't even know)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The live variables is used as part of the deploy process on the official servers as far as I understand, there is a script that copies this and some other folders into the deployed directory.

@@ -3266,27 +3266,34 @@ function appengine_call(method, args, on_success, on_error) {
}

function discord_call(message) {
// TODO: should it not post to the channel either way? I vote to remove this return
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd argue that the new variables.DISCORD.ENABLED configuration option has indeed made this check obsolete.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think standard, hardcore and test servers are run from the same directory though. So they all share the same variables.js file.

Should the test server actually post to the same channels as the live servers? The same question is true for hardcore servers.

I kinda feel they perhaps need their own configuration as theese two servers are a special kind of servers.

@Telokis
Copy link
Collaborator

Telokis commented Apr 19, 2024

@thmsndk Since this PR is big and I've left several comments, I'll let you tell me when you consider it ready for me to take another look at it.
Please write a small word after each of my open conversations to let me know how you've addressed it so I can better keep track.

@thmsndk thmsndk force-pushed the feature/environment-configuration branch 2 times, most recently from 71f6cb3 to eacd64a Compare April 22, 2024 19:41
@thmsndk thmsndk force-pushed the feature/environment-configuration branch from eacd64a to bba14b9 Compare July 27, 2024 07:03
@Telokis Telokis added Type: enhancement Changes the existing content of the game Scope: game server The game server is impacted Scope: central server The central server is impacted Scope: meta This relates to the repository, code, or environment without meaningful direct impact to the game Status: waiting for review This PR is waiting for a maintainer to review it labels Aug 13, 2024
@thmsndk thmsndk force-pushed the feature/environment-configuration branch from 9f63e68 to 3455645 Compare August 13, 2024 20:57
@thmsndk
Copy link
Contributor Author

thmsndk commented Sep 17, 2024

I'm unsure if my latest commit will break the live production servers as I've been messing around with how "https_mode" is utilized
8c20b82

More research and testing is needed, but server_eval is now fixed on my local docker environment.

@thmsndk thmsndk marked this pull request as draft September 17, 2024 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: central server The central server is impacted Scope: game server The game server is impacted Scope: meta This relates to the repository, code, or environment without meaningful direct impact to the game Status: waiting for review This PR is waiting for a maintainer to review it Type: enhancement Changes the existing content of the game
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discord Integration Custom channels
2 participants