philipdouglas / r2-d7

A bot for xwingtmg.slack.com
MIT License
13 stars 14 forks source link

Split list formatter into smaller methods #25

Closed Gerko32 closed 4 years ago

Gerko32 commented 4 years ago

Split list formatter into smaller methods

This is a safe refactor before making more changes. My real goal is to change the links so they point to the right pilot in cases of same pilot name being for multiple ships (e.g. Anakin). I was first splitting this into smaller methods to understand the code

Gerko32 commented 4 years ago

Fixed as you requested.

For the wiki_link, it seems like naming has a pattern, so maybe we can have a dictionary. Where are you loading all the data? I I will give it a try in a separate merge request

philipdouglas commented 4 years ago

Fixed as you requested.

For the wiki_link, it seems like naming has a pattern, so maybe we can have a dictionary. Where are you loading all the data? I I will give it a try in a separate merge request

The data is loaded here: https://github.com/FreakyDug/r2-d7/blob/8d5a0bc3fc26e85de264610c697892b6872d56b3/r2d7/core.py#L93 The cardlookup component has some hardcoded data structures similar to what you're describing here: https://github.com/FreakyDug/r2-d7/blob/8d5a0bc3fc26e85de264610c697892b6872d56b3/r2d7/cardlookup.py#L22 and does some preprocessing on the raw data here: https://github.com/FreakyDug/r2-d7/blob/8d5a0bc3fc26e85de264610c697892b6872d56b3/r2d7/cardlookup.py#L90

danrs commented 4 years ago

Hey @Gerko32, have you seen this post about the wiki? https://community.fantasyflightgames.com/topic/310627-wiki-is-now-a-disaster-and-its-fandoms-fault/

Fandom have rolled out a breaking change which has really messed up the x-wing Wiki, and Wazat hasn't decided yet whether he's going to update it to fix it.

I've never been a massive fan of the fandom wiki style anyway (especially on mobile), so I've been thinking about switching the R2-D7 links to point to https://xhud.sirjorj.com/xwing.cgi/? instead. This site is much cleaner and clearer, and also has the advantage of showing points history for cards. I've been chatting with sirjorj (the site author) and he's enabled an xws argument (xws=) which would be a lot less hassle than generating name-based links and matching them up with the wiki. What do you think?

Gerko32 commented 4 years ago

Its sad to hear about fandom :( but I will love to help with this project. @danrs how could I help? do we know when will this new site start supporting xws? Is this site's code also in github?

danrs commented 4 years ago

@Gerko32 the site in question is https://xhud.sirjorj.com/xwingcgi.html. It is run by sirjorj, and the code is open source on gitlab at https://gitlab.com/sirjorj/xwingcgi.

In terms of how you can help, I've barely looked at the URL-generating code, so you probably have a better idea of what needs to be done than I do. My rough thinking was that we can probably just change the link-generating methods to build links to xwing.cgi instead of to the wiki, and along the way we can remove any of the nasty workarounds that were needed for wiki links

I've been chatting with sirjorj in the X-Wing Vassal League Slack. He says the xws support is now online: you can use xws= as an arg instead of pilot=/upgrade=. For example: https://xhud.sirjorj.com/xwing.cgi/pilot2?xws=nusquadronpilot or https://xhud.sirjorj.com/xwing.cgi/upgrade2?xws=deadmansswitch. It looks like a few things are still missing (currently it only supports pilots and upgrades, not ships or conditions etc) so I'm following up with him now in the #tech channel in that slack

danrs commented 4 years ago

sirjorj has now added support for xws on the condition and ship endpoints