slash2009 / XWMM

XBMC Web Media Manager
Other
56 stars 20 forks source link

Big cleanup/rewrite of movies page. #44

Closed ghost closed 10 years ago

ghost commented 10 years ago

Let me start of by saying this is far from complete. It's more of a heads up on what I'm working on.

There's some new code but a lot of it is rearranging what's there to make the UI more consistent, I also need to make changes to how some of the page is structured behind the scenes to simplify the loading/saving process.

Feel free to try it out. But backup your database first, don't be surprised if it behaves like a new puppy and chews it all up, then looks up at you all innocent as if its not its fault.

un1versal commented 10 years ago

Fantastic, noted pulling as patch only for testing on a test machine, so if chews on something its all only a big bunch of nothing.

This puppy is not going anywhere near my real db. hehe

Also, on a very side note, the scrapers part of the addon seems to be a bit stale and dead, I cant even remember what its suppose to do, though it doesn't seem to do anything.

When your ready with this code, squash it all into one or two commits, to make it easier to cherry pick into other branches... I end up always doing a mess :embarrassed:

slash2009 commented 10 years ago

Very nice work, Initially, didn't do a clean job, so many thanks for the cleaning initiative. I was planning to fix the genre management issue, but when I saw that you are doing a big cleanup I stopped. Should we focus on the rewriting in new code ? Let me know.

un1versal commented 10 years ago

Just my two cents

I think a good overhaul should be done and this work goes a long way to it and fix all the things that are still broken (i.e. scraping updating) and getting it ready for repo re-inclusion. Or create a repo for XWMM which is beyond me.

un1versal commented 10 years ago

Ive marked this as pending for inclusion, would be nice to get this moving...

slash2009 commented 10 years ago

ok here is what I suggest:

un1versal commented 10 years ago

I disagree, but its your choice.

Code is merged into master and then filtered down to Gotham and or Frodo as necessary. Frodo is now fast becoming outdated generally working twowards having this included in a soon to be outdated xbmc repo as the start point is wasted efforts.

Working towards more Frodo, should indeed be put into Master. it should all go into master and then Frodo if it really must!

In any case the efforts should be to get XWMM into Gotham Repo to start so you have both time to work on this goal instead of working backwards.

In any case fyfes Work is not ready by his own comment, and when it is ready it will be merged into master.

I can merge it into master but not Gotham.

slash2009 commented 10 years ago

Your argument make a lot of sense and I have to agree with you. Sorry for creating confusion. So If I understand well, all the effort should be concentrated on porting XWMM to Gotham and merge it into the Gotham branch. Also, probably, publishing the addon when we have a stable release via the the new XBMC repo. I'm no expert in GITHUB and still learning: As some code might not be compatible between frodos and gothams XWMM, is it possible to maintain the Frodo branch without merging to master?

un1versal commented 10 years ago

Is it worth maintaining the Frodo branch separately? If so ya, just work on the Frodo branch code directly. You can do bugfixes here and there anywhere..

However managing two code fronts instead of just one it will be double work. IMO there's not enough manpower to maintain two addon with diverging code.

A word on XWMM Master vs XWMM Gotham which was a experiment I did, I bumped the addon dependencies in XWMM addon.xml and it works in XBMC Gotham still very buggy though.

However XWMM Master branch is the one that this PR will be merged in and then after XWMM Gotham will be rebased on master...

Any coding efforts should go into XWMM master so it is the HUB of development. We can do bugfixes independently in any branch still...

@fyfe cleanup this PR and polish it get it moving please when you have time.

XWMM needs to be submitted to XBMC Gotham repo When its more stable and most functions work which atm dont. < that's where all the efforts should go, which is why I say XWMM Frodo can bugger off. Do we really have time for that? Unless you dont care to have XWMM back into XBMC repo. then you have all the time you want.

slash2009 commented 10 years ago

Agreed. I'll focus on Gotham. Thanks

un1versal commented 10 years ago

er dont close this PR unless its been merged or its ready to be merged. In that case its merged and its closed automatically.. Unless you dont want this PR at all...

fyfe needs to work on this to get this PR in shape iron out bugs etc git allows the any work to be pushed and then this PR is automatically updated when so has been done, if you close it then fyfe would need to make new PR.

un1versal commented 10 years ago

@fyfe nice to see this is progressing nicely, good job, let me know when I should hit the switch.

ghost commented 10 years ago

Sorry for the lack of updates, the last couple of months have been a bit busy.

I've hit a bit of a brick wall with this work. ExtJS 3 doesn't have the best support and there's quite a few things that don't quite work or need a lot of effort to adapt them to XBMC as it's not the most standard compliant :-/

As I result I've begun some preliminary work on rewriting XWMM to use ExtJS4. fyfe/v4-rewrite@v4.0.1-alpha

un1versal commented 10 years ago

My question is what happens to this PR.

I wouldnt mind merging this v4 work into master (so pr for that then?), so this work and XWMM can go anywhere in some future, but its not really my call if this is way going forward.

Merging the V4.01 alpha into master (we would need a PR) gives everyone a chance of working on it perhaps.

@slash2009 @fyfe thoughts?

criticalfiction commented 10 years ago

So I mentioned on #51 that I was going to have a look at genres, and then I saw this PR. I get the impression genre management will be relatively messy given the limitations of the XBMC API, and I think some of the features that XWMM had may have to drop out altogether (feel free to correct me - I've only had a brief look). Like @slash2009, feel it might be better to wait for this PR before I look at anything else, (better off not duplicating efforts).

Also, agree with @fyfe on ExtJS 3 - I've never touched it before, so the lack of support makes it difficult for me, and I can totally understand the move to ExtJS 4. Just a question on that, however - given that everything needs to be migrated to use the API, and a new version of ExtJS, is this not effectively starting XWMM off from pretty much scratch? Might it be better to start off from a clean slate? The reason I ask is the source is currently fairly disjointed, and it can be a little jarring going back and forth between multiple files for a single UI action. Just a thought.

un1versal commented 10 years ago

If this was my decision, I would grab this new fufe's work and make it new master and then everyone can access source with newest code and js etc and develop from there.

Atm XWMM is not in anywhere good enough shape to get into Gotham repo and at this stage the XWMM rewrite is only gonna be XWMM 5.X.X at least.

Im more inclined atm waiting for fyfe to tell me what he wants to to, I would merge this into Gotham and then work get the new work on master and done.

Bugfixes would go into Gotham and new dev only on master.

ghost commented 10 years ago

I've had 2 main issues working with the current branch of XWMM:

I began work on the ExtJS 4 rewrite at Christmas with the intention of having something with the same functionality as the current version of XWMM completed in a couple of weeks. However mother nature decided to drop a large quantity of water on the South of the UK and everyone decided life should grind to a halt. So I've spent the last two months drafted in to help with this which has ment my day to day work and personal time has gone out the window and it's taken two and a half months to build a functional movies section.

Also there is a problem with my rewrite, XBMC's add-on rules state that "All source files must be included. No pre-compiled files or skins will be allowed". ExtJS 4 weighs in at just under 100 MB, it's possible to create a stripped down version of around 6 MB. But this goes against XBMC's add-on rules. How flexible are they with this I don't know but as an average user I would be put off by a 100+ MB add-on.

I think ExtJS 4 and a clean rewrite is the way forward. ExtJS 4 is a steep learning curve but it's the best option from a collection of bad options for what we are trying to create in my opinion. I'm not a big fan of javascript and I think a lot of the frameworks for developing apps are either over complicated or they lean towards developing for mobile platforms. But that's a rant I won't start here.

So looking forward the main features I would be looking for in XWMM as a user would be (where I refer to media I mean movies, tv shows, episodes, etc.):

I've got a basic implementation of the first two for movies on my v4-rewrite branch, if anyone wants to try it out you can download it as an XBMC add-on from here. I could do with some feedback so I know if I'm heading in the right direction.

un1versal commented 10 years ago

idk about 100 MB addons, Im pretty sure no one wants to downloads of use a addon that is bigger than xbmc.

You can provide compiled stuff as long as source is available (images and stuff is already binaries and thers hardly any sources for those also fonts are compiled not source so idk what and who is forbidden), I dont even know how others addons deal with this, jsonrpc is for the most part accepted as for anything else I have no idea and what I know is limited.

@slash2009 please comment, this is after all your addon.

my 2cents is XWMM is otherwise falling behind and definitely needs to be rethought the basis is solid anywho. Not being a dev my opinion is stricly from maintenance point of voew and usablility, for me its as easy to do the git stuff for whatever you guys want.

@fyfe I will try this source of yours... what about this PR then shall I merge into master or is this dead?

criticalfiction commented 10 years ago

I don't think using ExtJS would be a problem in of itself, so long as the addon provides all the source code. As ExtJS is a JS framework, it should be ok. However, what I don't think would be acceptable would be external pre-requisites (such as the ones on the current Alpha) - I suspect dependencies on other third-party components not bundled with the plugin would be a real problem. I imagine further down the road the streamlining would be done as part of the release/bundling process? I ask as a 100MB would almost certainly be a problem. It would definitely put me off, and in the official rules they do say:

The file size must be sane. There is no hard limit, but a 100mb skin is pushing it.

With regards to the rationale behind using ExtJS - is this to provide all browser support? XWMM is effectively just a webapp, so with the issues discussed here I'm not sure that simply writing a webapp using (smaller) JS libraries wouldn't be a better option. Just a thought.

ghost commented 10 years ago

@criticalfiction I need to make the prerequisites a little clearer, they are only required to generate the trimmed down version of ExtJS they aren't required to run the web application.

With regards to is ExtJS the best option, it is and it isn't. It does all the hard work with regard to the layout (html and css) it's just a bit restrictive in how it retrieves and holds the data. The main issue is ExtJS is written around CRUD, the XBMC API isn't. So it means you have to get a little creative with the interface between XBMC and ExtJS. What I've got at the moment is still ugly as I haven't been able to entirely get my head around how ExtJS does stuff.

@uNiversaI as far as this PR goes it doesn't break anything but it doesn't add anything either. It was mainly a starting point for cleaning up the code. If we're heading towards a re-write it's not an use and you might as well close this PR.

un1versal commented 10 years ago

@fyfe If the PR is cleaner code I can just merge it for now, then we can deal wit rewrites as they come. Even if we create a special branch for them, doesnt matter.

Any rewrites will take a long while anywho so it will likely wont be By gotham and we trying to get XWMM in xbmc repos again, or well have to look at creating own repo for easy user updates. idk how to do repos :haha.

Please rebase your PR on current master you can even squash a few to make it cleaner even on merge., we wouldn't want to break the fixes done until then.

As for discussion on rewrites and such, feel free to add a issue for it and or continue discusing it here.

@criticalfiction I agree with your view, theres no need to be clever with web apps when thers plenty of ways to get a small web interface that does what xwmm can and even more, it shouldn't be limited by restrictive things this Extjs sounds problematic already, it should be made simpler and easier to maintain and even develop further.

ghost commented 10 years ago

Ok I've created a new issue (#53) for the re-write discussion.

ghost commented 10 years ago

@uNiversaI merged with master for inclusion.

un1versal commented 10 years ago

yea thx, cleaner code at least is better in meanwhile, so this is in master and hopefully didn't break any current fixes that were in.. some testing of master would be nice too to confim it all works as expected.

Ill need some time to update the releases and changlogs and bump version again to make sure theres a history kept of changelogs.

Ya will continue rewrite discussion on issue #53

thank you guys.

un1versal commented 10 years ago

@fyfe

I hope its not too late, I merged this PR, and it created a movies 2 directory with a bunch of XWMM.something.js files inside.

Thats rather confusing that in any case two directories are required, can you have a look at it pls?

ghost commented 10 years ago

@uNiversaI Sorry about that I was doing this update in a separate folder (movies2) and forgot to move it over. I've fixed it and pushed the change. I don't know if GitHub will pick it up. I'll give it an hour and if it doesn't show up in this PR I'll create a new PR.

ghost commented 10 years ago

@uNiversaI can you re-open this PR? I think that's why GH isn't showing the new commit.

un1versal commented 10 years ago

ya i think I cant do it its beem merged and is closed auto., I one with crrection should be ok.

ghost commented 10 years ago

@uNiversaI I created a new PR #55.