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 Wii port #73

Merged
merged 1 commit into from
May 18, 2024
Merged

Add Wii port #73

merged 1 commit into from
May 18, 2024

Conversation

mardy
Copy link
Contributor

@mardy mardy commented Mar 22, 2024

Thanks Wouter for this nice game, it's written very well and it was a pleasure to port it :-)

Please note that in order for the port to be complete, we need a 128x48 icon file (png) and an XML manifest. If you can provide them to me (at least the icon and the description text for the manifest), then I can add them to this PR.

We should submit the game to the Open Shop Channel, then.

@mardy
Copy link
Contributor Author

mardy commented Mar 24, 2024

Hi Wouter, I went on and created the manifest and icon myself. Please let me know, if they are fine.

I also added a script to make a release. For the time being, I'll put the release on my own fork and submit the app to the Open Shop Channel, but as soon as you decide to support the Wii port in this repository, I'll submit a change so that this will become the reference repo for any future release.

@sharkwouter
Copy link
Owner

This is so cool, thanks for doing this! I'll take a look at it.

Copy link
Owner

@sharkwouter sharkwouter left a comment

Choose a reason for hiding this comment

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

This is a really good start, I really like the idea of seeing OceanPop on more platforms. I had a couple of comments, which I think would be doable to resolve before merging, but let me know if you have the time to address them. The current work is pretty good and I'm surprised you only needed 1 code change.

One thing I'd like to have added if it is not too much work, would be to add an automated build for the Wii to .github/workflows/build.yml. Would that be doable?

platform/wii/icon.png Outdated Show resolved Hide resolved
Copy link
Owner

Choose a reason for hiding this comment

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

Would it be possible to build the package using CMake instead?

I would suggest using the add_custom_command function in CMake to add post build commands for running the zip command the and the file command for copying.

For the config file, I would add version as an option (using the option function) and use the configure_file function to do the substitution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mmm... While I agree that it would be cleaner, it's also a bit more inconvenient, because you need to set the version number at the time when you run cmake. I can try to do that, but it will take me some more time.

@mardy mardy marked this pull request as draft March 24, 2024 17:37
@mardy mardy force-pushed the wii-port branch 2 times, most recently from d8073d6 to 59ca6a1 Compare March 24, 2024 20:00
@mardy
Copy link
Contributor Author

mardy commented Mar 24, 2024

One thing I'd like to have added if it is not too much work, would be to add an automated build for the Wii to .github/workflows/build.yml. Would that be doable?

Done (you can see a run in my fork). I'm not familiar with github actions, though, so let me know if it could be improved. And I have no idea where the artifacts have been uploaded to, so I can't even verify that they are correct :-D

@mardy mardy marked this pull request as ready for review March 24, 2024 20:08
name: oceanpop-wii
path: |
oceanpop/oceanpop*.zip

Release:
needs: [Linux, Windows, PSP, Vita]
Copy link
Owner

Choose a reason for hiding this comment

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

Can you add Wii here? This looks good otherwise. It's working well too.

Below here on line 161 you'll also need to add wii

@sharkwouter
Copy link
Owner

Sorry for getting back to you so late. I had not seen you made the requested changes. Thanks for that!

@sharkwouter sharkwouter merged commit 2c6238b into sharkwouter:master May 18, 2024
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

2 participants