Skip to content

Conversation

@PaintNinja
Copy link
Member

Shorthands can be nice, but too many can introduce ambiguity and inconsistency - this PR proposes to remove some in order to help with that.

There's some other cases not included in this PR yet but may be worth considering a redesign:

  1. jar files
jars {
    jar {
        file = "nested/vendor/dependency.jar"
    }
    jar "nested/vendor/another-dependency.jar"
}

Should we add a jar(String) adder shorthand to the root? Should the shorthand be removed altogether or the jar(Closure) removed instead?

  1. mixins
mixins {
    mixin "modid.mixins.json"
    mixin {
        config = "modid-server.mixins.json"
        environment = Environment.SERVER
    }
}

Should we have closures for client, server and common inside the MixinsBuilder?

  1. icons
icon {
    x16 = "small.png"
    x32 = "medium.png"
}
icon 64, "large.png"

Should we remove the icon(int, String) adder shorthand and replace it with an icon = "medium.png" setter shorthand in the root?

@PaintNinja PaintNinja requested a review from lukebemish July 17, 2024 21:28
it.'fabric-api' = "*"

mod "another-mod", ">=1.5.0"
mod("something-else", v(">0.5") & v('<1.0'))
Copy link
Member

Choose a reason for hiding this comment

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

I would like a version range combination test like this to remain in the file still

@lukebemish
Copy link
Member

lukebemish commented Jul 18, 2024

  1. the jar { } syntax should not be removed, as this matches how we handle lists of nested blocks elsewhere and that's the current structure of that in the FMJ, and other fields besides file could presumably be added in the future. It's also how stuff is handled at the plugin level!. I'd say either keep the jar 'string' syntax or move it to the root; I'm mildly in favor of the former as it matches, say, how we treat mixin configs but I have no huge preference.
  2. I'm generally of the opinion that if you're specifying more than one field it's probably not worth the shorthand; I certainly wouldn't have closures for client and server or whatever but I suppose I can see the point of single client('string') shorthand or the like?
  3. Not sure how the proposed setter shorthand would work -- don't you need the icon size still?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants