search

magnusbakken / espn-fantasy-autopick

A Chrome extension that lets you automatically add active players to the current roster in an ESPN NBA/NHL fantasy league.
MIT License
11 stars 6 forks source link

NHL #9

Closed Nathan-Lefebvre closed 3 years ago

Nathan-Lefebvre commented 3 years ago

Hi! Your extension is awesome! I wanted to used it also for hockey, so I made some change to it in order to works for hockey. It does work fine, but in some rare occasion the UTIL slot is not fill leaving an available player on the bench. I never coded with javascipt, so I was wondering if you were interested in the modified extension for NHL. Thank you!

magnusbakken commented 3 years ago

I'm definitely interested! Feel free to make a PR.

If the code is similar enough that both can be supported with the same codebase we can probably figure out a way to merge them. Alternatively you can fork the project and maintain your own copy.

I imagine there should be two separate extensions so they can have separate themes, even if the code is nearly identical.

magnusbakken commented 3 years ago

When it comes to the UTIL slot issue, I know there are already cases where the extension fails to put in all the players for the NBA. Some of it is due to issues with the automatic clicking (e.g. #7). My solution to that is to simply re-run the algorithm once if it detects that the output isn't as expected, which should cover the vast majority of cases.

Another issue is that the algorithm itself isn't perfect. It currently works by sorting the slots by how many players can fill them (those with fewer go first), and then selecting the healthiest available players for each. I don't know much about hockey or the NHL, so I don't know if there's something about that domain that makes this algorithm less suitable. There are definitely ways to improve the algorithm that could apply to the NBA as well.

I assume the positions and player types are completely different in hockey, so that could maybe be a factor as well? For NBA positions it's important to handle the generic guard and forward slots correctly.

Nathan-Lefebvre commented 3 years ago

I don't know if it worked, I'm new to github so its still confusing all those merge/commit/pull request. You are right about the code being nearly identical, I only changed the possible position for players and the link to the correct team (NHL). I also changed the name and logo of the extension for aesthetic purpose. Since it's a modify extension I don't know how to make an extension for chrome like you did, I only dropped a modified zip in my chrome extension in developer mode.

Thanks for your time!

magnusbakken commented 3 years ago

Looking at your PR it looks like there indeed are very few differences. It shouldn't be difficult to extend the codebase so it supports both sports, and publish them as two separate extensions. Thanks for the tip!

I'll probably look into this next weekend. I'll have to create a fantasy NHL team to test the implementation. I noticed there are only three positions (F, D, G) in addition to UTIL. Do you happen to know if it's possible to set up the league so the roster has dual positions like F/D, or any other more specific positions?

For copyright reasons it's probably not so wise to use the NHL logo as the extension icon, so I'll look for something more generic.

magnusbakken commented 3 years ago

@Nathan-Lefebvre I have good news and bad news.

The good news is that I've updated the code to support both leagues. As I expected it wasn't very hard, thanks to the work you did. I think the only stuff that remains to do is to add a different icon and change the color theme for the NHL version.

The bad news is that ESPN doesn't let me create or join an NHL fantasy league at this point, presumably because the season is too close to being done. Which means that I have no way of actually testing my code.

I know that my updated code still works for the NBA, but I have no way of knowing if it works for NHL leagues, and won't be able to test it for myself until the 21/22 season starts. So basically I need you to test the NHL version of the code. If it works for you I can publish it to the extension store, and if other people start using it I'll probably get some more feedback. It'll be tricky to write code that I can't really test at all, but hopefully there won't be too much that needs to be done.

In case you're interested in helping me out, here are the changes I made:

  1. There are now separate outputs for the NBA and NHL versions of the extension. If you run "yarn unpacked" it will build both, and create both "nba" and "nhl" folders inside the build folder. "yarn unpacked-nhl" will copy only the NHL files. When loading the unpacked extension you will now need to point it to build/nhl rather than the main extension folder.
  2. Most of the code is now in the src/shared folder. The stuff that's specific to each league is kept in src/nba and src/nhl. There's quite a lot of duplication between the two league folders, so I may try to do something smarter in the future.

Thanks in advance!

cameronkeif commented 3 years ago

@magnusbakken I tested in my league, but found a few bugs. I opened #11. I have the opposite problem of you, however :sweat_smile: . I do not have a basketball team and was unable to regression test the NBA functionality

magnusbakken commented 3 years ago

@cameronkeif Awesome! Your PR works for the NBA page with one small adjustment. I'll add a comment to the PR.

magnusbakken commented 3 years ago

I've merged the PR now. The only thing that remains is for me to fix up some of the visual stuff and create a new extension in the store. Thanks for the help to both of you. 👍

magnusbakken commented 3 years ago

@cameronkeif One last thing.

Could you take a couple of screenshots, similar to the ones on the Overview at https://chrome.google.com/webstore/detail/espn-nba-fantasy-team-aut/nmehekgchlioodlggejkfhiglajaonie, using the latest version of the NHL code? The one of the main page needs to be 1280x800 and the one for the popup should be 640x400, but I can crop them if they're bigger than that. Since the NHL page is a little taller you may not be able to fit the whole roster there, but the important thing is that the Auto buttons are visible.

Thanks again!

cameronkeif commented 3 years ago

Sure thing -- Here you go! I did not add the arrow to the first image since I haven't installed the necessary scripts in GIMP and I'm a bit busy this morning, but I can do it later if you don't want to.

Main Page (1280 x 800) main

Popup (640 x 400) popup

magnusbakken commented 3 years ago

I've submitted the new extension now. Since the weekend is coming it may not show up until next week. I'll close this issue when the extension is online and you guys can confirm that it's working well. Thanks again for all the help!

magnusbakken commented 3 years ago

Okay, after some issues with the extension being rejected for spurious reasons, the NHL version of the extension is now up at https://chrome.google.com/webstore/detail/espn-nhl-fantasy-team-aut/nageaaodmancfbkhklidfjdhahghejle

@cameronkeif @Nathan-Lefebvre I'm counting on you guys again to verify that it's working correctly, and also spread the word to anybody else you know who may be interested in the extension.

cameronkeif commented 3 years ago

I did a test run in my league, and the skaters look good, but it looks like #12 is affecting the hockey page as well. It was working fine when I wrote it (there's a .gif in the PR), but it looks like the warning at autosetup.js::305 is firing (it can't find the move button).

I did some quick digging but I haven't really gotten a chance to dig into the code that handles the click events and stuff too much. I'm guessing (as you mentioned @magnusbakken) that the DOM has slightly changed in that second table. I did confirm that it's still finding the correct player in the table, it just doesn't seem to know where to move them.

I will take a look later tonight or tomorrow morning and see if I can figure out what's going on.

magnusbakken commented 3 years ago

Ok, since that issue affects both NHL and MLB I suppose we can continue the discussion there.

cameronkeif commented 3 years ago

Looks like I missed two cases in my previous PR :sweat: , sorry about that! Opened #14 which should also fix the MLB mentioned in #12

magnusbakken commented 3 years ago

Beautiful! I'll check that the code still works with the NBA soon.

magnusbakken commented 3 years ago

At long last the fixed version is up. Google keeps rejecting the NHL extension for having an "inaccurate description" even though the description is almost identical to that of the NBA extension, which doesn't get rejected.

cameronkeif commented 3 years ago

Nice! That's so weird, I have never worked on an extension besides this one, so I'm not familiar with how the chrome web store works, but that is very strange!

Regardless, I have tested using the same leagues above (two point leagues, one categories league), and everything seems to work as expected for both goalies and skaters, and both the day and week buttons. Yay!

magnusbakken commented 3 years ago

Nice! I guess the NHL fantasy season is coming to an end so there probably won't be a lot of users for a while, but hopefully people discover it for next season. (Unless ESPN finally implements this simple piece of functionality themselves.) I'll close this issue. Thanks again!