Added support for grepolife.com -- OLD

grcrt / grcrt-script

GRCRT is "the" userscript for MMO web-browser strategy game Grepolis.

https://www.grcrt.net

GNU General Public License v3.0

14 stars 7 forks source link

Added support for grepolife.com -- OLD #2

Closed FivosM closed 6 years ago

FivosM commented 6 years ago

Pull Request type

[ ] Bugfix
[ ] Performance optimization
[x] Refactoring or styling (code change that is neither bugfix nor performance improvement)
[ ] New feature
[ ] Documentation changes

Short description of what it does:

Added support for www.grepolife.com stats

FivosM commented 6 years ago

For some reason, atom has removed some non-displayable characters. I hope it's not such a big problem for you.

tomaski commented 6 years ago

it's probably a whitespace trimming. We'll take a look on the code (and test it) somewhere during the weekend.

A friendly tip: this is not what we consider a "feature". Feature is when something completely new gets added. Something that has not yet been present (probably requiring separate module). Statistics are. Adding new source for stats is what I would consider, refactoring.

FivosM commented 6 years ago

Alright, thanks. Sorry for adding it as a new feature, I should have payed more attention to the pull request type.

tomaski commented 6 years ago

so after a quick glance on the grepolife a few comments:

Grepolis forces use of SSL for externally loaded content, and Grepolife does not provide HTTPPS connectivity (although would not break the script, users will be warned about potentially unsafe content)
Grepolife does not seem to be monitoring each an every world - what if user chooses grepolife for an unsupported world?

FivosM commented 6 years ago

While testing, I was not warned in any way about unsafe content. I don't think we can do anything about that.
We can create a list of supported countries and open https://www.grcrt.net when the current world's country is not supported.

tomaski commented 6 years ago

hi @FivosM how's it going with that list?

FivosM commented 6 years ago

@tomaski I was waiting for the ok to create the list, I will make it now.

tomaski commented 6 years ago

oh, ok :) a pro-tip @FivosM: as our main goal is performance and efficiency, try to make that list dynamic: if a world is not found on grepolife, use grcrt stats (our stats are "maintenance free": whenever new world is opened, stats are created automatically - so that makes it a good failsafe option)