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

GNUFrame is still needed? #37

Open
audetto opened this issue May 28, 2023 · 9 comments
Open

GNUFrame is still needed? #37

audetto opened this issue May 28, 2023 · 9 comments
Labels
refactor Refactoring

Comments

@audetto
Copy link

audetto commented May 28, 2023

Hi,

I did introduce GNUFrame to ease the native mac os build.

audetto@f03c09d

I had a quick look at this repo and I am not sure it is (ever was) still needed?

Can I merge CommonFrame and GNUFrame back together?

@audetto
Copy link
Author

audetto commented May 28, 2023

Actually forget this. It is still needed by MarianiFrame.
No prob.

@audetto audetto closed this as completed May 28, 2023
@audetto
Copy link
Author

audetto commented May 28, 2023

I wanted to add an argument to the constructor of CommonFrame, but I am getting confused with multiple / virtual inheritance... let me revisit it.

@sh95014
Copy link
Owner

sh95014 commented May 28, 2023

@audetto I'm not building GNUFrame and SDLRendererFrame at all.

Anyway, I would suggest revisiting the entire hierarchy of "frames". Maybe like this:

classDiagram
    FrameBase <|-- CommonFrame
    CommonFrame <|-- MarianiFrame
    CommonFrame <|-- LinuxFrame
    LinuxFrame <|-- SDLFrame
    class CommonFrame {
        Lifecycle (from SDLFrame)
        Execute (from SDLFrame)
        FrameBuffer (from LinuxFrame)
    }
    class LinuxFrame {
        Directories (from GNUFrame)
    }
    class SDLFrame {
        Bitmaps
        Key/mouse/drag/drop handling
        Window title, icon
    }
    class MarianiFrame {
        Directories
        Bitmaps
        Key/mouse/drag/drop handling
    }
Loading

I can try refactoring it as above and sending you a PR if you like. Because right now MarianiFrame inherits from SDLFrame and then comments out half of it because it's not SDL-based.

@sh95014 sh95014 reopened this May 28, 2023
@sh95014 sh95014 added the refactor Refactoring label May 28, 2023
@audetto
Copy link
Author

audetto commented May 29, 2023

The only thing I need to be careful about is Lifecycle and Execute as I still want to handle libretro and the ncurses frontend (which could be improved anyway).

Are you able to tell me exactly which methods (from SDL Frame) you use?

@sh95014
Copy link
Owner

sh95014 commented May 29, 2023

Are you able to tell me exactly which methods (from SDL Frame) you use?

Yup. Here's the diff that reparents MarianiFrame on CommonFrame instead. Note that I couldn't figure out how to get SDLRendererFrame to invoke the CommonFrame(options) constructor so I had to insert a dummy default constructor CommonFrame() that doesn't read the program options.

e869286

(This change would bring the #ifdef MARIANIs down to just eight, which is pretty exciting...)

@audetto
Copy link
Author

audetto commented May 29, 2023

Thank you. I will have a look. Your issue sounds very much the same as mine, so I hope we can fix it.

@sh95014 sh95014 changed the title GNUFrame is still still needed? GNUFrame is still needed? Dec 13, 2023
@sh95014
Copy link
Owner

sh95014 commented Dec 13, 2023

https://github.com/sh95014/AppleWin/tree/frame-refactor-2023 removes MarianiFrame's reliance on SDLFrame, and it hasn't been reliant on GNUFrame anyway, so you might want to use audetto#124 to just get rid of GNUFrame entirely?

@audetto
Copy link
Author

audetto commented Dec 24, 2023

I have merged audetto#124

I guess I can sort out GNUFrame now.

@sh95014
Copy link
Owner

sh95014 commented Dec 24, 2023

With your latest changes, I was able to remove all downstream #ifdefs except one, so you really got AppleWin building pretty cleanly for Linux and macOS at this point. Thanks!

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

No branches or pull requests

2 participants