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

Builds from most up-to-date fork cannot load CBR files #105

Open
mikemcduffie opened this issue Oct 16, 2015 · 10 comments
Open

Builds from most up-to-date fork cannot load CBR files #105

mikemcduffie opened this issue Oct 16, 2015 · 10 comments

Comments

@mikemcduffie
Copy link

Unless I'm mistaken it looks like Onizuka89 has implemented most of the various code changes from the various forks. It builds under Xcode 7 for me (Yay!). But will load CBR files, just CBZ and raw folders.

The binaries Onizuka89 hosts at http://www.stiandrobak.com/CustomBuilds/Simple_Comic.php have the same issue for me.

No error messages and the only pertinent entry in console I can see is:

10/16/15 1:40:52.150 PM Simple Comic[1873]: Drawn

Is this a known issue?

Is there another fork that builds with Xcode 7 that would be better as a starting point?

I love Simple Comic and donated back when I found it many years ago, but arauchfuss seems to have dropped off the map. I know he was off coding 2.0, but I'd be happy just to fix a few annoyances in 1.x.

Thanks,

Mike

@mikemcduffie
Copy link
Author

I've identified that all the forks that use the 3.3 XADMaster framework fail to decode RAR/CBR. arauchfuss' last version was 2.7 as far as I can tell. I compiled the latest v3.10 from the Unarchiver site and used it as a brute force replacement in an executable from Onizuka89's latest build and it now handles RAR. But dropping the compiled frameworks in the Xcode project fails to build QuickComic with error: "Cannot find protocol declaration for 'XADArchiveParserDelegate'".

A quick review of the 3.10 headers show a lot code pulled for XADArchiveParserDelegate including the @protocol statement. Weird that Simple Comic doesn't use this, only QuickComic. The Unarchiver app doesn't use it either based on a quick review of that source code.

The iComics fork that MaddtheSane is beginning to use has no version identifier that I can determine. But it appears to be an older version as much of the XADArchiveParserDelegate code is still there.

@mikemcduffie
Copy link
Author

Curiouser is that the headers from the compiled XADMaster framework with the Info.plist identifier as 3.3 don't match the 3.3 source code headers from The Unarchiver site. Modified for Simple Comic?

@Onizuka89
Copy link

I thought I had replied to this, sorry. XADMaster is the cause as you suspect. In the newest version on my branch now there should be a description in the README.md to maintain the RAR functionality if it is lost. It tells you how to checkout a version of XADMaster where it RAR works, and to remove references to two files that didn't exist in that commit of XADMaster.

@mikemcduffie
Copy link
Author

Thanks

@arauchfuss
Copy link
Owner

Could someone pull from master and verify this is does not happen. Seems to work fine for me, but that does not mean much.

@mikemcduffie
Copy link
Author

mikemcduffie commented Feb 10, 2017

@arauchfuss

Wow, great to see you back. :)

I just pulled and built and it solves the problem for me. Just a quick test to post this note so I haven't tested extensively yet.

@mikemcduffie
Copy link
Author

mikemcduffie commented Feb 10, 2017

@arauchfuss

I must say I'm sad to see you used the same status bar implementation as the forks. I greatly prefer the media player styled popup controls from the original design. This implementation either requires disabling the status bar (and useful functions/information) or having the status bar shown in fullscreen mode ruining the comic book reading effect and wasting screen real estate.

@arauchfuss
Copy link
Owner

That progress bar is from my really old transition to a modern fullscreen view, not the branches. Once I have refactored some things that will come back.

@mikemcduffie
Copy link
Author

@arauchfuss

Thanks. After I posted, I tried to remember where I had first seen the change, but wasn't able to find the commit where you had implemented it. I definitely remembered seeing it when I tried building one of the forks to fix the CBR issue, thus the mistaken attribution.

Any chance of focusing the new pinch-to-zoom to "center" on the cursor position?

@blmatthews
Copy link

Is this fixed in the current code? I've read the comments but don't really follow them. I'm using 1.7 downloaded from the website and CBRs don't work with it, which is probably 90% of my collection. If it's fixed in the current code I can probably figure out how to build it. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants