[Fice]

This post is more about my ideas I had once my PR ( https://github.com/xbmc/xbmc/pull/1654) be merged in, which are related to http://forum.xbmc.org/showthread.php?tid=154474&page=3. Since I can't post there, I will do that here. It's mostly technical stuff going on in the background.

Back when I started work on the context addons stuff I thought about how to clean up the context menu in ordner to make space for new addons. Of course I forgot about it, but "git branch" just reminded me that I started work on it ( 5 LOC's, so nothing to show)

Now to my idea:

Here is as an example of the context items I get on a tv show:

• "Queue" - used sometimes
  
• "Play" - never used, i have an OK Button on my remote
  
• "Play from here" - used a lot
  
• "Episode-Information" - !never! used as i have an info button on my remote
  
• "Mark as unwatched" -rarely used
  
• "Rename" - rarely used
  
• "Remove from db" - rarely used
  
• "Update DB" - used
  
• "currently playing" - since i used it alot, i mapped it to the screen of my logitech remote, so never used right now.
  

So, there are 2 items I never use (I'm sure everyone has one or two of them).
So my idea was, when working on the context addons, to make an IGUIContextItem interface that gets implemented by the addons items (as done in this PR) and later by the core ones too.
The main benefit (that's related to the above forum thread) would be that we could just show all those core context items in the addon browser alongside the actual python ones and allow the user to disable core items he doesn't want.
(Also moving settings that specify the context menu behaviour into their respective "core" addon-settings and clear up advanced- and Guisettings - given those settings exist, not sure about that)

There are also some other benefits beside that (not directly related to that forum discussion):

1. Remove code duplication
   
   1. right now every context item has a (sometimes complicated) if() to decide if it is visible and the same if() again before execution. This duplication could be removed with such a refactor.
      
   2. The same context items are implemented differently in different sections (music vs. video). Perhabs we could unify them.
      
   
   
2. All context items would than be handled by the GUIContextMenuManager. It would then be very easy to expose a json function à la "GetVisibleContextItems". Would be nice for the andoid/ios remotes. instead of openind the context menu on the tv screen (and the need to use the annoying on screen up/down buttons) you could just select the context item you want on your touch device
   
   
   
3. This might also help in doing sth about this:
   

   "5.3 Missing context-Menu commands
   Plugins can add commands to the context menu. They can also clear all original items while adding new items. But they can't clear and add some original items back because not all are available, e.g. "Add to favourites"."

   from: http://forum.xbmc.org/showthread.php?tid=154805
   

[Martijn]

thx

[Robotica]

Play / play from here should be on top. Both your and the teammember-only solution present play is second which is strange to me.

[Fice]

When i copied that list earlier, i was playing a tv show in the background... seems the "play" context item is not showing then... But "queue" is still the first, perhabs because I already have content in my playlist?!?

Anyways, added the play item in my first post (that I never use).

But the ordering of the context item is actually not the issue here, the basic idea is some refactoring in the code, so users can disable specific context items they never/rarely use.

[jmarshall]

Note that another thing to consider is that we might wish to alter the default "select" action to "choose" which gets you yet another context menu...

One presumes that Play isn't first on the general context menu because it's just select by default anyway (i.e. bringing up the context menus you want to do something other than Play).

Note that the if() block on the GetContextButton and OnContextButton may differ as the buttons action might need to be slightly different based on the item.

I'm not sure I understand exactly what your plan is for context menu add-ons though. Do you combine the context menu items across all add-ons that match a particular item, or are they combined as submenus and configured independently? Perhaps an example where you have (say) 10 context menu items from core (implemented however you propose) supplemented by 5 context menu add-ons that each have (say) 3 entries might help to hone down how things will interact?

Cheers,
Jonathan

[Fice]

Then let me explain step by step, what I planned to do.

I made a new branch on top of the current one with some pseudo code to show it. (Had to start preparing for exams, so getting some code done that actually works, would probably take some time. But perhabs this helps understanding of what I mean)
https://github.com/Fice/xbmc/compare/con...emRefactor


The first step would be to remove the context item logic out of the Media Windows and into their own classes. GUIContextMenuManager is then responsible to decide what context items will be shown. (Done in commit #1 and #2.
This is for 1.1 and 1.2
Since you are thinking about Nested context menu I made an example on how this could work (Commit #3)
I was thinking adding a little bit more meta-template programming, so you don't have to define

Code:

getMsgId() { 15; }

or

Code:

getLabel() { g_localizeStrings.Get(14077); }

everytime. So basically all you would have to do is implement the isVisible and the execute() functions for every context menu.

Until now, all of this is only refactoring (except perhabs the optional Commit #3, where context submenus are introduced)

Once all context items are converted, the next steps can follow (in no particular order)

Extend JSON API
expose some functions of the GUIContextMenuManager to the json api (Commit 4)
This would be for 2

Extend Python API
Another step could be to also expose the functions (and perhaps IGUIContextItem) to the Scripting API.
Some python scripter could then do sth. like: (I have no idea of the python api, I hope you know what I mean)

Code:

disableCoreContextItems  //as already possible
   vec xbmc.ContextManager.GetVisibleContextItems(); //Now he has all the context items (including context item addons) that would be visible
   for(vec : i)
   {
      if(SOME_MAGIC_TO_DECIDE)
          item.addContextItem(i);  //Only re-add the ones he needs
   }

Let Addon Manager handle all core items
Let the AddonManager also handle the IGUIContextItems. that way, all core context items would show up in the AddonManager, hence the user can deactivate/activate them. Also, some core context items could have settings (as shown with the Play context item, where the user could select "Always play from start", "Always resume" or "Ask"...
How to do this: Add "contextitem.core.FOO" context items (only consisting of a addons.xml and perhabs a settings file). This is already working and those will show up in the addon browser.
Make a Factory method that create the core IGUIContextItem objects.
Both can be seen in This commit

Hope that helps? If you still need some working code, this could take some time, as i'm busy studying over the next 3-4 weeks

Note that the if() block on the GetContextButton and OnContextButton may differ as the buttons action might need to be slightly different based on the item.

Yes, you could of course still do that in the execute() function. I don't know where, but somewhere I saw a huuuuge if() in the GetContextItems function and then a completely different (not just reorderd) if just before execution. Seemed wrong to me.

Note that the if() block on the GetContextButton and OnContextButton may differ as the buttons action might need to be slightly different based on the item.

You could still make such a distinction in the execute() function

5 context menu add-ons that each have (say) 3 entries

What do you mean by that? an addon that implements the "xbmc.context" extension point several times? I thought that it's not possible for one addon to implement the same extension point several times. Haven't really checked that though. Of course that could be changed.