MSU-1 Support Not Working on OSX (Zelda LTTP ROMs)

[spacepony]

I’ve tried multiple methods/patches for getting MSU-1 support to work, in my case specifically with a Zelda LTTP randomizer ROM (also tried the US/J release ROMs, same result). I’ve confirmed the MSU-1 functionality works as expected on Windows 10 x64, but when I switch over to OSX all I get are game sounds with no music.

I’ve tried multiple versions to see if there was any difference (1.55, 1.58, 1.59), but have not had success in getting MSU-1 to work with any of them on OSX.

[bearoso]

I no longer have a OSX system to fix this, so I can't figure out what's wrong. You'll have to wait for someone with OSX expertise to come along.

[MichaelBuckley]

I'm a Mac developer with some experience porting emulation code to the platform, and I would be willing to take a look.

That said, the Mac code seems to be in a serious state. There are still targets to build for PPC and OS versions that Xcode no longer supports. The main Interface builder can't even be opened on 64-bit Macs. Is it important is it to preserve compilation for building on older Macs?

One big reason I ask is that Apple is expected to announce macOS 10.15 in a couple weeks (with a full release in the autimn), and not only will it drop support for 32-bit Intel, but it is rumored to completely drop support for OpenGL and other libraries that SNES9x uses on Mac (like Quicktime). If this is true, (and I have no inside knowledge) at a very minimum, the renderer will have to be rewritten, movie export will either have to be rewritten or removed, and the entire project converted to compile 64-bit.

Suffice it to say, it will be a lot easier to accomplish this I can drop support for older machines. I believe the new system requirements would be OS X 10.11 (released in 2015) on 64-bit machines with GPUs capible of using Apple's Metal API (every Mac made since mid-2012).

However, I don't want to drop support for older Macs if support for older Macs is important to the project.

[OV2]

If there are good reasons (as those issues pointed out) I have no problems if we drop support for older macs. Since the mac port is basically on life support at the moment, any active development is really appreciated.

[MichaelBuckley]

Update on this.

Apple did indeed announce macOS 10.15 at WWDC, and as expected, Snes9x does not run on it.

In the meantime, I have taken a deeper look at the Mac code, and almost all of it uses system frameworks that are now gone. I have a lot of nostalgia for the Mac port of Snes9x, and I'd like to port as much as I can over, but, realisticly speaking, I probably won't be able to finish the minor features (like netplay and music box) before 10.15 releases this autumn.

My hope is to get Snes9x into a working state before the launch of 10.15. Only after that's done will I be able to look at this particular issue.

[sbzappa]

Greetings! I spent some time debugging the MSU-1 issue on MacOSX and I think I found out what the problem is (at least on MacOS 10.14 and on current master head commit (https://github.com/snes9xgit/snes9x/commit/30e2671e08b63f3986291e61959017df41094014)

The following fixes it on my system

index c40641c..014ba1b 100644
--- a/macosx/mac-file.mm
+++ b/macosx/mac-file.mm
@@ -295,13 +295,17 @@ static void AddFolderIcon (NSURL *fref, const char *folderName)
                else
                {
                        _splitpath(Memory.ROMFilename, drive, dir, fname, ext);
-                       _makepath(filePath[index], drive, dir, fname, inExt);
+                       
+                       strcat(fname, inExt);
+                       _makepath(filePath[index], drive, dir, fname, "");
                }
        }
        else
        {
                _splitpath(Memory.ROMFilename, drive, dir, fname, ext);
-               _makepath(filePath[index], drive, dir, fname, inExt);
+               
+               strcat(fname, inExt);
+               _makepath(filePath[index], drive, dir, fname, "");
        }

        return (filePath[index]);
@@ -436,4 +440,4 @@ void S9xCloseSnapshotFile (STREAM file)
                path[index][l - 1] = 0;

        return (path[index]);
-}
\ No newline at end of file
+}

There are some issues with how _splitpath and _makepath handle extensions. The function call S9xMSU1OpenFile concatenates a ".msu" or ".msu1" extension string to the filename while these functions would instead expect extensions without a dot separator. Also, the same functions are used to append track suffixes (e.g. "-1.pcm").

[MichaelBuckley]

Thank you so much for looking into and fixing this.

I always get a little nervous when I see a call to strcat. Although I don't think an overflow would cause security issues in this case, it could cause memory corruption when opening deeply nested files. However, this isn't the only use of strcat in our code. I'm going to merge this and follow up with a change that replaces them all with strncat.

@sbzappa Would you like to submit a Merge Request so that you're credited in the repo history? If not, I'm happy to apply the fix myself.

[sbzappa]

Of course, I'd be happy to do a merge request with these changes 😄

One thing though. I had to also include and modify snes9x.cpp to compile the functions _makepath and _splitpath. In doing so, I had to exclude a big chunk of the file's code through #ifndef __MACOSX__ as functions were missing on the MacOS port.

Is the Mac build broken on current master?

[bearoso]

Yeah, I moved the splitpath functions to snes9x.cpp because they were identical for all non-windows ports. If you can suggest a better core file to put those in, I'll change it.

[MichaelBuckley]

Oops, looks like you made that change a month ago, and I haven't pulled from upstream since then.

Centaralizing them makes sense, but the Mac port of Snes9x doesn't compile or link snes9x.cpp because it doesn't have a CLI or parse config files.

Looking at the files that already exist, globals.cpp is the only file that kind of makes sense, but only if you ignore what it's for. The rest of the files all seem dedicated to some aspect of emulation, and aren't appropriate for holding these functions.

Maybe we could move them into their own file? Something like makepath.cpp?

[bearoso]

Maybe we could move them into their own file? Something like makepath.cpp?

Yeah, I though that might be appropriate, too.

[sbzappa]

Very well then. I can commit my changes to my fork of snes9x and wait for the changes to makepath to create a pull-request.

I only have my Mac machine to test on right now, so I can't really validate the other platforms, but I can make sure it works on MacOS :)

[bearoso]

Ok, I rushed and added a compat.cpp file with just those functions for now. If there's any more platform-specific, duplicated functions we can add those there.

[MichaelBuckley]

@bearoso Thank you for doing that! Sorry the Mac port is causing issues like that.

@sbzappa master is now building again.

[sbzappa]

Just created a pull-request with the fix: https://github.com/snes9xgit/snes9x/pull/663

Looks like CI was cancelled though. User is too new to use Community clusters! Please check with support!

Ahaha .. looks like I'm a newbie ;)

[MichaelBuckley]

Thanks for the pull request, and thank you @bearoso for merging it. I will try to get a signed build out with this fix and a couple others later today or tomorrow.