[plugin.video.embycon] 1.3.15

[faush01]

Description

Addon to display and play back media libraries from the Emby Server.

Checklist:

• My code follows the add-on rules and piracy stance of this project.
• I have read the CONTRIBUTING document
• Each add-on submission should be a single commit with using the following style: [plugin.video.foo] v1.0.0

Additional information :

• Submitting your add-on to this specific branch makes it available to any Kodi version equal or higher than the branch name with the applicable Kodi dependencies limits.
• add-on development wiki page.
• Kodi pydocs provide information about the Python API
• PEP8 codingstyle which is considered best practice but not mandatory.
• This add-on repository has automated code guideline check which could help you improve your coding. You can find the results of these check at Codacy. You can create your own account as well to continuously monitor your python coding before submitting to repo.
• Development questions can be asked in the add-on development section on the Kodi forum.

[faush01]

Is this the correct place to submit addons?
Been a week or so since submission, just wondering if I have submit this correctly.

If you are looking for more info about the addon you can check the emby server forums:
https://emby.media/community/index.php?/topic/46651-embycon/

[tadly]

It's the right place and I started reviewing it too.
This just takes time with bigger add-ons and I don't have as much time to spend on reviewing lately.

[tadly]

A few more things.

1. As of our addon-rules, your icon should not have transparency.

2. The whole GoogleAnalytics thing. As far as I can tell there's no way of opting out and personally I'd prefer it to be opt-in all together. I asked the team what they think and will come back to you on that. But having a way of turning it on/off in the settings will be the minimum.

3. Can you please explain the use of default_views.json to me? Reason being that we don't like addons changing the current view.
   Instead, listitems should be prepared properly (setting media-types etc.) and skins should work off of those information's. With the limited time I have I couldn't quite figure out what exactly it is you're are doing so you shall explain it :)

[faush01]

1 - I will update the icon

2 - You can opt out in the settings, under advanced "Log error and Anonymous metrics" perhaps this need qualification "Remotely Log Errors and Anonymous Metrics"
Adding remotely should clear that up.

3 - By default no view is set, it leaves it up to the Skin and Kodi.
However this is not ideal as even when you set the correct media type in the addon for the list items and want to set a new view for say all your seasons or episodes you need to go in and set the view you want in each and every path, i.e. go into tv show X and set the seasons to wide list, set the episodes for season one to wall, go into season 2 and set the view to wall etc, each and every location needs to have the view set.
This option lets you set the view type for each media Type and it is applied if you view media of that type, it does this on a Skin by Skin basis, that is the view IDs need to be available in the defined skin views file and then you need to go inot the addon menu and select Set Default Views to set the views you want for each media type for your selected skin.

[tadly]

Regarding Analytics...
Just got response form a member and he pointed me to our Addon-Rules where it says:

directly using analytics (Google Analytics for example) from within add-ons is not allowed. This should be handled server side by using user-agent.

So for use to be able to accept this add-on the whole analytics stuff needs to be removed

[faush01]

Can you explain what this means

This should be handled server side by using user-agent.

[tadly]

Regarding point 3. I know those problems (Been there myself with a google music plugin) however we do not accept this as of our Addon-Rules:

Plugins should not force certain skin viewtypes on their own. They should set the correct media content from which the correct skin view type is automatically provided.

As a personal note:
We (kodi) try to constantly improve the core and since krypton I don't have to set any views for my google music add-on anymore as the defaults (due to properly set mediatypes etc.) are just fine.

fixed some grammar

[tadly]

Can't really explain it as I didn't write that rule myself but I'd assume this means that in your case, emby itself should do analytics, not the kodi add-on.

Same would apply to other add-ons like plex, gmusic etc.

[faush01]

Ok, so to move forward I would need to:

• remove all remote error and metrics logging
• revamp the internal addon/kodi logging
• remove the default view type setting

Anything else?

the first two are internal and only effect devs, the last one I would run past the user base on the emby forums to see if it was a deal breaker or not.

[tadly]

Sounds good, yes.

Regarding the last point. Maybe turn the custom-view types of and test your add-on with estuary.
The default views estuary picks should be just fine (unless you forgot to set a mediatype etc. which you should double/tribble check at this point).

If estuary looks good but other skins don't it suggests that (whatever skin it may be) needs some work to properly support all mediatypes.

[faush01]

It still only sets a view for a particular path, from my understanding the view is stored in the DB for each path. This means that for each and every season or episode list I wanted to set a view for I would have to go in and set that view.
As I pointed out above this would need to happen for each path in the the library.

I am sure the addon is setting the media type correctly, well actually you can tell me you can see the code in the pull request, look for the getContent() call in functions.py

How should it work, am I seeing the correct functionality of the default Kodi or is something wrong?

[tadly]

I know the problem that arises (as mentioned before, I hit it myself) and you are right in assuming that the user would than have to change the view for every single element (as the path changes due to ids etc.)

My position currently is to just enforce the rules we have.
However I personally do not like this rule either and asked internally if we can make an exception.

[faush01]

So if this is a know issue why are addons not allowed to set the view?
What was the original reasoning behind this? who came up with this?

[tadly]

It's not an issue, it's just a limitation which has been improved upon over the years and will be further improved in coming years.
Forcing a viewtype is considered to be hacky'isch as kodi will load a specific view (depending on the content) first and only after you're able to force-overwrite it again.
That's a bad sequence of events (as you can imagine).

I bet there're other reason too though...

[faush01]

ok point taken. I will work on this over the weekend.

[tadly]

I'm still discussion internally but it's not looking that great, sorry.
The gist of it is that viewtypes and their codes are skin-dependent hence it's bad for add-ons to change it.

[faush01]

I completely understand you dont want display decisions being made by addons, that really should be skins that do that.

Perhaps there should be a built in way for Kodi to apply a Skin View for all content of that type.
i.e.
You navigate to episodes, there is an option (built in function) that you can select that for all content of the type you are currently viewing you use the current view ID from that point on.
This would be very similar to the current approach where the Skin sets the default view for best user experience but if the user wants they can say use this view type for all media of this type from now on.

[faush01]

I some how screwed up this pull request. I have created another one

#1309

All the requested changes should now be in the new pull request.
This one can be closed.

[faush01]

I am going to close this one as it us redundant now, see #1309 for the new pull request with all the changes in it.