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.

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

Jump to bottom

if cmdr is near a body it'll show the distance from said body to the … #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

if cmdr is near a body it'll show the distance from said body to the … #12

wants to merge 1 commit into from

Conversation

iMouath
Copy link

@iMouath iMouath commented Sep 6, 2018

  • Added a way to view nearby body.
  • query EDDB through kodeblox api to get body distance from arrival.

houndci-bot
houndci-bot reviewed Sep 6, 2018
View reviewed changes
edj.logparser.js
case 'StartJump':
edjdata.player.pos = {
...edjdata.player.pos,
...logItem,
Copy link

Choose a reason for hiding this comment

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

Expected an assignment or function call and instead saw an expression.
Missing semicolon.
Unrecoverable syntax error. (50% scanned).

edj.logparser.js
edjdata.player.pos = {
...edjdata.player.pos,
...logItem,
};
Copy link

Choose a reason for hiding this comment

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

Unexpected '}'.

edj.logparser.js
break;
case 'StartJump':
edjdata.player.pos = {
...edjdata.player.pos,
Copy link

Choose a reason for hiding this comment

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

Expected '}' to match '{' from line 189 and instead saw '...'.
Missing semicolon.

edj.logparser.js
};
break;
case 'StartJump':
edjdata.player.pos = {
Copy link

Choose a reason for hiding this comment

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

Expected '}' to match '{' from line 1 and instead saw 'edjdata'.

edj.logparser.js
...logItem,
};
break;
case 'StartJump':
Copy link

Choose a reason for hiding this comment

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

Expected '}' to match '{' from line 3 and instead saw 'case'.
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
Expected '}' to match '{' from line 2 and instead saw ':'.

edj.logparser.js
case 'Location':
edjdata.player.pos = {
...edjdata.player.pos,
...logItem,
Copy link

Choose a reason for hiding this comment

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

Expected an assignment or function call and instead saw an expression.
Missing semicolon.

edj.logparser.js
edjdata.player.pos = {
...edjdata.player.pos,
...logItem,
};
Copy link

Choose a reason for hiding this comment

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

Unexpected '}'.
Unnecessary semicolon.

edj.logparser.js
break;
case 'Location':
edjdata.player.pos = {
...edjdata.player.pos,
Copy link

Choose a reason for hiding this comment

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

Expected '}' to match '{' from line 183 and instead saw '...'.
Missing semicolon.

edj.getdistance.js
const jsonResponse = JSON.parse(req.responseText);
console.log(jsonResponse.docs[0].distance_to_arrival);
edjdata.player.pos.DistanceToArrival = jsonResponse.docs[0].distance_to_arrival;
edjGui.updateGui();
Copy link

Choose a reason for hiding this comment

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

'edjGui' is not defined.

edj.getdistance.js
/* edjGui */
edjGetdistance = {
request(sysname) {
const req = new XMLHttpRequest();
Copy link

Choose a reason for hiding this comment

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

'XMLHttpRequest' is not defined.

itssimple
itssimple requested changes Sep 6, 2018
View reviewed changes
Copy link
Member

@itssimple itssimple left a comment

Choose a reason for hiding this comment

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

Would it be too much to ask, to change the API, to use the one we at FuelRats have instead?

https://github.com/FuelRats/EDDB_JsonAPI
https://colinhiggs.github.io/pyramid-jsonapi/client.html#filter-examples

@iMouath
Copy link
Author

iMouath commented Sep 6, 2018

Sure thing, I will read on the FuelRats API docs, and revise the PR :)

@itssimple
Copy link
Member

itssimple commented Sep 6, 2018 •
edited
Loading

Thanks a bunch! :) (I don't know if the docs are working, but it should be fairly simple.)

https://system.api.fuelrats.com is the domain you'd want to use at least. ^^

You can do searches like this:
https://system.api.fuelrats.com/systems?filter[bodies.name:like]=Fuelum*&include=bodies,bodies.stations&fields[systems]=name,is_populated&fields[stations]=name,max_landing_pad_size,distance_to_star&fields[bodies]=name,group_name,type_name,distance_to_arrival

@iMouath
Copy link
Author

iMouath commented Sep 7, 2018

Hmm, is the filter case sensitive?
this will return true data:
https://system.api.fuelrats.com/systems?filter[bodies.name:like]=Sol
this won't:
https://system.api.fuelrats.com/systems?filter[bodies.name:like]=SOL

and how would I escape spaces, since %20 does't work either.

@UncleClapton
Copy link
Member

UncleClapton commented Sep 7, 2018 •
edited
Loading

Butting in here as I have prior experience with our systems API.

To answer your first question, like is indeed case sensitive. ilike is the case insensitive of like.

As a word of warning, I would avoid using like/ilike filters as they are extremely slow and quite taxing on the server. Since we're never really dealing with partial names, I would recommend using the eq filter instead.

As for the case sensitivity problem you're seeing... I think there is something up with the field selector you're using that is causing the system data to only appear when the name is in lower case. Weird...

With all this in mind, what you probably want to be using is this query:
/systems?filter[name:eq]=${encodeURIComponent(system.toUpperCase())}&include=bodies.stations

EDIT: punctuation and updating the query provided

@itssimple
Copy link
Member

Yeah, my experience with the system-api we have, is what I learned from trial and error during the 20 minutes I poked at it yesterday. :) So better listen to @UncleClapton, he probably knows better than me. ^^

@iMouath
Copy link
Author

iMouath commented Sep 7, 2018 •
edited
Loading

@UncleClapton Thank you for clarifying that :)

Since @itssimple is readying from the journal all systems and bodies are uppercase.
But I just tested more systems and they work however some don't and I cannot find what is the cause exactly

No results for UGP 184 2"

Works perfectly:

notice that It doesn't even require encoding them.

edit: fix urls
edit note: I'm querying the bodies not the systems.

edit3: https://system.api.fuelrats.com/systems/18158/bodies shows no bodies
So I guess database doesn't have them? They do exist in EDDB however.

@UncleClapton
Copy link
Member

@iMouath looks like UGP 184 does not have any known bodies in our database. If you look at the system (https://system.api.fuelrats.com/systems?filter[name:eq]=UGP%20184&include=bodies) you will notice that there are no bodies that exist.

@iMouath
Copy link
Author

iMouath commented Sep 7, 2018

@UncleClapton thank you so much for confirming that lol ... that was fun.

@itssimple It's up to you, I already wrote the for both API the in-house and https://elitebgs.kodeblox.com/api/eddb/v3/

@UncleClapton
Copy link
Member

UncleClapton commented Sep 7, 2018 •
edited
Loading

@iMouath yeah idk how long ago our bodies table was updated, but I'll ask if we can get a comprehensive update done

Btw, if you're interested in squeaking with us in real time, our IRC is always open! Feel free to hop on https://kiwi.fuelrats.com/ and ping me (Clapton) so I can get you into our semi-public dev room. If you're already familiar with IRC, you can point your client to irc.fuelrats.com:+6697

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@itssimple itssimple itssimple requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@iMouath @itssimple @UncleClapton @houndci-bot