Project

General

Profile

Actions

Bug #91

closed

[PATCH] makes winxml take same parameters as xbmc

Added by nuka1195 over 13 years ago. Updated over 13 years ago.

Status:
Closed
Priority:
High
Assignee:
arnova
Category:
Other (un-categorized)
Target version:
Start date:
Due date:
% Done:

0%

Resolution:
fixed
Affected Version:

Description

brings inline winxml with xbmc


Files

xbmc4xbox-winxml.diff (12.1 KB) xbmc4xbox-winxml.diff nuka1195, 19/08/2010 02:23 PM
skininfo.diff (799 Bytes) skininfo.diff nuka1195, 26/08/2010 09:23 PM
Actions #1

Updated by arnova over 13 years ago

  • Status changed from New to Closed
  • Resolution set to fixed

r30547. Cheers!

Actions #2

Updated by arnova over 13 years ago

Just curious: what's the benefit of this for xbmc4xbox?

Actions #3

Updated by nuka1195 over 13 years ago

allows scripts to work on both platforms without changes. i'll test more, ticket 91 should not happen.

Actions #4

Updated by nuka1195 over 13 years ago

i mean ticket 99

Actions #5

Updated by arnova over 13 years ago

  • Status changed from Closed to Feedback
  • Resolution deleted (fixed)
Actions #6

Updated by arnova over 13 years ago

I've reverted this in r30567 since it caused major issues with several scripts. I could reproduce it by running Navi-X as a script which totally crashed my Xbox.

Actions #7

Updated by nuka1195 over 13 years ago

weird that it crashes, not sure where the script is, could you post the call that loads the window.

i think a better solution maybe to have this patch modified to also include the check for skin.xml first.

even better though is to have the scripts updated.

if these are scripts you host post a link and i will update the files.

don't take this as a threat, it's not :), but my scripts updated will not work unless this patch is in. that includes amt and any that use a gui, like amt lites showtimes, home theater experience...

i just don't see the need to keep two versions.

Actions #8

Updated by arnova over 13 years ago

Replying to [comment:7 nuka1195]:

weird that it crashes, not sure where the script is, could you post the call that loads the window.

i think a better solution maybe to have this patch modified to also include the check for skin.xml first.

even better though is to have the scripts updated.

if these are scripts you host post a link and i will update the files.

don't take this as a threat, it's not :), but my scripts updated will not work unless this patch is in. that includes amt and any that use a gui, like amt lites showtimes, home theater experience...

i just don't see the need to keep two versions.

What do you mean by "could you post the call that loads the window." ? Of course the scripts should be updated but on the other hand, it should not crash XBMC and it would be huge win if we could keep backwards compatibility (which I think is possible by just detecting the type of argument used). I surely understand that you don't want to create seperate versions for XBMC4Xbox so I don't take it as a threat, no worries there. Maybe you could change the patch with your (and mine) suggestions? I surely like to see this sorted before we release 3.0

Actions #9

Updated by nuka1195 over 13 years ago

could you point me to the script that is crashing xbmc4xbox, so i caqn test the exact issue.

yes i think a simple check for skin.xml first and make sure the 4th passed variable is a string or a bool should work.

but i agree, crashing xbmc is weird, just wondering if it's this change or not.

Actions #10

Updated by arnova over 13 years ago

Use Navi-X as a script (NOT plugin) from here http://code.google.com/p/navi-x/downloads/list. It crashes right after executing it.

Actions #11

Updated by arnova over 13 years ago

Isn't just "skinInfo.Load(basePath);" missing in the new code?

Actions #12

Updated by arnova over 13 years ago

  • Status changed from Feedback to In Progress

Found the problem. It's the if (*res) check in skininfo (totally other location. RESOLUTION is a struct and is not initialized properly everywhere (to INVALID).

Actions #13

Updated by nuka1195 over 13 years ago

haha, i just found it and was going to post a patch :p, i will anyways.

Actions #14

Updated by arnova over 13 years ago

Fixed properly in r30570.

@Nuka1195: Please inspect what I did in winxml & winxmldialog. Not entirely sure the legacy stuff is correct.

Actions #15

Updated by nuka1195 over 13 years ago

@arnova, you may want to check my patch, it only affected the one file, it was something i missed from xbmc. of course it's based on your pre revert.

about the legacy stuff, i'll see what i can find.

Actions #16

Updated by arnova over 13 years ago

Replying to [comment:15 nuka1195]:

@arnova, you may want to check my patch, it only affected the one file, it was something i missed from xbmc. of course it's based on your pre revert.

about the legacy stuff, i'll see what i can find.

Maybe I'm missing something but I don't really see why this would help in case *res is not properly initialized? My changes specifically init RESOLUTION variables to INVALID everywhere now, so we can check that in GetSkinPath. Mind elaborating?

Actions #17

Updated by arnova over 13 years ago

And if would use your patch, shouldn't it be:
RESOLUTION tempRes = INVALID;
if (*res != INVALID)
res = &tempRes;

?

Actions #18

Updated by nuka1195 over 13 years ago

no my patch is what i missed directly from xbmc. it must rely on not initializing res, like you did.

Actions #19

Updated by arnova over 13 years ago

Ok, so we might as well leave it the way I've modified it (?)

Actions #20

Updated by nuka1195 over 13 years ago

sure, they are different enough from xbmc, that any changes in xbmc won't directly patch anyways.

but you should mention it to jmarshall or fix it in xbmc.

Actions #21

Updated by nuka1195 over 13 years ago

i mean fix the not initializing res.

Actions #22

Updated by arnova over 13 years ago

  • Status changed from In Progress to Closed
  • Resolution set to fixed
Actions

Also available in: Atom PDF