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

Add Amiberry for x86 platforms as well (fixes #3937) #3938

Merged
merged 2 commits into from
Jun 28, 2024

Conversation

midwan
Copy link
Contributor

@midwan midwan commented Jun 20, 2024

Did a quick test and this seems to work, but there might be more changes you want to make.

I've only added x86-64 as the platform, as there's no 32-bit version for x86 available.
I've also taken away the platform restrictions, but you know best on how to use those (perhaps just add that x86 is permitted instead).

@cmitu
Copy link
Contributor

cmitu commented Jun 27, 2024

@midwan thanks for the PR.

I have a couple of notes on it, aside from what @joolswills may want to add:

  • don't remove the current _flags, just add the new platform (x86) to it.
  • you can modify the _get_branch_amiberry function and for the x86 platform add a different branch for building, something like
    ...
    if isPlatform "dispmanx"; then
         echo "v5.7.1"
    elif isPlatform "x86"; then
         echo "preview"
    else 
         echo "v5.7.2"
    fi
     ...

@midwan
Copy link
Contributor Author

midwan commented Jun 27, 2024

@midwan thanks for the PR.

I have a couple of notes on it, aside from what @joolswills may want to add:

  • don't remove the current _flags, just add the new platform (x86) to it.
  • you can modify the _get_branch_amiberry function and for the x86 platform add a different branch for building, something like
    ...
    if isPlatform "dispmanx"; then
         echo "v5.7.1"
    elif isPlatform "x86"; then
         echo "preview"
    else 
         echo "v5.7.2"
    fi
     ...

Thanks for the suggestions.
I've modified the PR accordingly.

We should also fix the directories, as it seems there are some outdated things:

  • There are more directories included in the amiberry repo by default. Most are optional and configurable anyway, but it would be best to exist, even if empty.
  • External libraries now live under the new plugins directory, so for example capsimg.so should be moved there.
  • The FloppyBridge library is included in the sources and is part of the Makefile/CMake, and also lives under the plugins directory, but it's not handled by this script.

@cmitu
Copy link
Contributor

cmitu commented Jun 27, 2024

I've modified the PR accordingly.

Thanks, looks fine.

We should also fix the directories, as it seems there are some outdated things:

* There are more directories included in the amiberry repo by default. Most are optional and configurable anyway, but it would be best to exist, even if empty.

Excluding plugins, which is missing, I'm not sure if we're missing much. Looking at the Pi5 release (5.7.3), we're missing cdrom, floppies,harddisks and lha, but these should be located (in RetroPie) in the amiga roms folder and we have no use for them in the installation folder.

* External libraries now live under the new `plugins` directory, so for example `capsimg.so` should be moved there.

* The FloppyBridge library is included in the sources and is part of the Makefile/CMake, and also lives under the `plugins` directory, but it's not handled by this script.

OK, I'll take a look at including the plugins folder.
Looking at the Pi5 package from https://github.com/BlitterStudio/amiberry/releases/tag/v5.7.3, I see plugins is included, but it only contains the floppybridge.so library - is capsimg.so missing on purpose there ?

@midwan
Copy link
Contributor Author

midwan commented Jun 27, 2024

Yes, the extra dirs are not needed in RetroPie, and can be configured in the amiberry.conf file to point to wherever you want. Their main purpose is the default path used for the file requesters, if you try to load something from within Amiberry's GUI.

The capsimg.so is not included in the latest packages, but that was due to an issue with the build pipeline on my end. I'll get that fixed, probably include the sources in the repo while I'm at it (it's currently a submodule). But it should be included inside the plugins directory, otherwise you won't be able to load SPS/IPF images.

So essentially, if you can fix the plugins directory missing, and optionally update the paths in amiberry.conf for the Floppies/CDROMs/HDD/Lha to point to the roms/amiga folder, I think we're good.

@cmitu
Copy link
Contributor

cmitu commented Jun 28, 2024

I think I'll add the game folders configuration and the plugins folder inclusion in a separate modification - I haven't had the chance to fully test the changes to implement the former.
If you can squash the commits I think we can merge this PR with the addition of the new (x86) platform.

@midwan
Copy link
Contributor Author

midwan commented Jun 28, 2024

Sounds good.
Isn't the squashing done at the merging time? Github has an option to Squash and merge in one step.

@joolswills joolswills merged commit 6601234 into RetroPie:master Jun 28, 2024
@joolswills
Copy link
Member

Thank you.

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.

None yet

3 participants