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

wool to make use of global dye.dyes #2366

Merged
merged 1 commit into from May 26, 2019
Merged

wool to make use of global dye.dyes #2366

merged 1 commit into from May 26, 2019

Conversation

Zweihorn
Copy link
Contributor

@Zweihorn Zweihorn commented May 19, 2019

affects two files:

  • mods/wool/depends.txt
  • mods/wool/init.lua

I see no reason why 'wool' should establish local dye definitions by ignoring the global dye.dyes

@SmallJoker
Copy link
Member

To be precise: wool does not depend on dyes, as any other mod could add items with the required item groups. Well, I guess it's fine to link it with dye because it's loaded anyway 🤷‍♂️

@paramat
Copy link
Contributor

paramat commented May 20, 2019

I'm neutral on this, but reducing dependencies is useful for those who use certain MTG mods unchanged in their own games.

@Desour
Copy link
Member

Desour commented May 21, 2019

The order of the tables is different, but this should make no problems.

Look at both tables if you want.

local dyes = {
{"white", "White"},
{"grey", "Grey"},
{"black", "Black"},
{"red", "Red"},
{"yellow", "Yellow"},
{"green", "Green"},
{"cyan", "Cyan"},
{"blue", "Blue"},
{"magenta", "Magenta"},
{"orange", "Orange"},
{"violet", "Violet"},
{"brown", "Brown"},
{"pink", "Pink"},
{"dark_grey", "Dark Grey"},
{"dark_green", "Dark Green"},
}

dye.dyes = {
{"white", "White"},
{"grey", "Grey"},
{"dark_grey", "Dark grey"},
{"black", "Black"},
{"violet", "Violet"},
{"blue", "Blue"},
{"cyan", "Cyan"},
{"dark_green", "Dark green"},
{"green", "Green"},
{"yellow", "Yellow"},
{"brown", "Brown"},
{"orange", "Orange"},
{"red", "Red"},
{"magenta", "Magenta"},
{"pink", "Pink"},
}

Imo, this makes sense.
If any other game wants to use the wool mod, it can provide its own dye mod.

👍

@paramat
Copy link
Contributor

paramat commented May 26, 2019

Yes, it does make sense in the context of MTG 👍

@Zweihorn
Copy link
Contributor Author

Thank you very much.

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.

None yet

4 participants