Closed davis-junior closed 1 year ago
Hi @davis-junior - thanks for the (first) PR! I'd been considering adding nNASOS support for a while but never got around to it. I've had a quick look at the changes and they look good. I'll review them in more detail later today.
Was it easy enough to find where you needed to make the changes in the source code? The plugin has grown well beyond the design for the v2 refactor, especially with the multi-disc support, so there's some redundancy and overly verbose function names in there. It could probably use some more comments too.
I took the approach of basing the new extractor off of your existing ones, mostly using text search and built-in code definition searching like F12 as browsing the code. Though it didn't take long to at least have an elementary understanding of how some of these components work and integrate together.
By no means do I have a full understanding of the code base, not even close, but this is more a result of literally only spending a few hours in it, specifcially just to make this change.
Comments and extra docs, especially on classes and methods are always good. But I felt it was pretty intuitive. I'm also more of a Python/Java/RPG (IBM RPG) programmer, so I'm not super versed in C#.
But yes, just let me know if you notice anything in this commit that could get revamped, organized better, etc and I'll update it.
Thanks for the feedback. Getting to work on those changes now. The behavior of Path.GetExtension()
only parsing from the last period is probably the reason I couldn't get the "intermediate" handling of partial extractions to work. Though I never attached a debugger to see what was really happening. Now that I know how to remote debug a remote machine with Visual Studio, along with this new insight, I should be able to get it to work.
Well, I got hung up in Visual Studio trying to step into async code. I just couldn't get it to step in. Maybe it's just a remote debugger issue. I do see a glimpse of the Task pop up in the Task view, but I know of no way to simply break on all new tasks. If I remember right, native code debuggers like x64dbg can break on all new threads. So, I'd assume it'd be possible to break on all newly created .NET/CLR threads/tasks in Visual Studio debugger.
Nonetheless, I think all the those changes are implemented, and I tested caching a few games to make sure it still worked.
Thanks for taking a look at those few comments, I'll review again some time tonight.
In terms of attaching the debugger, I create a debug build in Visual Studio which is copied to the plugin folder of a local LaunchBox installation as part of the build process. Then next time I start LaunchBox it'll prompt to attach a debugger when the plugin first loads. At this point I won't attach it (unless I'm debugging something in the UI or launch setup), and let it run as usual.
When a game is launched, LaunchBox calls the extraction process (what it thinks is 7-zip, but is the cache manager process) and there will be a second prompt to attach a debugger. At this point I'll attach the debugger, and can step through the code, use breakpoints, etc. After the process finishes, there'll be a third prompt to attach a debugger, as LaunchBox calls the extraction process again, this time to get the file listing of an archive.
Hope that makes sense. I don't know how well this translates to debugging on a remote machine, but might be something to try.
I found some workarounds for remote debugging. Rather than stepping into async code, you can set a breakpoint on the method called in the Task (and just use the Call Stack to see the context of the async call if needed). And since Debugger.Launch()
does not have any effect on the remote debugger, a simple message box will block while you attach the process manually. I added a check for the REMOTEDEBUG compiler constant, so if anyone else needs to remote debug, maybe it will save them some time figuring out a workaround.
Thanks again for the PR and the work you've put in, it's much appreciated.
I'll add a contributions list with your handle to the readme if that suits you? Do you have a LaunchBox forum handle I can tag for the next release?
I'm not sure when the next release will be, probably in the next 1-2 weeks. There's a few things I want to consolidate across the various extractor components, and maybe shift things to the ThirdParty
folder so they're all in one place.
Perfect! Also, no need for the credit, but if you insist, just use my full name or GitHub as I'm not active on the forum.
Also, I realize the main loop doesn't need to be specialized to nNASOS for checking if only one-pass (or more) of extraction has been done (aka partial extraction) as opposed to all passes (full extraction), but I wasn't sure how to generalize it across all multi-pass extractors. Maybe just a simple check in the main loop to see if at least one of the files in the cache path matches one of the expected emulator file extensions would be a better generalized solution here (if I'm not reinventing the wheel and this hasn't already been implemented).
I have not looked into any of the LaunchBox API or user settings, but maybe there is a way to tell LaunchBox to not recursively search plugin folders for DLLs. Then the executable and any DLLs could still be placed within the plugin's folder as opposed to ThirdParty/.
And as an alternative, though getting a little hacky where this is probably more work than it's worth, but I guess some kind of permanent DLL injection could be used to integrate the .dll directly into the PE file, then there is simply a standalone executable. Then the standalone .exe could just be hosted again somewhere for quick download.
nNASOS is a spin-off of the original NASOS tool made by a Redump developer a few years ago. It is meant to compress in a lossless way GameCube and Wii .iso files into .iso.dec. nNASOS is available here: https://archive.org/details/nNASOS1.8. I had issues placing the executable in the plugin's extractors folder because it seems LaunchBox was trying to load one of the utility's DLL's as a plugin. So, I ended up placing it in LaunchBox/ThirdParty/nNASOS1.8. I also attempted to perform an intermediate step in the case a .iso.dec was already extracted/copied to the cache, which would then only need worked by nNASOS rather than the .7z being copied and extracted over Samba. However, as it stands, it doesn't perform this intermediate step, and instead it will be considered not cached from both launch and batch cache, which is better than manually deleting first. Anyhow, I batch cached about 10 games so far successfully with this commit.