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

Remove hardcoded URLs in favor of Config.routes #5557

Merged
merged 6 commits into from
Jul 1, 2019

Conversation

scheibo
Copy link
Contributor

@scheibo scheibo commented Jun 26, 2019

As discussed previously, the primary motivation here is to be able to host a staging/testing environment for Pokemon Showdown's current set up. Supporting arbitrary client configurations and truly generic hosting setups is a non-goal. Starting with the server side changes here because the client changes are more involved.


After this change, running git grep pokemonshowdown | grep -v "* Pokemon Showdown - http://pokemonshowdown.com/":

LICENSE:Copyright (c) 2011-2019 Guangcong Luo and other contributors http://pokemonshowdown.com/
PROTOCOL.md:> `http://play.pokemonshowdown.com/action.php?act=upkeep&challstr=CHALLSTR`
PROTOCOL.md:> Otherwise, you'll need to make an HTTP POST request to `http://play.pokemonshowdown.com/action.php`
README.md:  [1]: http://pokemonshowdown.com/
README.md:  [4]: http://pokemonshowdown.com/
README.md:- See http://pokemonshowdown.com/credits

Docs

config/config-example.js:exports.loginserver = 'http://play.pokemonshowdown.com/';
config/config-example.js:	root: 'pokemonshowdown.com',
config/config-example.js:	client: 'play.pokemonshowdown.com',
config/config-example.js:	dex: 'dex.pokemonshowdown.com',
config/config-example.js:	replays: 'replay.pokemonshowdown.com',
package.json:  "homepage": "http://pokemonshowdown.com",

Config

data/rulesets.js:		// implemented in sim/battle.js, see https://dex.pokemonshowdown.com/articles/battlerules#endlessbattleclause for the specification.
server/chat-plugins/info.js: * Pokemon Showdown - https://pokemonshowdown.com/
server/ladders-local.ts: * play.pokemonshowdown.com.
server/ladders-remote.ts: * play.pokemonshowdown.com.

Comments

server/chat-plugins/info.js:			`- <a href="https://replay.pokemonshowdown.com/gennextou-120689854">Zergo vs Mr Weegle Snarf</a><br />` +
server/chat-plugins/info.js:			`- <a href="https://replay.pokemonshowdown.com/gennextou-130756055">NickMP vs Khalogie</a>`

Specific replays which wouldnt exist on another server

server/chat.js:const LINK_WHITELIST = ['*.pokemonshowdown.com', 'psim.us', 'smogtours.psim.us', '*.smogon.com', '*.pastebin.com', '*.hastebin.com'];
server/chat.js:			'pokemonshowdown.com': 1,

Whitelisting

server/ladders-remote.ts:		return [`<tr><td><strong>Please use the official client at play.pokemonshowdown.com</strong></td></tr>`];

Only 'play.pokemonshowdown.com' is official, so this is still correct

server/static/404.html:<p>This is a <a href="http://www.pokemonshowdown.com/">Pok&eacute;mon Showdown</a> server.</p>
server/static/index.html:<p>This is a <a href="http://www.pokemonshowdown.com">Pok&eacute;mon Showdown</a> server!</p>

PITA to change without introducing a build for these files to replay the constants. Will need to do that on the client, but seems like overkill here. Also contains a reference to psim.us, once again, not sure worth what it would take to generalize

test/server/chat.js:			`<a href="//dex.pokemonshowdown.com/pokemon/oshawott" target="_blank"><psicon pokemon="Oshawott"/></a> &gt;w&lt;`
test/server/chat.js:			`<a href="//dex.pokemonshowdown.com/items/beastball" target="_blank">[Beast ball]</a> &gt;w&lt;`

Tests

translations/dutch.json:		"[TN: Link to the PS rules for your language]https://pokemonshowdown.com/rules": "https://pokemonshowdown.com/pages/rules-nl",
translations/english.json:		"[TN: Link to the PS rules for your language]https://pokemonshowdown.com/rules": "",
translations/french.json:		"[TN: Link to the PS rules for your language]https://pokemonshowdown.com/rules": "https://pokemonshowdown.com/pages/rules-fr",
translations/german.json:		"[TN: Link to the PS rules for your language]https://pokemonshowdown.com/rules": "https://pokemonshowdown.com/pages/rules-de",
translations/hindi.json:		"[TN: Link to the PS rules for your language]https://pokemonshowdown.com/rules": "https://pokemonshowdown.com/pages/rules-hi",
translations/italian.json:		"[TN: Link to the PS rules for your language]https://pokemonshowdown.com/rules": "https://pokemonshowdown.com/pages/rules-it",
translations/japanese.json:		"[TN: Link to the PS rules for your language]https://pokemonshowdown.com/rules": "https://pokemonshowdown.com/pages/rules-ja",
translations/portuguese.json:		"[TN: Link to the PS rules for your language]https://pokemonshowdown.com/rules": "https://pokemonshowdown.com/pages/rules-pt",
translations/simplifiedchinese.json:		"[TN: Link to the PS rules for your language]https://pokemonshowdown.com/rules": "https://pokemonshowdown.com/pages/rules-zh",
translations/spanish.json:		"[TN: Link to the PS rules for your language]https://pokemonshowdown.com/rules": "https://pokemonshowdown.com/pages/rules-es",
translations/traditionalchinese.json:		"[TN: Link to the PS rules for your language]https://pokemonshowdown.com/rules": "https://pokemonshowdown.com/pages/rules-tw",
translations/turkish.json:		"[TN: Link to the PS rules for your language]https://pokemonshowdown.com/rules": "https://pokemonshowdown.com/pages/rules-tr",

Should probably also not be hardcoded, but translations are half baked anyway

@scheibo scheibo requested a review from HoeenCoder as a code owner June 26, 2019 19:45
server/chat-formatter.ts Outdated Show resolved Hide resolved
config/config-example.js Outdated Show resolved Hide resolved
config/config-example.js Outdated Show resolved Hide resolved
server/index.js Outdated Show resolved Hide resolved
@Zarel
Copy link
Member

Zarel commented Jul 1, 2019

Looks good. One day, though, we should strongly type the Config object exported from config-loader.

Maybe it should extract the type exported from config-example.

@Zarel
Copy link
Member

Zarel commented Jul 1, 2019

There's a pending review request from Hoeen so I guess I'll leave this unmerged for now?

@scheibo
Copy link
Contributor Author

scheibo commented Jul 1, 2019

The pending review request is automatic due to CODEOWNERS, but I dont think this is a change Hoeen would care about...

@scheibo scheibo merged commit a61c10e into smogon:master Jul 1, 2019
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.

3 participants