NEW-CYLANDIA / little-warioware

A collectively developed take on Nintendo's WarioWare series, made in Godot Engine
https://new-cylandia.itch.io/little-warioware
MIT License
44 stars 12 forks source link

Initial build-and-publish GH Actions workflow [#27] #30

Closed lazerwalker closed 1 year ago

lazerwalker commented 1 year ago

This copies (and ever-so-slightly updates) our build-and-publish workflow from A Little Game Called Mario:

  1. Auto-format GDScript via the gdformat tool, making a bot git commits if necessary
  2. Creates a credits.txt file containing a list of GitHub usernames based on GH contributor API in the root directory of the project, ready for consumption by Godot
  3. Builds the game and uploads it as an artifact
  4. Deploys the build to Itch.io

On merging this, steps 1-3 should work (although we are not yet consuming the credits.txt file). Step 4 requires us to generate a butler API key (instructions here: https://itch.io/docs/butler/login.html#running-butler-from-ci-builds-travis-ci-gitlab-ci-etc). This is gated on @iznaut either generating a key or giving me access to the new-cylandia itch account.

The two pieces that exist in little-mario that haven't been ported:

  1. Deploy previews. I'm going to attempt building these using Azure Static Web Apps instead of Netlify
  2. The various grab bag of checks we had in place (validating narrative JSON, collision mask layer checks). My instinct is to add these sorts of checks ad-hoc as issues arise.

I think this should be good to merge, unless anyone has any thoughts.

My one open question: this hardcodes us as using Godot 3.5.2. I think this is correct, but isn't documented anywhere. I'd love someone else who's been working on this to confirm that, and if so we should probably add it to the README and add the same sort of popup (#29)

paulloz commented 1 year ago

Two thoughts:

lazerwalker commented 1 year ago

I agree that automatic commits are generally Not Great.

A problem we ran into frequently with Little Mario was a lot of people would commit changes that converted whitespace from tabs to spaces (or the other way around, can't remember) since that was a per-editor setting you couldn't enforce at the project level. A general concern was asking folks with less git and/or programming experience to make and push stylistic changes; a specific concern was that the only autoformatter we could find required you to have a functioning local python3 setup and be comfortable operating it from the CLI. For that set of constraints, I still think an autocommit bot was the right choice. Notably, we have also not yet (to my knowledge) encountered a situation where the autoformatter has caused a bug.

What might have changed now?

  1. There might be better autoformat tools (i.e. an editor plugin to do autoformat-on-save that we can include on a project level). This doesn't appear to be the case from a quick search (gdformat now has an editor plugin, but still requires a python3 install), but I might be missing something
  2. Godot might now let you enforce whitespace settings at a project level (rather than an editor-level setting you can't enforce via project file). I see no evidence this is the case, but I could be wrong.
  3. This project might be sufficiently different from Little Mario to suggest different requirements, which leads into a discussion of "core" vs "microgame" contributions.

I agree we should be more stringent about code review standards for metastructure changes than individual microgames. But I still like the idea of keeping the barrier to contribution for "core" changes as low as possible -- if the autoformatter allows a class of PR complaints to be irrelevant, I'm not sure if it's better for the project to have "core" contributions have a hard gdlint check (and a requirement to just set up and run gdformat locally) rather than stopping it from being able to share a technical workflow with game contributions and having the more stringent standards be purely social/code review?

Maybe it's also the case that we don't care about tabs vs spaces for individual games, but having that be inconsistent feels wrong to me.

I truly don't know the right choice here. My gut is still that enforcing formatting with auto-commits across all commits is an okay strategy for our specific project. I think it's less the case here than it was with Little Mario, but it still feels like a low-effort low-friction option compared to coming up with a more complicated strategy.

lazerwalker commented 1 year ago

I guess to also try to shift this from philosophical debate to something more actionable (although I think the discussion is important!): this is a relatively small amount of code, that we know from Little Mario works reasonably well. My instinct is to merge this in as-is, and I'm not at all opposed to replacing it sooner rather than later if you want to put together an alternative flow.

lazerwalker commented 1 year ago

Merging this in for now! To summarize my current viewpoint about code formatting:

  1. I think it's important that we save moderators from having to explain to new contributors how to fix whitespace issues. An auto-formatter that makes a bot commit after merge is an imperfect solution; I'd be excited for alternatives that still preserve the property of not confusing non-coders / the git-unfamiliar in order to spare moderator effort.
  2. I think we can assume more competency for "core" contributors, but I personally can't be bothered to create a more complicated setup that has separate technical workflows / GH Actions paths for "core" vs "microgame" contributions. If someone wants to do that work (to e.g. not auto-format core contributions), I'd potentially be into that as a future improvement.