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

mGBA: add Ubuntu script #152

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

NotToDisturb
Copy link

@NotToDisturb NotToDisturb commented Jun 22, 2024

This adds a script for mGBA, a Game Boy, Game Boy Color and Game Boy Advance emulator.

Only tested for Kubuntu 24.04.

@cobalt2727
Copy link
Owner

756de1c should prevent this from causing any issues.
Two comments:

  • $GBAUserInput in your modified update.sh should be $MGBAUserInput (also can we make the M at the front lowercase to match the project name)
  • does mGBA build with QT6 yet, or is it still on QT5?

@NotToDisturb
Copy link
Author

Oh interesting, does the DUSE_SYSTEM_LIBMGBA=OFF flag turn off mGBA completely or just forces Dolphin to use an internal one? As for the comments:

  • Will change
  • I see a mention to Qt6 in the CMakeLists.txt file but the README only mentions Qt5, why?

@cobalt2727
Copy link
Owner

I think the flag to disable mGBA completely is -DENABLE_MGBA or something.

If QT6 isn't supported yet don't worry about it - I figured if that WAS supported the script would be futureproof for longer if it was built with that over QT5.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think you forgot to take out the redundant error function definition during your testing.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really sure what it does, most of the script is copied over from melonDS and this is present there (link)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's my fault, then - it's a duplicate from functions.sh

Will remove from melonDS in a moment

@theofficialgman
Copy link
Collaborator

I do not feel there is any benefit to the inclusion of this script to the Megascript when mGBA is already available on flathub. It is just additional maintenance burden. For users that want to build this emulator from source for development purposes that is what the documentation at the emulator's official github is for.

The pre-existing emulators on the Megascript were added either:

  • before flatpak support was added
  • before the emulator was available on flathub
  • the emulator isn't available on flathub

@cobalt2727
Copy link
Owner

I've got no issue allowing it in, personally - particularly when Flathub support doesn't work correctly 100% of the time.
I imagine being able to build it from source will be futureproof for 18.04 longer than Flathub will officially support it.

More options are a good thing - and something like mGBA is probably pretty low maintenance.

We can always reduce the maintenance burden a bit by checking if the system repos have the newest release available.

@theofficialgman
Copy link
Collaborator

Maintenance burden is too high for 18.04

I am not (and have not for a year) been maintaining anything for it in mind and neither has CTCaer.
For everything in the L4T-Megascript, there is no benefit to using 18.04 over other (newer) Switchroot releases.

@cobalt2727
Copy link
Owner

Sure, 18.04 is a pain, but me hitting the merge button on this is me saying I'll maintain support for it - you don't have to. I'm still interested in keeping the scripts I built for it working on stuff that old as a convenience for anyone still using those environments.

@NotToDisturb
Copy link
Author

Removed the error function in case we want to move forward with merging, I have no preference either way.

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

Successfully merging this pull request may close these issues.

3 participants