Navigation Menu

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

Changed # of add-on settings controls and categories from 100 to 512 #14154

Closed
wants to merge 1 commit into from

Conversation

basrieter
Copy link
Contributor

The new add-on settings are now limited by the generic limit on the number of controls that make up a settings category in Kodi. For those add-ons with 80+ settings, this was causing a problem.

Description

Bumped the limit from 100 to 512 with 475 buttons per category and 37 possible categories.

Motivation and Context

This keeps add-ons what a lot of settings working with the new settings format.

How Has This Been Tested?

Local compile under Windows x64. Tested some add-ons with a lot and with few settings. Double check some Kodi settings.

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@zag2me
Copy link
Contributor

zag2me commented Jul 6, 2018

Anything that needs 80+ settings is just bad interface design, in my most humble opinion ;)

@basrieter
Copy link
Contributor Author

Nah, that is a bit too fast to conclude. For example: my add-on supports over 80 seperate channels (and that number is growing) that allow watching legal on demand content.

One of the options I have is to hide or show a channel and to set a maximum bitrate. So that already exceeds the 80 settings. I hide part of the settings based on a country of origin selection setting.

@peak3d
Copy link
Contributor

peak3d commented Jul 6, 2018

Already mentioned in slack: why not using a list for this?
Sorry if I missed the answer.

@basrieter
Copy link
Contributor Author

That will not help me, as the limit for settings will still be 80 per category. And even though I could convert the enabled/disabled options to a multi selection list in the new settings (but breaking compatibility with non-leia builds), I would run in the exact same problem with some other settings that are related to almost al channels, such as bitrate configurations.

@yol
Copy link
Member

yol commented Dec 19, 2018

@basrieter Status?

@basrieter
Copy link
Contributor Author

After some discussion on Slack there was the viewpoint that an add-on with +100 setttings was just not designed properly. So I kinda gave up and changed my add-on to do some settings from the context menu instead of via the add-on settings.

I did test this PR and it works fine, but nobody knew exactly what the impact would be on other parts of the code.

@yol
Copy link
Member

yol commented Dec 20, 2018

So #14721 is not a valid use case? (Honest question, I can't say really)

@basrieter
Copy link
Contributor Author

Well, it is the exact same use case I had. There won't by many add-ons that work like this, but some will. But again, nobody could explain or elaborate if bumping the number of controls could cause issues. I still don't really understand why there is a limitation or what the purpose is. But again, I already worked around it.

If the skinners in the team have an opinion on this and don't mind, we could still do this and prevent more add-ons from breaking when they upgrade to Leia.

@basrieter
Copy link
Contributor Author

PS: as an example: if I wanted to disable the usage of the InputStream Adaptive add-on on a per channel basis it would require 75 settings for me. And it does not seem like a strange settings to include.

@yol
Copy link
Member

yol commented Dec 21, 2018

I'm not really qualified to comment here. You said you changed your add-on to context menu usage, did you feel that was a bad change UX-wise and you would have rather kept the settings or did that end up OK?

Either way we should reach a conclusion and either merge this or close the PR and #14721 as WONTFIX.

@basrieter
Copy link
Contributor Author

Well, the result from a UX perspective is that a user now has two locations to do settings: from the actual add-on settings and via some context menu items. For some that initially was confusing. If it had been possible to keep it in a single place, I would have probably kept it there.

As background info: I eventually moved the most common settings (those that all channels have, suc as channel selection, channel bitrate, etc) into a specific json setting file in the user_data folder and update that file if the user updates settings via the context menu. The add-on specific or specific channel settings (account info for a channel, etc) are still in the Kodi add-on settings.

@yol
Copy link
Member

yol commented Dec 21, 2018

@basrieter Can you get some more add-on devs to chime in here? I'm just trying to resolve issues for v18 but I'm afraid I can't say much here, and much less decide anything.

As background info: I eventually moved the most common settings (those that all channels have, suc as channel selection, channel bitrate, etc) into a specific json setting file in the user_data folder

To me that doesn't sound like a solution we want to recommend to add-on devs

@basrieter
Copy link
Contributor Author

To me that doesn't sound like a solution we want to recommend to add-on devs

I agree, but at the time I had to do something to make it work. Perhaps @zag2me can join the discussion? I also had a discussion with @ronie and @romanvm regarding this.

@mleczan
Copy link

mleczan commented Jan 2, 2019

I have the same issue - more than 80 options is not supported.
In my case in one category there are multiple services which can be enabled, and if so additional 10+ options pop up (hidden by default).
So in normal case scenario users see 20-30 options while there is much more hidden. However due to current limit some part is not accessible.
Category could be splitted into two categories, but it would be more a workaround than a correct design, since I'd end up with categories:

  1. Services 1
  2. Services 2
  3. Video
  4. Database
    .. all other categories

On Kodi pre v18 issue was not visible

@reddit-reaper

This comment has been minimized.

@SerpentDrago

This comment has been minimized.

@drinfernoo

This comment has been minimized.

@drinfernoo

This comment has been minimized.

@MartijnKaijser
Copy link
Member

Looking at the last people who commented and what stuff they have in their repo it makes it perfectly clear we should not even consider unofficial repositories ever again.
Please stop commenting if you are involved with anything piracy or whatever stupid wizards/builds

@xbmc xbmc locked as off-topic and limited conversation to collaborators Feb 7, 2019
@basrieter
Copy link
Contributor Author

No longer relevant.

@basrieter basrieter closed this Nov 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants