Skip to content

Fix for the sketchy Yahoo API #115

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.

Sign up for GitHub

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

Open
michael-elbert wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix for the sketchy Yahoo API #115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c39d5de
Fix for the sketchy Yahoo API
michael-elbert Oct 12, 2016
bd708da
Oops, left this in there when I was testing.
michael-elbert Oct 12, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 20 additions & 13 deletions lib/mule.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ function addreloader(mule, target) {
var rld = $('<div class="button">')
rld.text('\u21bb')
if (mule.data) {
var updated = new Date(mule.data.query.created)
var updated = new Date(mule.data.created)
Copy link
Owner

@atomizer atomizer Oct 13, 2016

Choose a reason for hiding this comment

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

the query object and it's created property are inherent to the yql api. With your changes this is same as new Date() which is not correct if loading from cache. You need to set this timestamp yourself after updating.

rld.attr('title', 'last updated: ' + updated.toLocaleString())
}
rld.click(function(){ mule.reload() })
Expand Down Expand Up @@ -224,7 +224,11 @@ Mule.prototype.query = function(ignore_cache) {

this.log('loading data')
window.realmAPI('char/list', CR, function(xhr) {
xhr.done(onResponse).fail(onFail)
if(xhr.Chars){
onResponse(xhr);
} else {
onFail();
}
})

function onFail() {
Expand All @@ -239,15 +243,9 @@ Mule.prototype.query = function(ignore_cache) {
}

function onResponse(data) {
//console.log(data);
self.busy = false;
if (!data.query || !data.query.results) {
self.error(data.query ? 'server error' : 'YQL service denied');
if (data.query) {
self.log('full response:' + JSON.stringify(data.query))
}
return;
}
var res = data.query.results
var res = data;

function watchProgress(percent) {
if (typeof percent != 'string') {
Expand Down Expand Up @@ -309,13 +307,22 @@ var STATTAGS = 'MaxHitPoints MaxMagicPoints Attack Defense Speed Dexterity HpReg
var STATABBR = 'HP MP ATT DEF SPD DEX VIT WIS'.split(' ')
Mule.prototype.parse = function(data) {
if (this.overlay) this.overlay.hide()

var d = data.query.results.Chars
console.log(data);
Copy link
Owner

@atomizer atomizer Oct 13, 2016

Choose a reason for hiding this comment

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

no

if(data.Chars){
var d = data.Chars
} else {
var d = data.query.results.Chars
}
d = {
Char: d.Char,
Account: d.Account || {}
}
data.query.results.Chars = d
if(data.Chars){
data.Chars = d
} else {
data.query.results.Chars = d
}

Copy link
Owner

@atomizer atomizer Oct 13, 2016

Choose a reason for hiding this comment

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

These switches are presumably for compatibility with yql? If you intend to have it as a fallback, actually do it.


// check if data changed?
/* if (this.data && compare(d, this.data.query.results.Chars)) {
Expand Down
20 changes: 16 additions & 4 deletions lib/realmapi.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@

Copy link
Owner

@atomizer atomizer Oct 13, 2016

Choose a reason for hiding this comment

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

all of the whitespace changes are uncalled for. also use tabs for indentation (in all the other places)

(function($, window) {

var BASEURL = [
Expand Down Expand Up @@ -27,11 +28,10 @@ function update_counter() {
$('#counter').text(_cnt).parent().toggle(!!_cnt);
}


var x2js = new X2JS();
Copy link
Owner

@atomizer atomizer Oct 13, 2016

Choose a reason for hiding this comment

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

I don't see where this is coming from

function realmAPI(path, opts, extraopts, callback) {
opts.ignore = Math.floor(1e3 + 9e3 * Math.random())
var url = BASEURL + path + '?' + $.param(opts)

if (typeof extraopts == 'function') {
callback = extraopts
extraopts = {}
Expand All @@ -48,15 +48,27 @@ function realmAPI(path, opts, extraopts, callback) {
ifr.src = url
return
}

/*
Copy link
Owner

@atomizer atomizer Oct 13, 2016

Choose a reason for hiding this comment

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

please don't leave commented code, version control exists for a reason

queue_request({
dataType: 'jsonp',
url: 'https://query.yahooapis.com/v1/public/yql',
data: {
q: 'select * from xml where url="' + url + '"',
format: 'json'
},
complete: callback
complete: function(result){
console.log(JSON.stringify(result));
callback(result);
}
})
*/
queue_request({
url: 'https://cors-anywhere.herokuapp.com/' + url,
complete: function(result){
var jsonResult = x2js.xml_str2json(result.responseText);
//console.log(JSON.stringify(jsonResult));
callback(jsonResult);
Copy link
Owner

@atomizer atomizer Oct 13, 2016

Choose a reason for hiding this comment

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

you changed the type of the argument that is passed to callback (it used to be the xhr object) and didn't fix all instances where this function is used (namely the migrate/progress call in mule.js)

}
})
}

Expand Down