Kir-Antipov / mc-publish

🚀 Your one-stop GitHub Action for seamless Minecraft project publication across various platforms.
MIT License
228 stars 20 forks source link

[Feature Request] allow alternative type for dependencies #111

Closed rlnt closed 7 months ago

rlnt commented 7 months ago

Description

I want to use the mc-publish action in a reusable workflow because my organization has plenty of repositories where the build and publish action is always the same. When using a reusable workflow, future changes only need to be made in a single place.

It works pretty well so far. The only culprit I found was the specification of dependencies. It accepts an array. The problem with that is that you can't pass an array to a reusable workflow because the workflow dispatch event has to array input type.

Since each mod may have different dependencies, I need to find a way how to pass the respective dependencies to the workflow. But it doesn't seem to be possible without you adjusting the action a bit.

Alternative Solutions

As an alternative solution, I tried to pass a JSON object string and then transform it into an array like this: https://github.com/AlmostReliable/almostunified/blob/1.20.1/.github/workflows/release.yml#L52

And then use a string input in the reusable release workflow where I split it up when passing it to the mc-publish action: https://github.com/AlmostReliable/.github/blob/main/.github/workflows/release-java17.yml#L384

It's commented because it didn't work and I made a release without dependencies. Here is the workflow where it was still enabled. The type wasn't accepted sadly: https://github.com/AlmostReliable/almostunified/actions/runs/8118148397

Risks

I am not sure if you can do something like

dependencies?: Dependency[] | string;

since the action might not be able to infer the type? If it works, you could then pass it to some parser to split it up. Or if you define a specific format for the string, that would also be fine.

A multiline string could also work and then split the dependencies after \n.

Other Information

No response

Kir-Antipov commented 7 months ago

Hi! I think you just got a little bit confused :)

The action.ts file you brought up is not a part of the public interface of this action.

If it works, you could then pass it to some parser to split it up. Or if you define a specific format for the string, that would also be fine.

A multiline string could also work and then split the dependencies after \n.

This is already how everything works :D GitHub Actions only receive user input in the form of strings. So, whatever you specify, I always receive a string:

So, sadly, you never had an option of supplying me with an array/object as is because GitHub won't allow that. Thus, everything was, is, and will be done via you providing me with strings and me then parsing those strings into objects I actually need.

At some point, parsing the parameters in-place became too tedious and cumbersome because of mc-publish's continuous growth. Thus, to de-spaghettify my code and make it more robust, I invested a lot of time and effort into making a module that can process the user input beforehand, perform all the needed parsing and checks, so I don't need to handle strings in the business logic, and can focus on writing the actual working code. And I pretty much succeeded at that, so now the process of exchanging inputs between you and the action looks like this:

You provide strings -> magic -> I get a strongly typed input object, the structure of which you discovered in the action.ts file mentioned above.

So, don't search for answers in action.ts, look for them in the README, as internally I deal with quite different things than those that could be seen outside. And in the README, I provide the following example of usage of the dependencies input:

          dependencies: |
            required-dependency
            optional-dependency@0.1.0(optional)
            recommended-dependency@0.2.0(recommended)
            embedded-dependency@0.3.0(embedded)
            conflicting-dependency(conflicting)
            incompatible-dependency(incompatible)
            fabric@0.81.1+1.19.4(required){modrinth:P7dR8mSH}{curseforge:306612}#(ignore:github)

This is not an array. This is just a multi-line string syntax in YAML (well, 1 out of 9 available, not even a joke). Thus, finally going back to your configuration, passing dependencies around is as easy as with every other input:

- dependencies: "['jei(optional){curseforge:238222}{modrinth:u6dRKJwZ}', 'rei(optional){curseforge:310111}{modrinth:nfn13YXA}']"
+ dependencies: |
+   jei(optional){curseforge:238222}{modrinth:u6dRKJwZ}
+   rei(optional){curseforge:310111}{modrinth:nfn13YXA}

Let me know if this answers your question! :)

Kir-Antipov commented 7 months ago

Since each mod may have different dependencies [...]

Btw, personally, I would let mc-publish deal with dependencies. This way, you do not need to worry about such things at all. So, here are the dependencies from fabric.mod.json and mods.toml of the mod in question:

  "depends": {
    "fabric": ">=${fabricApiVersion}",
    "minecraft": ">=${minecraftVersion}",
    "java": ">=17"
  },
  "suggests": {
    "jei": ">=${jeiVersion}",
    "roughlyenoughitems": ">=${reiVersion}",
    "emi": ">=${emiVersion}"
  }
[[dependencies."${modId}"]]
modId = "forge"
mandatory = true
versionRange = "[${forgeVersion},)"
ordering = "NONE"
side = "BOTH"

[[dependencies."${modId}"]]
modId = "minecraft"
mandatory = true
versionRange = "[${minecraftVersion},)"
ordering = "NONE"
side = "BOTH"

[[dependencies."${modId}"]]
modId = "jei"
mandatory = false
versionRange = "[${jeiVersion},)"
ordering = "BEFORE"
side = "BOTH"

[[dependencies."${modId}"]]
modId = "roughlyenoughitems"
mandatory = false
versionRange = "[${reiVersion},)"
ordering = "BEFORE"
side = "BOTH"

[[dependencies."${modId}"]]
modId = "emi"
mandatory = false
versionRange = "[${emiVersion},)"
ordering = "BEFORE"
side = "BOTH"

mc-publish can already correctly infer fabric-api, jei, and emi from the Fabric jar, and just jei and emi from the Forge jar. The only thing we need to help mc-publish with is roughlyenoughitems, since this mod uses a slug that differs from its mod id. And it's quite easy to do right inside your mod's metadata.

So, add this to your fabric.mod.json (Fabric doesn't support declaring dependencies as objects yet, so all the additional information goes to the top-level custom.mc-publish entry):

"custom": {
  "mc-publish": {
    "dependencies": [
      "roughlyenoughitems(optional){curseforge:310111}{modrinth:nfn13YXA}"
    ]
  }
}

And add this to your mods.toml (on the other hand, Forge dependencies are always declared as objects, so we can attach all the information we need directly to those):

[[dependencies."${modId}"]]
modId = "roughlyenoughitems"
mandatory = false
versionRange = "[${reiVersion},)"
ordering = "BEFORE"
side = "BOTH"
    [dependencies."${modId}".mc-publish]
        curseforge=310111
        modrinth="nfn13YXA"

And done. Now, mc-publish will automatically specify fabric-api, jei, emi, and rei when you publish the Fabric version of your mod; and jei, emi, and rei when you publish the Forge version of your mod. I think this is much better than keeping a third list of all the needed dependencies hardcoded into your workflows.

rlnt commented 7 months ago

Thank you for the detailed response. It's always funny to submit an issue here because of your in-depth explanation and the side blows against Microsoft and how they implemented actions. 😄

I like to define my stuff explicitly so auto-detection is not really a consideration for me although it's a nice feature. I corrected my workflow to use a multiline string instead. I didn't know about the difficulties of inputs so your responses are appreciated. Thank you.

My issue is solved with the tweak to the input :)

Kir-Antipov commented 7 months ago

Well, I made this action to make life a bit easier for people, so it's only good when people actually know how to use it. And since I haven't finished a proper wiki yet, I'm always here to help :)

because of your in-depth explanation

I guess this is what quite a few hours put into answering on StackOverflow do to you xD Also, I always try to explain to folks not only how something works, but also why it works the way it does, so they can see a bigger picture. Because this is how I myself prefer to learn stuff.

and the side blows against Microsoft

I guess this is what obviously bad design decisions do to people. I just cannot help myself on this matter xD

I like to define my stuff explicitly so auto-detection is not really a consideration for me although it's a nice feature

I see you. Somebody prefers verbosity, somebody prefers their configs as small as possible. I'm just providing options :)

My issue is solved with the tweak to the input :)

Awesome! If you ever have any more questions - I'm always glad to help!