Nightly builds (NEW)

roidy · 8 August 2020 15:57

Yes, I need the return parameter in order to return to the previous window and not the parent node of the library. What issue is it supposed to fix the commit notes are lacking to say the least

Ray · 8 August 2020 16:06

When I check the commit it seems like the “return” was added to ActivateWindow there and was lacking before.

Read the comment to some piece that was added:

github.com

xbmc/xbmc/blob/master/xbmc/windows/GUIMediaWindow.cpp#L551



  // check for network up
  if (URIUtils::IsRemote(m_vecItems->GetPath()) && !WaitForNetwork())
  {
    m_vecItems->SetPath("");
    resetHistory = true;
  }
  if (resetHistory)
  {
    m_vecItems->RemoveDiscCache(GetID());
    // only compute the history for the provided path if "return" is not defined
    // (otherwise the root level for the path will be added by default to the path history
    // and we won't be able to move back to the path we came from)
    if (!returning)
      SetHistoryForPath(m_vecItems->GetPath());
  }
}
if (message.GetParam1() != WINDOW_INVALID)
{
  // if this is the first time to this window - make sure we set the root path
  // if "return" is defined make sure we set the startDirectory to the directory we are

roidy · 8 August 2020 16:18

Yep, I can now see why this fix was created but it was merged before being adequately tested and has ended up breaking something else.

Ray · 8 August 2020 16:21

I actually never poked in that area but if I understand it correctly it should only do the history thing if “return” is not added. I don’t even no exactly what you expect with that stuff cause I am no skinner.
I am sure it can be fixed.

roidy · 8 August 2020 16:29

I don’t think that is quite correct, adding the return simply shifts the parent node of the library to the path you specify in ActiveWindow or ReplaceWindow.

For instance when going from Home to the library using:-

ActivateWindow(Videos,videodb://movies/titles/,return)
You go:-
Home -> movies/titles and then pressing back returns you directly to Home

However omitting the return you go;-
Home -> movies/titles and then pressing back takes you back to videodb//movies/ and back again takes you to the Video node instead of directly back Home

roidy · 8 August 2020 16:41

@Ray While on the subject of changes how much trouble would it be to get this included in CoreELEC

Ray · 8 August 2020 16:45

If you have a backport for Leia I think we could add it.

About the issue:
You have actually just 2 changes in code:
Following was always done but now only if no “return” is used:

github.com

xbmc/xbmc/blob/cb8b7f9b4fa0437ad184b5170f065333e44c5b51/xbmc/windows/GUIMediaWindow.cpp#L561


    m_vecItems->SetPath("");
    resetHistory = true;
  }
  if (resetHistory)
  {
    m_vecItems->RemoveDiscCache(GetID());
    // only compute the history for the provided path if "return" is not defined
    // (otherwise the root level for the path will be added by default to the path history
    // and we won't be able to move back to the path we came from)
    if (!returning)
      SetHistoryForPath(m_vecItems->GetPath());
  }
}
if (message.GetParam1() != WINDOW_INVALID)
{
  // if this is the first time to this window - make sure we set the root path
  // if "return" is defined make sure we set the startDirectory to the directory we are
  // moving to (so that we can move back to where we were onBack). If we are activating
  // the same window but with a different path, do nothing - we are simply adding to the
  // window history.
  if (message.GetParam1() != message.GetParam2())

Then you have this:

github.com

xbmc/xbmc/blob/cb8b7f9b4fa0437ad184b5170f065333e44c5b51/xbmc/windows/GUIMediaWindow.cpp#L572


    }
  }
  if (message.GetParam1() != WINDOW_INVALID)
  {
    // if this is the first time to this window - make sure we set the root path
    // if "return" is defined make sure we set the startDirectory to the directory we are
    // moving to (so that we can move back to where we were onBack). If we are activating
    // the same window but with a different path, do nothing - we are simply adding to the
    // window history.
    if (message.GetParam1() != message.GetParam2())
      m_startDirectory = returning ? dir : GetRootPath();
  }
  if (message.GetParam2() == PLUGIN_REFRESH_DELAY)
  {
    Refresh();
    SetInitialVisibility();
    RestoreControlStates();
    SetInitialVisibility();
    return true;
  }
}

Which was also used before but now only if param1 is not equal to param2.

roidy · 8 August 2020 17:02

Unfortunately I don’t have a deep enough understanding of the Kodi code base or how the window history works internally to offer up a fix. All I know is that ReplaceWindow worked as intended before that fix and now it doesn’t I’ve created an issue about this but unfortunately the original author doesn’t have the time to fix it, he even goes ask far as saying that it should never of been back ported into Leia.

How would I go about creating a backport for Leia?

Ray · 8 August 2020 17:04

All I meant is that the changes in the commit are tiny and it is probably easy to fix.

About the backport I see that you commited this Pull Request to Kodi master so I assumed you knew how to port it to Leia.

Ray · 8 August 2020 17:06

@roidy You also have a slack account in our slack. Maybe we can continue discussion there. I can look into the issue next week when I know what to test.

roidy · 8 August 2020 17:10

Do I just create a pull request with the same changes against the Leia branch?

Thanks I appreciate you taking the time to look at this

Ray · 8 August 2020 17:13

Well it would be good if you can test it before doing a PR.

roidy · 8 August 2020 17:17

I can build and test a Windows version of Kodi, that’s how I initially created the fix, but I can’t build CoreELEC… I’ve tried numerous times in the past and never managed to successfully get CoreELEC to build for me

Ray · 8 August 2020 17:46

That’s fine. If you can confirm this works on Leia as intended I can make a CE build for you and we can have it merged.

Boulder · 9 August 2020 08:19

I keep having crashing problems with the latest nightly with the N2. This time I tried to resume a movie from my library over an SMB share. The screen just dimmed but playback never started. After some time, Kodi returned to the main menu and showed me the changes of the latest installed nightly (20200731) which means it totally crashed.

I don’t see anything particularly strange in the crash log itself.
https://paste.kodi.tv/uvikukadif.kodi

relkai · 9 August 2020 09:10

Please update to the latest stable release (9.2.4), which is more up to date than the latest nightly.
If the problem is reproducible on stable please post a debug log.

Ray · 9 August 2020 09:37

@roidy I have backported your PR to CoreELEC and included it into the testbuild. I also partly reverted: https://github.com/xbmc/xbmc/commit/cb8b7f9b4fa0437ad184b5170f065333e44c5b51

This results in 2 testbuilds (ending with part1 and part2) for you. the fanartres thing should be fixed in both but test both for the original issue so we can debug it further.
https://mega.nz/folder/IxJSUQoI#KO1gy9orGOpZomwHqWd0vw

roidy · 9 August 2020 11:15

@Ray Thanks, after testing I can confirm that the fanartres fix works perfectly in both parts 1 and 2.

The ReplaceWindow bug remains in part1 but is fixed in part2.

Again thanks for taking the time to look at this.

Ray · 9 August 2020 11:25

@roidy Great I will push the patch for fanartres.
About the original issues. It is this “if”:
https://github.com/xbmc/xbmc/blob/cb8b7f9b4fa0437ad184b5170f065333e44c5b51/xbmc/windows/GUIMediaWindow.cpp#L571

Can you please test part2 build if it still solves:

roidy · 9 August 2020 12:08

@Ray Ok, after testing I don’t think it does fix the problem from that issue.

I’ve created a simple context menu addon to test with:- add-on removed (see updated version below)

From any movie in the library section bringing up the context menu and using the new option *** ActivateWindow *** simply calls

ActivateWindow(Videos,videodb://movies/actors,return)

To move you into the actors section, now the problem in the above issue is that pressing back returns you to the librarys parent node and not node you called it from. This is the exact behaviour when using your test part1.

Now using your test2 build when pressing back after using the context menu item you are returned to the home page and not the calling node. I think the initial issue is that you should be returned to calling node.