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

added SuperFlappyBirds scriptmodule to libretrocores #2598

Merged
merged 1 commit into from Feb 7, 2019

Conversation

IgniparousTempest
Copy link
Contributor

Super Flappy Birds is an original-ish game created by me, specifically with retropie in mind. The game and screenshots are here.

It is written as a libretro core, hence why the script is under the libretrocores folder, but it installs to the ports folder on retropie. This pattern seems odd, but it is what Dinothawr does, see here.

Copy link
Member

@hhromic hhromic left a comment

Choose a reason for hiding this comment

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

Good to see original creators! I left some minor comments, good luck!
One extra note: I'm not sure about the $romdir/ports/superflappybirds/ last parameter in your addPort, are you using any content stored there?

scriptmodules/libretrocores/lr-superflappybirds.sh Outdated Show resolved Hide resolved
scriptmodules/libretrocores/lr-superflappybirds.sh Outdated Show resolved Hide resolved
scriptmodules/libretrocores/lr-superflappybirds.sh Outdated Show resolved Hide resolved
@IgniparousTempest
Copy link
Contributor Author

IgniparousTempest commented Feb 5, 2019

@hhromic I have fixed the library name as you suggested.

The $romdir/ports/superflappybirds/ is being used to store high scores.

I wasn't sure how else to do it, as RETRO_ENVIRONMENT_GET_SAVE_DIRECTORY was returning / as the save directory. I didn't want to store it in the same directory as the game files either, because if the user updated the game, they would wipe the high score data. Is there a better place to store this?

@hhromic
Copy link
Member

hhromic commented Feb 5, 2019

@IgniparousTempest thanks for the changes. Just for your information, the library naming convention is more from the libretro project, than from RetroPie :)

Regarding the high scores saving, I think most cores use the system dir for this purpose and not the save one. The latter I understand is for game statesaves/battery backups. So maybe you want to explore that in the libretro API. In RetroPie, the system dir is mapped to $biosdir, usually ~/RetroPie/BIOS.

#define RETRO_ENVIRONMENT_GET_SYSTEM_DIRECTORY 9
                                           /* const char ** --
                                            * Returns the "system" directory of the frontend.
                                            * This directory can be used to store system specific 
                                            * content such as BIOSes, configuration data, etc.
                                            * The returned value can be NULL.
                                            * If so, no such directory is defined,
                                            * and it's up to the implementation to find a suitable directory.
                                            *
                                            * NOTE: Some cores used this folder also for "save" data such as 
                                            * memory cards, etc, for lack of a better place to put it.
                                            * This is now discouraged, and if possible, cores should try to 
                                            * use the new GET_SAVE_DIRECTORY.
                                            */

You mention that your game has content (game files), but your scriptmodule doesn't seem to be installing any of that. Is the user supposed to copy the game content manually herself? You should document this in the rp_module_help variable. (clarified)

I saw that your project has the game files in github already, maybe you can sort out a way to automatically download these into the $romdir. The idea is that the user, after installing your game, shouldn't need to do any extra steps but to launch it from EmulationStation. (clarified)

@hhromic
Copy link
Member

hhromic commented Feb 5, 2019

Also, after your changes and when you feel your scriptmodule is ready to be merged, you should squash your commits in this PR into a single one. Do not open a new PR, just force-push.

@hhromic
Copy link
Member

hhromic commented Feb 5, 2019

Ah, my bad! Just realised you are copying the resources folder in the install function. This is being copied to /opt/retropie/libretrocores/lr-superflappybirds.
So there is no real use for a directory in the ports system as everything in your game is included in the game installation folder. You just need the standard shell script in $romdir/ports.

Perhaps you can simply leave out the last parameter of addPort and should be good to go.

@IgniparousTempest
Copy link
Contributor Author

@hhromic, I have changed my highscore file to use RETRO_ENVIRONMENT_GET_SYSTEM_DIRECTORY and have applied your other suggestions.

Thanks so much for all the help, I really appreciate it.

@hhromic
Copy link
Member

hhromic commented Feb 5, 2019

@IgniparousTempest looking good now! Happy to help you.

Having your highscores in the $biosdir ensures users don't lose them when uninstall/re-installing, and keeps the $romdir clean as well.
Don't stop improving your game, is a nice hobby!

All that rests now is for the boss to review and merge your scriptmodule and you are set!

@hhromic
Copy link
Member

hhromic commented Feb 5, 2019

It's-a me again :) I think you also should take a look to the dependencies of your game. For instance, your build uses cmake, so you should add at least the following depends function (usually before the sources function):

function depends_lr-superflappybirds() {
    getDepends cmake
}

If your game needs any other library to compile, you should add it there too. Cheers!

@IgniparousTempest
Copy link
Contributor Author

@hhromic, I updated the url and added the dependency.

I didn't add cmake, because I thought it came standard on Raspbian. The game's engine/framework was all written by me, so it doesn't have any other dependencies.

@hhromic
Copy link
Member

hhromic commented Feb 5, 2019

The getDepends function makes sure cmake is installed before trying to build your game, if a particular system already has it then it will just move on. The cmake dependency is present in other scriptmodules as well, as I'm not sure it comes pre-installed on Raspbian Lite (used for RetroPie base image).

Your PR is looking good to me. Just noticed that you left a trailing . (period) in the wiki URL. Also don't forget to squash your commits. I will give your scriptmodule (and your game) a real try tomorrow on my raspberry pi. Cheers and good work!

@hhromic
Copy link
Member

hhromic commented Feb 6, 2019

@IgniparousTempest I tested your scriptmoduel now on my RPI3. Works flawlessly.
Also, I can confirm the entertaining value of your game as I spent a solid 15 minutes with it just for testing!

One final (for real!) suggestion for you to include your license and documentation in the installed files (like many other cores do), by adding this to your install function:

function install_lr-superflappybirds() {
    md_ret_files=(
        'superflappybirds_libretro.so'
        'resources'
        'LICENSE'
        'README.md'
    )
}

And also check the potential naming licensing issues mentioned in the forum.
@joolswills apart from that, this scriptmodule is ready to go IMHO.

Super Flappy Birds is an original-ish game created by me, specifically with retropie in mind. The game and screenshots are [here](https://github.com/IgniparousTempest/libretro-superflappybirds).

It is written as a libretro core, hence why the script is under the `libretrocores` folder, but it installs to the ports folder on retropie. This pattern seems odd, but it is what Dinothawr does, see [here](https://github.com/RetroPie/RetroPie-Setup/blob/master/scriptmodules/libretrocores/lr-dinothawr.sh).
@IgniparousTempest
Copy link
Contributor Author

IgniparousTempest commented Feb 6, 2019

@hhromic I am glad that you enjoyed it. 😁

I added the licence and the readme as you suggested, that seems like a good idea.

As for the naming issue, I have looked into it and I don't think it is an issue:

  1. The article mentioned was from 2014 and I can't find any reference to takedown notices after this point. TechCrunch
  2. Ultimate Arcade, Inc.'s trademark is now dead, so I doubt they'll be an issue. These were the people referenced in the TechCrunch article that were issuing the takedown notices. Trademark Electronic Search System
  3. There are a plethora of Flappy X games on the app store, a few released after 2014. Play Store

I think point 2 is probably the most important there.

Ulimate Arcade, Inc trademark

  1. Goto TESS
  2. Select "Basic Word Mark Search (New User)"
  3. Enter "Flappy" as the Search Term.
  4. Press "Submit Query"
  5. It is serial number "86223697"

I couldn't figure out how to permanent link to that page, but you'll see that the trademark is dead. It was abandoned shortly after being filed.

@joolswills joolswills merged commit f3e8280 into RetroPie:master Feb 7, 2019
@joolswills
Copy link
Member

Thanks.

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