firebelley / godot-export

Automatically exports your Godot games.
MIT License
486 stars 59 forks source link

Update 4.0.0 #78

Closed firebelley closed 2 years ago

firebelley commented 2 years ago

This action has become bloated in an effort to support many disparate use cases. Now that there are more actions available, this PR strips down the feature set and recommends other actions in conjunction with this one. Now, godot-export is solely responsible for exporting the builds and optionally archiving them.

To test, see the new examples and documentation here: https://github.com/firebelley/godot-export/tree/update-4

manuel3108 commented 2 years ago

Hey @firebelley

great to see you back! This action seemed quite abandoned when I started using it last week or so. That's why I decided to create a new action on my own, with similar intentions than what you described in the issue description.

I don't have time to try your action right now, but here are my key takeaways / ideas / question i had while recreating this.

  1. Make the configuration more accessible. Do not let the users provide the full urls of the downloadable urls in the config and make it simple, by only letting them choose the right version. But we need to make sure that we also support the mono version. See my config example below
  2. Make sure to download the godot executable and export templates from the GitHub releases. There are two reasons for this: Tuxfamily are often quite slow, and second: Downloading GitHub files in actions is incredibly fast. In my tests last week, sometimes the pipeline managed to download both files ~600 MB in under five seconds.
  3. In the action, provide an output variable, with a list of the generated files (most likely zip's). This will make it easy to use the results in action like this https://github.com/ncipollo/release-action#example
  4. Is it possible to cache the downloaded files with the GitHub Actions cache https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows to speed up the build even more? (PS: I have no clue, just an idea)
  5. Optional: Provide a nice way to work with the android keystore files. Normally those keystore files should never be commited to a repository, and never be commited with the appropriate password. Here is an idea i recently played around with, after finding this great article: https://stefma.medium.com/how-to-store-a-android-keystore-safely-on-github-actions-f0cef9413784
  6. Provide example projects in the repo, for easy testing. We might also use this for some ci
  7. I kinda liked the option you provided, that would only create a zip file, if there is more than one file in the export directory. Because i.e. personally I would not like that my generated APK gets zipped, because it will only have one file for sure. But with the options you provide here, I should be able to not zip the files, and to that in a later step of the action myself.

This is what my entire config looks like (taken from https://github.com/manuel3108/github-godot-export/blob/main/.github/workflows/example-mono.yml)

name: Example 2 (Mono)

on:
    push:
        branches:
            - main
jobs:
    export_game:
        runs-on: ubuntu-latest
        name: Export game
        steps:
            - name: checkout
              uses: actions/checkout@v2.0.0

            # Read the version from a txt file so we can also use this version in our application
            - name: Read version.txt
              id: version
              uses: juliangruber/read-file-action@v1
              with:
                  path: ./examples/godot_3.4.4_mono/version.txt

            # Call the export godot action to build and zip the executables for the different targets
            # Provides a comma separated list of generated files, used to i.e. generate a new release
            - name: export godot
              id: godot
              uses: manuel3108/github-godot-export@main
              with:
                  godot_version: 3.4.4-stable
                  use_mono: true
                  base_dir: ./examples/godot_3.4.4_mono/

            # Use the generated artifacts and the version read from a file to create a new github release
            - name: create release
              uses: ncipollo/release-action@v1
              with:
                  artifacts: ${{ steps.godot.outputs.artifacts }}
                  generateReleaseNotes: true
                  tag: v${{ steps.version.outputs.content }}
                  token: ${{ secrets.GITHUB_TOKEN }}

This is also the full link to my repo: https://github.com/manuel3108/github-godot-export. The code is not cleaned up, but this file https://github.com/manuel3108/github-godot-export/blob/main/src/index.js might give you an idea on my opinions about the points 1 and 2.

In order to combine our efforts, I have just marked my repo as an archive. I'm going to try to test your version here in the upcoming days.

What are your opinions on the points mentioned above?

firebelley commented 2 years ago

@manuel3108 thanks for the detailed feedback! Let me respond to each of your points:

  1. I agree that the configuration should be more straightforward than it is now. Ideally we'd simply be able to detect which version of Godot the project is being built with, but we don't have that info. There are a couple of things about providing the url that I think are nice compared to simply specifying the version number. You can supply a link to your own custom build of the engine and a link to custom templates. Additionally, it's very easy to just put a link to either standard or mono here with no configuration. Open to more suggestions here, would be happy to address this sometime after this PR gets merged since this functionality hasn't changed. Perhaps we can use some kind of smart detection to see if the value provided is a simple version number or download link. So we can support both cases.
  2. This is a good idea! I honestly can't say I've noticed any slowness downloading Godot from Tuxfamily, but now that you mention it I'm sure it can be sped up. Again, I'd propose that this change be made after the merge of this PR since the download functionality hasn't changed in this PR.
  3. I would love to do this, but the last time I looked it wasn't trivial to export an array of data from a step. Has that changed? I suppose worst-case is we could output a JSON string or comma-delimited array. Open to ideas here!
  4. I think so yes, though I am not sure how much benefit there would be. If switching to GitHub downloads for the Godot builds only takes a couple seconds then I'm not sure it's worth implementing. Let me know if you think otherwise.
  5. Admittedly I know next to nothing about packaging for Android, would appreciate community help making this smoother. Would using GitHub secrets for keystore data be useful here? I think the big question is how to get the sensitive data into the action so it can be used for the export. Perhaps those could be optional inputs to this action?
  6. Good idea, I already have a test repo built for this - probably could just migrate that over here!
  7. I am not sure how far back to pare this action. I do think some zipping utilities are nice to include, but I am wary of getting super specific with the configuration options. I think ideally this action would run the export from Godot with minimal processing and then spit out the location of the built files so that individuals can do whatever they like in the next steps of their action.

Those are my thoughts, please feel free to push back on anything. Also, I would love your opinion on what changes should be included in this PR and what changes can be done in 4.x.

Thanks again for your collaboration on this!

manuel3108 commented 2 years ago

@firebelley Sorry for my late feedback, but here you go:

  1. Ok, I didn't know having custom engine builds was really a thing, but if that's the case, then you have chosen the right way for sure. But I would like the idea for the future, that you have the possibility to add only the version to those two properties and that we then generate the full URL in the background. But for sure, we can do that later on.
  2. Yeah, I kinda agree. But I think it would be as simple as updating the README with the github url, that have the following pattern: https://github.com/godotengine/godot/releases/download/3.4.4-stable/Godot_v3.4.4-stable_mono_linux_headless_64.zip
    But sure, we can do this later.
  3. Yeah, sadly, that's still a thing. We could either export a comma separated string. But I kinda also like the idea you mentioned in the 4.0 README. I would also be very happy if we would, just, export this path from the action $HOME/.local/share/godot/dist/*. This would already remove a step from the action file, and be less prone to errors, if it becomes necessary that we change this path in the future.
  4. Yeah, I'm not sure also. I think we can leave that out for now.
  5. Yeah, the link I pasted above uses the GitHub secrets for this. But as said in other places, we could add this as a 4.x. So need to wait for this.
  6. Great, let's do this. But maybe it's better to leave that in a separate repo, so that we can also create release, without spamming the action releases? But I think linking that repo in the README would be a great idea.
  7. Yeah, that's fine for me. I just had a though that the APK file primary source will be the play store nevertheless, so zipping the file shouldn't be a problem at this stage for me.

As far as I can see, all the changes we discussed would not be breaking at all, so I don't think that there are things we need to do before merging.

Things I personally would like to be done before merging this branch:

  1. Switch to GitHub downloads in the README for shorter download times.
  2. Link your test repo in the README
  3. Export the path mentioned in 3 from the action by default

For 3. this would probably be the layout of the steps after the change (changed lines in double stars):

    - name: export game
      # Use latest version (see releases for all versions)
      uses: firebelley/godot-export@update-4
      **id: godot**
      with:
        # Defining all the required inputs
        # I used the mono version of Godot in this example
        godot_executable_download_url: https://github.com/godotengine/godot/releases/download/3.4.4-stable/Godot_v3.4.4-stable_mono_linux_headless_64.zip
        godot_export_templates_download_url: https://github.com/godotengine/godot/releases/download/3.4.4-stable/Godot_v3.4.4-stable_mono_export_templates.tpz
        relative_project_path: ./
        archive_output: true

    - name: create release
      # This release action has worked well for me. However, you can most likely use any release action of your choosing.
      # https://github.com/softprops/action-gh-release
      uses: softprops/action-gh-release@v0.1.14
      with:
        token: ${{ secrets.GITHUB_TOKEN }}
        generate_release_notes: true
        tag_name: v${{ steps.version.outputs.content }}
        **files: ${{ steps.godot.outputs.distFolder}}**

And for that to work, we would need to add a line similar to this core.setOutput('files', distFolder); somewhere in the code.

Also, as a further note: I tested the 4.0 export this morning by specifying uses: firebelley/godot-export@update-4 and everything worked as expected. The only thing that I'm still missing, is that the update of the windows icon is working. But maybe that's a configuration issue on my side.

firebelley commented 2 years ago

@manuel3108 Thanks! I appreciate your feedback as always. I agree with all your points above, particularly your suggestion to export the dist folder location. I like your suggestions for what you'd like to see done before merging, so I'm going to go ahead and work on those three things. The other things, since they aren't breaking changes, can come in 4.x versions.

Also, thanks for testing! Glad to hear everything works except for the windows icon update. That seems to be finicky, I'll do some double checking on my end for that.

Thanks again, I'll get to work on those changes before merging. Let me know if any other ideas come to mind!

manuel3108 commented 2 years ago

Sure, no problem @firebelley

Maybe it's also a misconfiguration on my end? Which setting am i expected to set for this windows icon thing? Because locally it's also not working for me (windows host). I will also do some research in that direction!

firebelley commented 2 years ago

@manuel3108 Apologies for the delay on this, I've addressed all your feedback. There are now 2 outputs, 1 for raw build files and one for archived build files, as well as a directory and documentation for example projects that can be built by forking the repository. I also went through and updated the documentation quite substantially.

Regarding windows icons, I was able to use this feature without issue. You should just need to set the wine_path as well as specify an .ico file for your export. Perhaps you need to clear the icon cache as suggested in the docs? Sometimes Windows won't display the most up-to-date icon. https://docs.godotengine.org/en/stable/tutorials/export/changing_application_icon_for_windows.html#testing-the-result

Let me know if you have any other feedback before I merge!

manuel3108 commented 2 years ago

Hey, also sorry for the delay on my side. I finally managed to try out the new version, and it seems to work seamlessly. Also I tried the archive export variable, which also worked out great.

As for the icon. Usually the bug sits in front of the computer, we would say in geramny. Also in this case, apparently I screwed up my pipeline and was completely missing the wine step. Adding it, finally got me an exe with the correct icon set. 👍

So everything looks fine to me. I like that you changed the inputs of the readme to a table. Also readme looks fine to me. So feel free to merge.

firebelley commented 2 years ago

Awesome, thanks for taking a look! I love that phrase "The bug sits in front of the computer." I am going to start using that!

Merging 👍