# OLog Userscript - Logging messages, planets and researches

For the online text-based browser game OGame I am working on an application with as aim to assist the users where possible, for this I have a server-side part and a client-side part, the respective repositories can be found here for the server-side part and here for the client-side part.

The client-side part is implemented via an userscript, it has been tested to work with GreaseMonkey in Firefox.

The script currently offers the following functionality:

• Setting the server URL via a settings page.
• Retrieving special report keys from the players' inbox, combat, espionage, missile, and recycle messages have their own report keys.
• Retrieving the planets of the user
• Retrieving the research levels of the user

I'd like the review to focus in particular on maintainability since I am new to user scripts. Also, please note that the structure of the content on the web page cannot be changed, as example some div' classes will be named weirdly.

The userscript:

// ==UserScript==
// @name                OLog
// @namespace           http://www.olog.com/
// @description         OLog Userscript
// @updateURL           https://github.com/skiwi2/OLog-Userscript/raw/master/olog.user.js
// @version             0.3pre
// @include             http://s*-*.ogame.gameforge.com/game/*
// @grant               GM_xmlhttpRequest
// @grant               GM_getValue
// @grant               GM_setValue
// ==/UserScript==

"use strict";

'        <span class="textlabel">OLog Settings</span>\n' +
'    </a>\n' +
'</li>\n');

if (b(c.target).closest(".dropdown").length == 0) {
b(".currentlySelected a").removeClass("hover");
b(".currentlySelected").removeClass("focus");
}
});

showOLogSettings();
});

if (window.location.hash === "#ologsettings") {
showOLogSettings();
}

function showOLogSettings() {
for (var i = 0; i < menuTableLiList.length; i++) {
}

var contentWrapperDiv = document.getElementById("contentWrapper");
while (contentWrapperDiv.firstChild) {
contentWrapperDiv.removeChild(contentWrapperDiv.firstChild);
}

'<div id="inhalt">\n' +
'    <div id="planet" style="background-image: url(http://gf2.geo.gfsrv.net/cdndd/09a2a0d07394b5a7b5db40f5cbb8cc.jpg);">\n' +
'        <div id="detailwrapper">\n' +
'            <div id="header_text">\n' +
'                <h2>OLog Settings</h2>\n' +
'            </div>\n' +
'            <div id="planetdata">\n' +
'                <div class="overlay"></div>\n' +
'                <div id="planetDetails">\n' +
'                    <table width="100%" cellspacing="0" cellpadding="0">\n' +
'                        <tbody>\n' +
'                            <tr>\n' +
'                                <td class="desc">Server Group</td>\n' +
'                                <td class="data">' + getWindowVariable("constants.language") + '</td>\n' +
'                            </tr>\n' +
'                            <tr>\n' +
'                                <td class="desc">Universe ID</td>\n' +
'                                <td class="data">' + getWindowVariable("constants.name") + '</td>\n' +
'                            </tr>\n' +
'                            <tr>\n' +
'                                <td class="desc">Player ID</td>\n' +
'                                <td class="data">' + getWindowVariable("playerId") + '</td>\n' +
'                            </tr>\n' +
'                            <tr>\n' +
'                                <td class="desc">Player Name</td>\n' +
'                                <td class="data">' + getWindowVariable("playerName") + '</td>\n' +
'                            </tr>\n' +
'                            <tr>\n' +
'                                <td class="desc">Userscript Version</td>\n' +
'                                <td class="data">' + GM_info.script.version + '</td>\n' +
'                            </tr>\n' +
'                        </tbody>\n' +
'                    </table>\n' +
'                </div>\n' +
'            </div>\n' +
'        </div>\n' +
'    </div>\n' +
'    <div class="c-left"></div>\n' +
'    <div class="c-right"></div>\n' +
'    <div id="buttonz">\n' +
'        <div class="header">\n' +
'            <h2>Your settings</h2>\n' +
'        </div>\n' +
'        <div class="content">\n' +
'            <div class="fieldwrapper">\n' +
'                <label class="styled textBeefy">OLog Instance URL:</label>\n' +
'                <div class="thefield">\n' +
'                    <input class="textinput w200" type="text" size="30" value="' + oLogInstanceUrl + '" id="ologInstanceUrlInput" />\n' +
'                </div>\n' +
'            </div>\n' +
'            <div class="textCenter">\n' +
'                <input class="btn_blue" type="submit" value="Save settings" id="ologSaveSettingsInput" />\n' +
'            </div>\n' +
'            <div class="footer"></div>\n' +
'            <br class="clearfloat" />\n' +
'        </div>\n' +
'    </div>\n' +
'</div>\n' +
'');

var oLogSaveSettingsInput = document.getElementById("ologSaveSettingsInput");
saveSetting("planets_cache", "");
saveSetting("researches_cache", "");
showSuccessMessage("Settings have been saved.");
});
}

var page = getWindowVariable("currentPage");

if (page === "messages") {
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(mutation) {
for(var i = 0; i < mutation.addedNodes.length; i++) {
var node = mutation.addedNodes[i];
if (node.localName === "ul" && node.classList.contains("tab_inner")) {
//send all messages at once such that all report keys can be sent at once to the server
processMessageNodes(node.querySelectorAll(".msg"));
}
}
});
});

observer.observe(document, { childList: true, subtree: true });
}
else if (page === "research") {
var itemBoxDivs = document.querySelectorAll(".item_box");
processResearchNodes(itemBoxDivs);
}

var planetListDiv = document.getElementById("planetList");
if (planetListDiv !== null) {
processPlanetNodes(planetListDiv.children);
}

function processMessageNodes(nodes) {
var reportKeys = {
sr: [],     //spy report
cr: [],     //combat report
rr: [],     //recycle report
mr: []      //missile report
};

for (var i = 0; i < nodes.length; i++) {
var node = nodes[i];
var apiElement = node.querySelector(".icon_apikey");
if (apiElement !== null) {
var key = apiElement.parentNode.href.replace(/.*?:\/\//g, "");
var type = key.substring(0, 2);
if (reportKeys.hasOwnProperty(type)) {
reportKeys[type].push(key);
}
}
}

postData({
endpoint: "keys",
data: {
reportKeys: reportKeys
}
});
}

function processPlanetNodes(nodes) {
var planets = [];

for (var i = 0; i < nodes.length; i++) {
var planetDiv = nodes[i];
var planetId = planetDiv.id.replace("planet-", "");
var planetName = planetDiv.querySelector(".planet-name").innerHTML;
var planetCoordsRaw = planetDiv.querySelector(".planet-koords").innerHTML;
var planetCoordsList = planetCoordsRaw.slice(1, -1).split(":");
planets.push({
id: planetId,
name: planetName,
galaxy: planetCoordsList[0],
solarSystem: planetCoordsList[1],
position: planetCoordsList[2]
});
}

var planetsString = JSON.stringify(planets);
if (getSetting("planets_cache", "") !== planetsString) {
postData({
endpoint: "planets",
data: {
planets: planets
}
});
saveSetting("planets_cache", planetsString);
}
}

function processResearchNodes(nodes) {
var researches = [];

for (var i = 0; i < nodes.length; i++) {
var node = nodes[i];
var detailButtonElement = node.querySelector(".detail_button");
if (detailButtonElement !== null) {
var researchId = detailButtonElement.getAttribute("ref");
var cloneLevelNode = detailButtonElement.querySelector(".level").cloneNode(true);
var children = [].slice.call(cloneLevelNode.children);

for (var j = 0; j < children.length; j++) {
var child = children[j];
if (child.className === "textlabel" || child.className === "undermark") {
cloneLevelNode.removeChild(child);
}
}
var researchLevel = cloneLevelNode.innerHTML.trim();

researches.push({
id: researchId,
level: researchLevel
});
}
}

var researchesString = JSON.stringify(researches);
if (getSetting("researches_cache", "") !== researchesString) {
postData({
endpoint: "researches",
data: {
researches: researches
}
});
saveSetting("researches_cache", researchesString);
}
}

function postData(object) {
console.log(JSON.stringify(object.data));
GM_xmlhttpRequest({
method: "POST",
url: oLogInstanceUrl + "api/userscript/" + object.endpoint,
data: JSON.stringify(object.data),
"Content-Type": "application/json"
},
},
onerror: function(response) {
console.log("error");
}
});
}

data.serverGroup = getWindowVariable("constants.language");
data.universe = getWindowVariable("constants.name");
data.playerId = getWindowVariable("playerId");
data.playerName = getWindowVariable("playerName");
}

/**
* Returns a global variable.
*
* Needed because a @grant other than none puts the script
* in a separate scope.
*
* @param {string} name The name of the global variable.
*/
function getWindowVariable(name) {
return window.eval(name);
}

function showSuccessMessage(message) {
}

function showErrorMessage(message) {
}

function saveSetting(key, value) {
var fullKey = getWindowVariable("constants.language") + ":" + getWindowVariable("constants.name") + ":" + getWindowVariable("playerId") + ":" + key;
GM_setValue(fullKey, value);
}

function getSetting(key, defaultValue) {
var fullKey = getWindowVariable("constants.language") + ":" + getWindowVariable("constants.name") + ":" + getWindowVariable("playerId") + ":" + key;
return GM_getValue(fullKey, defaultValue);
}


The user script can be seen in action below, where it adds the "OLog Settings" menu and shows that settings menu.

Note: Since this question has been posted the project has been renamed to OGNext.

Your code overall is a bit hard to follow (due to it's size), but it is really clean.

I really like the idea of your project, and may be helpful to many.

But lets review the code!

• You have mixed quotes everywhere. But you are consistent about that. You use single-quotes for multi-line strings and double-quotes for everything else.
• Your multi-line strings have this structure:

''+
'    <html>\n'+
'        some cool content\n'+
'    </html>\n'+
''


There's a few things you can improve:

• Get rid of those empty strings
• Place the HTML intentation outside the string. This will allow code minimifiers to reduce the code even further, and removes needless whitespace from your strings.
• Get rid of those \n. It's useless. It won't improve the HTML readability, since you can see it pretty well formatted on an element inspector.
• On your getSettings and setSettings, you have a repeated line of code. You can easily move it to a new function getFullKey().
• Instead of this:

function getWindowVariable(name) {
return window.eval(name);
}


You could do this:

function getWindowVariable(name) {
return Function("'use strict'; return this." + name + ";")();
}


This will prevent lots of security issues, but won't prevent others.
One of the issues is that your functions will be harmful if someone tries to run getWindowVariable('while(1);').
With your version, it will block the browser, but, with this new version, it will throw a ReferenceError, if there's no while method on window.

Or, you can use this slower method:

function getWindowVariable(name) {
var window = Function("return this;")();
var props = name.split(".");
var current = window[props.shift()];
while(props.length) {
if(!(props[0] in current))return;
current = current[props.shift()];
}
return current;
}


This is completelly safe and may prevent some XSS vulnerabilities that you may find. While my first version would still crash if you run getWindowVariable('a,+function(){while(1);}();'), this one is totally safe.

It simply splits the variable name by . and tries to see if each "piece" is a property on the "current" object. It tries to return undefined as soon as possible, reducing the number of checks and speeding up the code.

• On your showOLogSettings(), you have this line:

for (var i = 0; i < menuTableLiList.length; i++) {


You should always store the length of an array as a variable. Like this:

for (var i = 0, length = menuTableLiList.length; i < length; i++) {


This will speed up your code by quite a bit. Reading properties from an object is a lot slower than reading a local variable.

Everytime you have a for loop with the length of an array, you should always store the length in a local variable. And you have a few of those around.

It seems that the O.P. doesn't believe in my claim that storing the length in a local variable doesn't improve performance.

I've built a very basic test bench, where you can order to run a few performance tests.

It isn't perfect, but it is enough to give an idea that I'm not entirelly wrong about this. Depending on the size of the array, it is faster to store the length in a variable. At least on IE and Google Chrome 47.0.2526.106, on my hardware.

Here's the test script:

window.testBench = {
tests: {
'check length on loop': function(array){
for(var i = 0; i < array.length; i++);
},
'save length before': function(array){
for(var i = 0, l = array.length; i < l; i++);
}
},

runTest: function(size, test){
var key = size + ': ' + test;
var array = Array(size);

console.time(key);
this.tests[test](array);
console.timeEnd(key);
},

startTests: function(turns){
console.time('Total time');
console.group('Running performance tests, ' + turns + ' times');

/*
Detects if we are in IE and create smaller arrays.
Running an array any bigger will take FOREVER!!!
It took me 38 seconds for an array with 100000000 (1e8) elements
*/
var limit = 'ActiveXObject' in window ? 1e7 : 1e9;

for(var i = 0; i < turns; i++)
{
console.groupCollapsed('iteration: ' + (i + 1));
for(var size = 100; size <= limit; size *= 10)
{
for(var test in this.tests)
{
this.runTest(size, test);
}
}
console.groupEnd();
}
console.groupEnd();
console.log('Completed running tests ' + turns + ' times');
console.timeEnd('Total time');
}
}

document.getElementById('start').onclick = function(){
var turns = document.getElementById('turns').value | 0;
if(turns == turns && turns > 0)
{
testBench.startTests(turns);
}
}
}
html, body {
font-family: sans-serif;
}
<input type="text" id="turns" placeholder="Turns"/>
<input type="button" id="start" value="Start">
<br>
<sub>For an accurate time, open the console (<kbd>F12</kbd>)</sub>

• Reading through your code, you have the following function:

function postData(object) {
console.log(JSON.stringify(object.data));
GM_xmlhttpRequest({
method: "POST",
url: oLogInstanceUrl + "api/userscript/" + object.endpoint,
data: JSON.stringify(object.data),
"Content-Type": "application/json"
},
},
onerror: function(response) {
console.log("error");
}
});
}


Which looks good, but here is what you can improve in it:

• You log into the console the result of console.log(JSON.stringify(object.data));.

Reading through the documentation of GM_xmlhttpRequest(), you will see that it has an aditional context property, that will have all the properties you've sent.

This means: Remove that line, it isn't doing much there.

• Your console usage is pretty weak. You aren't using it to it's potential.

You simply log load or error, which is useless for debugging. Try this:

function postData(object) {

var url = oLogInstanceUrl + "api/userscript/" + object.endpoint;

GM_xmlhttpRequest({
method: "POST",
url: url,
data: JSON.stringify(object.data),
"Content-Type": "application/json"
},
console.info("Loaded from '%s': %O", url, response);
},
onerror: function(response) {
}
});
}


The %s is used to display a string, and %O will show an object. You can read more about it on the MDN documentation page.

Surprisingly, Google Chrome has %s and %O, Firebug has %s and %o (lowercase O) that offers the same functionality. If you want to run this code on Internet Explorer (is it possible?), you would need to remove the %O from the string.

• You have 2 instances with this structure:

var <something>String = JSON.stringify(<something>);
if (getSetting("<something>_cache", "") !== <something>String) {
postData({
endpoint: "<something>",
data: {
<something>: <something>
}
});
saveSetting("<something>_cache", <something>String);
}


Each <something> is either researches or planets.

You could move it to an entirelly new function. Like this:

function updateEndPoint(endpoint, data) {
var dataString = JSON.stringify(data);
if (getSetting(endpoint + "_cache", "") !== dataString) {
var post = {
endpoint: endpoint,
data: {}
};

post.data[endpoint] = data;

postData(post);
saveSetting(endpoint + "_cache", dataString);
}
}


It doesn't look pretty, but keeps your code DRY.

• As for my final point, as @rolfl stated on another answer:

[...] For Firefox, you need to wrap the header section in to a "preserved" comment block:

/** @preserve
// ==UserScript==
.....
// ==/UserScript==
*/


The preserve is required to keep the comment block after the javascript is processed. This allows the 'compiled' version to keep the references, and for FireFox to know what it is about still.

Other browsers may not have the same requirements.

• Nice review. However, I don't see the need to add that extra "security" measure in getWindowVariable; if the user decides to run getWindowVarible("while(1);");, they are knowingly wrecking their own browser. – SirPython Dec 26 '15 at 5:41
• I don't buy that saving the array length to a variable is a good idea: It creates unnecessary code and it does not seem to improve performance nowadays, see stackoverflow.com/a/17995000/2057294 – skiwi Dec 26 '15 at 13:19
• @SirPython I've built a different version. I know you still don't see the need, but it could help to prevent some XSS attack vectors. (It isn't a certain thing, but nothing is certain). – Ismael Miguel Dec 26 '15 at 17:34
• @skiwi I've built a few basic tests. You can try it yourself. It seems to be somewhat faster for larger arrays, but doesn't make a difference for smaller ones. If you think the tests are skewed in any way, just tell me and we can try to come to a consensus where we agree that the performed tests and the results are accurate. – Ismael Miguel Dec 26 '15 at 17:35

Your code is good, but it could use some improvements here and there:

var fullKey = getWindowVariable("constants.language") + ":" + getWindowVariable("constants.name") + ":" + getWindowVariable("playerId") + ":" + key;


Array joining is faster than messy string concatenation:

var fullKey = [getWindowVariable("constants.language"), getWindowVariable("constants.name"), getWindowVariable("playerId"), key].join(":");


Instead of calling eval or whatever alternative is slightly less horrible, consider wrapping your entire solution in an Immediately Invoked Function Expression and passing in the window as a parameter.

Additionally, moving your code into a prototype chain solution would be more effective.

(function(global){
var OLog = function(){
// constructor
};
OLog.prototype.showOLogSettings = function(){};
// ...
global.OLog = new OLog();
}
}(Function('return this'));


Then you can access the global scope with global.* and internal properties with the this keyword.

If you're specifying the selector, then you shouldn't be using document.querySelector.

document.querySelector is much slower than document.get*.

node.querySelector(".icon_apikey")


into:

node.getElementsByClassName('icon_apikey')[0];


The same goes for all other usages of querySelector and querySelectorAll.

# HTML building

The way you're doing it at the moment is simply not right.

There's two other ways that immediately come to mind on how to do it better:

1. Build it with DOM Elements.
2. Store the HTML template in an external file and use a replacement helper to replace the placeholders with your values.

Like @IsmaelMiguel said, using massive strings like that is bad in both practise and efficiency, and can majorly reduce the minified version's effectiveness.

I would suggest using Option 1, because it is much cleaner and easier to maintain, however if you decided for Option 2, you could use GreaseMonkey's GM_getResourceText function to access the template file from another source.

The first part of the structure in DOM elements:

var div_inhalt = document.createElement('div');
div_inhalt.id = "inhalt";
var div_planet = document.createElement('div');
div_planet.id = "planet";
div_planet.style.backgroundImage = "url(http://gf2.geo.gfsrv.net/cdndd/09a2a0d07394b5a7b5db40f5cbb8cc.jpg)";
var div_detailwrapper = document.createElement('div');
div_detailwrapper.id = "detailwrapper";
var div_headertext = document.createElement('div');
var h2_ologsettings = document.createElement('h2');
h2_ologsettings.textContent = "OLog Settings";
var div_planetdata = document.createElement('div');
div_planetdata.id = "planetdata";
var div_overlay = document.createElement('div');
div_planetdata.appendChild(div_overlay);
var div_planetdetails = document.createElement('div');
div_planetdetails.id = "planetDetails";
var table_data = document.createElement('table');
table_data.width = "100%"
table_data.cellSpacing = "0";
var tbody_data = document.createElement('tbody');
var dataElements = {
"Server Group": getWindowVariable("constants.language"),
"Universe ID": getWindowVariable("constants.name"),
"Player ID": getWindowVariable("playerId"),
"Player Name": getWindowVariable("playerName"),
"Userscript Version": GM_info.script.version
};
Object.keys(dataElements).forEach(function(fieldName){
var tr = document.createElement('tr');
var td_desc = document.createElement('td');
td_desc.textContent = fieldName;
var td_data = document.createElement('td');
td_data.textContent = dataElements[fieldName];
tr.appendChild(td_desc);
tr.appendChild(td_data);
tbody_data.appendChild(tr);
});
div_planetdetails.appendChild(table_data);
div_planetdata.appendChild(div_planetdetails);
div_detailwrapper.appendChild(div_planetdata);
div_planet.appendChild(div_detailwrapper);
div_inhalt.appendChild(div_planet);


The choices for ids on your HTML elements is inconsistent:

   <div id="detailwrapper">
...
<div id="planetDetails">


Stick to a consistent naming structure throughout.

Why are you using a class attribute on a singly used selector:

<div class="overlay"></div>


This is what id is for.

• (1) This may just be some crazy notion in JavaScript, but I know that in (at least) x86 assembly, it is faster to group similar instructions together because of instruction pipe-lining (ex: group your moves, then do your arithmetic). Therefore, it might be faster to group the element creation in your HTML building section. (2) I've seen many webpages use classes for singly used selectors. I think the reason for this is because ID should only be used for certain types of things (sorry, I can't be any more specific as I don't know much about it). – SirPython Dec 26 '15 at 5:46
• 1) There's not really any benefit in grouping them like you suggested, although it's up to Skiwi how he wants to structure that block, I just prefer to do it like the structure entails 2) I know that webpages do use classes for singly used elements however, it's better to get into the practise of using classes for multi-styled elements and ids for singly styled elements – Quill Dec 26 '15 at 5:51
• 1) I was actually using a variant of Option 1 for your HTML building before, however I switched from that as it was simply a very big maintenance hell, you absolutely don't want to be doing that. 2) As noted in the question, I have no control over how I name the div's and if it is with ids or with classes, I need to use the exact same ones as OGame uses right now in order to get a good result. – skiwi Dec 26 '15 at 13:21
• Ah, I see. I would suggest using using Option 2 then – Quill Dec 26 '15 at 13:23
• Your Function('return this') is missing the invocation part. It should be Function('return this')(). Other than that, after a careful analisis, I think you nailed it. Except that I wouldn't call the variable global but window, since it is the window object that is returned. – Ismael Miguel Dec 27 '15 at 9:32