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

add link to ship page (#19) #71

Open
wants to merge 5 commits into
base: release
Choose a base branch
from

Conversation

jan-krueger
Copy link
Contributor

@jan-krueger jan-krueger commented Nov 29, 2020

Hey :),
as requested in issue #19 I have added button to link to their ship detail page. It looks like this at the moment:

https://i.imgur.com/9E8lCcy.png

This was just an idea, and it might be better to add the link to the title and image, or somewhere else. I also added a script that can automatically generate the src/web_resources/HangarXPLOR.Ships.js file. In the script you can define this array to determine what fields you want to extract:

const fields = {
    'name': 'name',
    'url': 'url',
    'thumbnail': 'media.0.images.heap_infobox'
};

The script can be executed by simply calling npm run generate-ships-file

@Da-Geek
Copy link

Da-Geek commented Feb 13, 2021

This looks like it could be very useful.
Maybe link to something a little more useful like fleetyards.net pages rather than the RSI Page?

@jan-krueger
Copy link
Contributor Author

jan-krueger commented Feb 14, 2021

@Da-Geek I think this is a really good idea. I have added it now. - There is a handful of ship variations (18 to be exact) that are just some special variety of another ship like "Caterpillar Pirate Edition" and they don't have a direct match on Shipyard. It shouldn't be too difficult to make educated guesses in the code as to which ship page it should link but I just haven't gotten around to implementing this yet. I will probably do this tomorrow. As far as everything else is concerned, everything seems to be working we might just wanna pick a more suitable icon.

@peter-dolkens
Copy link
Member

peter-dolkens commented Feb 15, 2021

Sorry, have been busy this weekend but I'll try and review more this week.

One thing I'll say, the original PR was ok, but HangarXPLOR won't ever include any direct links to third parties - not a battle I want to fight RE: privacy/security concerns - can already get tricky at times without any references to third parties or passwords.

I'd actually intended to support some third party integrations by setting up a settings page that would let you paste in third-party URLs that you wish to integrate with - but it would always be up to the user to paste those links in the first time.

I'd been meaning to add support for exporting your fleet list to https://www.starship42.com/fleetview/ in this settings area, but there's no reason we couldn't also have options for third party "wiki" links in there too.

@jan-krueger
Copy link
Contributor Author

Fair enough. I can see how that could lead to issues. I have reverted the changes now.

Comment on lines +39 to +43
fs.writeFile('src/web_resources/HangarXPLOR.Ships.js', content, "utf-8", function (error) {
if (error) {
console.error("Failed to write the 'HangarXPLOR.Ships.js' file", error);
};
console.log("Successfully created the 'HangarXPLOR.Ships.js' file");
Copy link
Member

Choose a reason for hiding this comment

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

I like this idea, but I'll have to double check the sort works as required.

The actual order of the items in the list is important - you generally need variants to appear BEFORE the base versions so that the replacement logic works.

Comment on lines -13 to -16
{ 'name': '600i Executive', 'thumbnail': '/media/ew63gkfbd7ki2r/heap_infobox/600i_02.jpg' },
{ 'name': '600i Exploration', 'thumbnail': '/media/8sqwytgh6ij03r/heap_infobox/600i-Exploration.jpg' },
{ 'name': '600i Touring', 'thumbnail': '/media/z642zdp6d3mkzr/heap_infobox/600i-Touring.jpg' },
{ 'name': '600i', 'thumbnail': '/media/8sqwytgh6ij03r/heap_infobox/600i-Exploration.jpg' },
Copy link
Member

Choose a reason for hiding this comment

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

e.g. Notice here that the 600i is the LAST item in the list, whilst the others are alphabetical

Comment on lines -23 to -28
{ 'name': 'Aurora CL', 'thumbnail': '/media/fh1gtu93mndsur/heap_infobox/Rsi_aurora_cl_storefront_visual.jpg' },
{ 'name': 'Aurora ES', 'thumbnail': '/media/9u8061zhf29fir/heap_infobox/Rsi_aurora_es_storefront_visual.jpg' },
{ 'name': 'Aurora LN', 'thumbnail': '/media/ljgowkr9tdwetr/heap_infobox/Rsi_aurora_ln_storefront_visual.jpg' },
{ 'name': 'Aurora LX', 'thumbnail': '/media/xfq27owiysn6ar/heap_infobox/Aurora-LX_Ortho.jpg' },
{ 'name': 'Aurora MR', 'thumbnail': '/media/ohbfgn1ebcsnar/heap_infobox/Rsi_aurora_mr_storefront_visual.jpg' },
{ 'name': 'Aurora', 'thumbnail': '/media/9u8061zhf29fir/heap_infobox/Rsi_aurora_es_storefront_visual.jpg' },
Copy link
Member

Choose a reason for hiding this comment

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

And another Example

Comment on lines -29 to -33
{ 'name': 'Avenger Stalker', 'thumbnail': '/media/3dx8jqsd79dmpr/heap_infobox/Avenger_storefront_visualjpg.jpg' },
{ 'name': 'Avenger Titan Renegade', 'thumbnail': '/media/cg2gcecohj7s6r/heap_infobox/Avenger_cargo_right.jpg' },
{ 'name': 'Avenger Titan', 'thumbnail': '/media/cg2gcecohj7s6r/heap_infobox/Avenger_cargo_right.jpg' },
{ 'name': 'Avenger Warlock', 'thumbnail': '/media/qcv2n7ms9qwj8r/heap_infobox/Avenger_EMP_02.jpg' },
{ 'name': 'Avenger', 'thumbnail': '/media/3dx8jqsd79dmpr/heap_infobox/Avenger_storefront_visualjpg.jpg' },
Copy link
Member

Choose a reason for hiding this comment

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

And an even better example where there isn't just the base "Avenger", but also "Avenger Titan" is different to "Avenger Titan Renegade"

Comment on lines -42 to -47
{ 'name': 'Constellation Andromeda', 'thumbnail': '/media/vzyhde6cjgsn7r/heap_infobox/Andromeda_Storefront.jpg' },
{ 'name': 'Constellation Aquila', 'thumbnail': '/media/u0pbc9k058nuhr/heap_infobox/Aquila_Storefront.jpg' },
{ 'name': 'Constellation Phoenix Emerald', 'thumbnail': '/media/kkakjxny421xfr/heap_infobox/Connie_Emerald.jpg' },
{ 'name': 'Constellation Phoenix', 'thumbnail': 'https://media.robertsspaceindustries.com/mibl3d305r0q6/heap_infobox.png' },
{ 'name': 'Constellation Taurus', 'thumbnail': '/media/3vj4o4l5uggk7r/heap_infobox/Taurus-Storefront.jpg' },
{ 'name': 'Constellation', 'thumbnail': '/media/3vj4o4l5uggk7r/heap_infobox/Taurus-Storefront.jpg' },
Copy link
Member

Choose a reason for hiding this comment

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

Constellation in with special ordering

@peter-dolkens
Copy link
Member

peter-dolkens commented Mar 11, 2021

Sorry, I forgot to hit "start review" on the comments above.

I always forget you can't see them until I start the review :\

I've also started work on the "settings" pane - you'll see it actually has something in it in the release I'm about to do!

Hoping to add more in there soon, which can then be hooked and used for third party links.

@peter-dolkens
Copy link
Member

peter-dolkens commented Mar 18, 2021

I've taken a few ideas from this PR and am in the process of working them into a heavily rewritten version of HangarXPLOR

https://github.com/dolkensp/HangarXPLOR/compare/v2

I'm actually loading the ship matrix dynamically now, altering and injecting records as required, then sorting it in the required ordering.

This means I'll have access to lots more functionality going forwards.

All part of an effort to make HangarXPLOR a bit more resilient to changes to support some upcoming collaboration work.

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