-
Notifications
You must be signed in to change notification settings - Fork 63
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
base: main
Are you sure you want to change the base?
Feature/environment configuration #116
Conversation
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. |
There was a problem hiding this 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.
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}); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
apple_token: "acXXXXXXXX...", | ||
steam_key: "8aXXXXXXXXX...", | ||
steam_web_key: "B4XXXXXXX...", | ||
steam_partner_key: "F9XXXXXXX...", |
There was a problem hiding this comment.
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", |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
@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. |
71f6cb3
to
eacd64a
Compare
eacd64a
to
bba14b9
Compare
game_name,appengine_id,domain name can also be configured
discord url can be configured in secrets.py
…er changing internal ip at will
9f63e68
to
3455645
Compare
…er container using actual_ip
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 More research and testing is needed, but |
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.
api/characters_and_servers
would return a hardcoded192.168.1.125
ip when running in SDK modegame_name
,appengine_id
,live_domain
,sdk_domain
IP_TO_SUBDOMAIN
has also been moved here.environment.py
variables.js
template.variables.js
server.js mode
drm_check
that causes the authfail debuffnotverified_debuff
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 suppliedvariables.js
make sure to configure the discord section to beENABLED
, the botTOKEN
and theEVENT_CHANNELS
environment.py
the IP_TO_SUBDOMAIN should be untouched, if running in SDK mode, make sure to map REQUEST_IP_TO_HOSTNAME properly