Refactoring to accommodate macOS-style app bundling

[sh95014]

A macOS app is a bundle (specific directory structure) that defines where things like resources are placed, and this is proving tricky to override due to the "Unix" view of the filesystem in the commonframe and sdlframe code.

Some specific suggestions:

• getResourcePath() should be pushed down from commonframe, with a new API that just lets a subclass magically provide a file path to any specified resource. In macOS, I actually can't easily give you a path where a subdirectory called resource exists, and if I don't then getResourcePath() throws an exception. (I may also just be too rusty with C++ but I don't think it's possible to override getResourcePath() in its current form?)
• SDLFrame::GetBitmap() should call an API to get a file path back instead of composing its own. Something along these lines:

diff --git a/source/frontends/sdl/sdlframe.cpp b/source/frontends/sdl/sdlframe.cpp
index 8d4974bb..57d2f7d4 100644
--- a/source/frontends/sdl/sdlframe.cpp
+++ b/source/frontends/sdl/sdlframe.cpp
@@ -197,10 +197,15 @@ namespace sa2
     return myWindow;
   }

-  void SDLFrame::GetBitmap(LPCSTR lpBitmapName, LONG cb, LPVOID lpvBits)
+  const std::string SDLFrame::GetBitmapFilename(LPCSTR lpBitmapName)
   {
     const std::string filename = getBitmapFilename(lpBitmapName);
-    const std::string path = myResourcePath + filename;
+    return myResourcePath + filename;
+  }
+
+  void SDLFrame::GetBitmap(LPCSTR lpBitmapName, LONG cb, LPVOID lpvBits)
+  {
+    const std::string path = GetBitmapFilename(lpBitmapName);

     std::shared_ptr<SDL_Surface> surface(SDL_LoadBMP(path.c_str()), SDL_FreeSurface);
     if (surface)
diff --git a/source/frontends/sdl/sdlframe.h b/source/frontends/sdl/sdlframe.h
index e6304ea3..951a51c5 100644
--- a/source/frontends/sdl/sdlframe.h
+++ b/source/frontends/sdl/sdlframe.h
@@ -24,6 +24,7 @@ namespace sa2

     void FrameRefreshStatus(int drawflags) override;
     int FrameMessageBox(LPCSTR lpText, LPCSTR lpCaption, UINT uType) override;
+    virtual const std::string GetBitmapFilename(LPCSTR lpBitmapName);
     void GetBitmap(LPCSTR lpBitmapName, LONG cb, LPVOID lpvBits) override;

     void ProcessEvents(bool &quit);

It's not a big deal, but I was just trying to minimize the #ifdefs.

Thoughts? :)

[audetto]

I need to understand a bit how you are structuring your code.

LinuxFrame has nothing to do with Linux, we could call it Non_AppleWin_Frame.

I do not want to add anything else re resources there as the version for Qt deals with resources in a completely different way.

CommonFrame as you say, could be renamed PlainUnixFrame.

Are you going to inherit from SDLFrame without reusing the renderer nor the ImGui backend?

It seems to me that we are adding a new dimension to the design: Linux vs Apple. What if tomorrow you want to adapt to MacOS the terminal version?

We could create a new interface / object: ISystem and we put all the system specific calls there.

Then, CommonFrame (and all it subclasses) must receive such an object in the constructor.

Something like this

class ISystem
{
public:
    std::string getProgramDir() = 0;
    ... getSnapshotDir() = 0;
    ... getResourceDir() = 0;
};

Then, we only have one #ifdef where such an object is created.

?

But I would like to see how this code is going to be implemented in MacOS, and how this overlaps with https://github.com/audetto/AppleWin/issues/54

[sh95014]

I'm trying to maintain both sa2 on macOS as well as an actual macOS app. So sa2 would still be installed into /usr/local/bin and pick up its resources from /usr/local/share/applewin/resource. No real Apple versus Linux difference for sa2 except for the `/proc/self/exe' thing in #54.

The difference with the macOS app is that there are NSBundle methods that help you find the resources that are included in the app bundle, but the parent class code is assuming that it can build a resource path that ends in .../resource/ which wouldn't be the case in macOS.

So the idea is to assemble these paths at the bottom of the hierarchy and let them differ by platform. The parent class would just be demanding data back given a resource file name, and let the child class figure out where to pick those up.

As you suspected, I'm subclassing SDLFrame right now without using the renderer (because I want to render to a view instead of a window) nor imgui (because I'm building native UI.)

Does that clarify the request or am I making it more confusing?

[audetto]

Ok, then we need

1) ifdef to make sa2 work as is, since you will not touch the code 2) some virtual magic to make your new Frame work

agreed?

At the moment I see 2 usages of getResourcePath, one for /resource/ and the other /bin/.

Ignoring the latter, I don't think the problem is limited to bitmaps but the same thing happens in CommonFrame::GetResource, so we need your virtual function to replace

const std::string path = myResourcePath + filename;

whereever this happens and I see at least 3 places: CommonFrame::GetResource, SDLFrame::SetApplicationIcon, SDLFrame::GetBitmap

agreed again?

[sh95014]

Right, the reason that I can work around GetResource() is by overriding the entire method from CommonFrame. I couldn't do that with GetBitmap because the BMP processing code is valuable. SetApplicationIcon I basically just #ifdef'ed out because that'd be set up from the IDE, so it turned out to be GetBitmap that was making things the most awkward.

But generally allowing the platform-specific subclasses to actually go find the resource file is I think worthwhile.

Thanks!

[audetto]

See if you like this: https://github.com/audetto/AppleWin/pull/56

[sh95014]

Looks great! I've added a comment to the PR as well. Thanks again!