regseb / castkodi

WebExtension to cast videos and music to Kodi.
MIT License
104 stars 24 forks source link

feat: Ajouter le support de VTM GO #48

Closed tatankat closed 3 years ago

tatankat commented 3 years ago

This PR adds support for VTM GO, a commercial belgian video platform. To play the videos the kodi plugin for VTM GO must be installed. For most videos, a login is required. They may also be geo-locked. This PR is tested with the latest version (1.1.2) of the VTM GO plugin.

regseb commented 3 years ago

Thank you for the proposal. I looked at the VTM GO site and found 3 types of video. It would be easier to create a method for each type:

tatankat commented 3 years ago

Thanks for looking into this. I created it this way because the data-id also works for *://vtm.be/vtmgo/afspelen/e* and *://vtm.be/vtmgo/afspelen/m*. You arrive in that last link after clicking on a *://vtm.be/vtmgo/*~m*, where the data-id does not work. So if implemented this way, when they decide to change the html or the url, these pages will continue to function in the addon. When I split it up the url-parsing urls and the data-id urls, this functionality will be lost. (and one of the 2 ways to get the id should be chosen)

The *://vtm.be/vtmgo/afspelen/m* don't follow the exact same url name scheme of the *://vtm.be/vtmgo/*~m* links, but other than that they share the same logic and plugin url. Splitting *://vtm.be/vtmgo/*~m* from *://vtm.be/vtmgo/afspelen/e* will only simplify the regex (keeping the categories-structure for *://vtm.be/vtmgo/afspelen/m*) or remove the need for the categories-structure (keeping almost the seem regex for both m-links). Splitting the 3 url-parsing urls (both m-links and the e-link) does not really simplify, but duplicates the necessary code.

Can you reconsider with this extra information and advise me again on a way forward? Thanks!

regseb commented 3 years ago

I apply the KISS principle (keep it simple, stupid) on this project. With your proposal, there is a disadvantage now and it may have an advantage in the future. With code handling only current cases: there is an advantage now and it may have a disadvantage in the future. I prefer to favor a solution that I am sure I will reap the benefits of now.

Separating into 3 methods, here is the code needed:

tatankat commented 3 years ago

Yes, you're code is much simpler. You did not take the additional movie-url into account (://vtm.be/vtmgo/afspelen/m), so I created 4 methods to keep the code as simple as possible.