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

Specify supported dyes to allow co-existing with dye addon mods #4764

Open
wants to merge 1 commit into
base: 1.20.x
Choose a base branch
from

Conversation

PssbleTrngle
Copy link

This is a proposed fix for #4589

I extracted all required places which use DyeColor.values() and replaced it with a reference to a centralized location which manually specifies the vanilla dyes.
This way, dye colors injected by mods like dye depot are simply ignored.

@CraftyZombie
Copy link

Is there no way to somehow implement this fix into Dye Depot itself?
The number of mods it crashes with for the same reason is endless.
The current implementation is obviously centered around Botania, but is there no way to make the game less strict about hardcoded dyes?

@PssbleTrngle
Copy link
Author

The only way to fix this in dye depot I could think of, which would have been the alternative, is mixing into Botania and doing the same thing there.
Doing what this PR does seems to my like the cleanest and most simple solution to the problem

@PssbleTrngle
Copy link
Author

The only way to make the game less strict about hardcoded dyes is by not assuming that they are hardcoded, which this PR does. Dye Depot crashed with some other mods, but almost none directly on start time and most so far could be solved by mixing into one or maybe two specific method, in Botania there are a lot more places like you can see

@TheRealWormbo
Copy link
Collaborator

I don't see any mention of the Spectrolus in the changes. Did you test that as well?

@PssbleTrngle
Copy link
Author

No, I have not changed how the Spectrolus works, it still only requires vanilla wool. But I've checked how it's implemented and since it simply counts the index up to 15 and then resets to 0, it will not cause any problems with dye addons.

@TheRealWormbo
Copy link
Collaborator

@PssbleTrngle: Could you please run spotlessApply on your changes to fix the build error? It's probably just an unused import.

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

Successfully merging this pull request may close these issues.

3 participants