Skip to content

Conversation

ah-OOG-ah
Copy link
Contributor

Syntax sugar for cases where you may only want to exclude a mod if a config is set (or not set).
Current use case: a transformer in Hodgepodge that works with DAPI, but only if a slow RFB plugin is enabled.

Syntax sugar for cases where you may only want to exclude a mod if a config is set (or not set).
@ah-OOG-ah ah-OOG-ah enabled auto-merge (squash) July 17, 2025 05:18
@mitchej123 mitchej123 requested a review from a team July 17, 2025 05:40
@Alexdoru
Copy link
Contributor

This is wrong :

It doesn't use a supplier and checks for the config right away (might not be loaded

And you can already do that with the current system by simply extending the class and adding your method

@ah-OOG-ah
Copy link
Contributor Author

ah-OOG-ah commented Jul 17, 2025

This is wrong :

It doesn't use a supplier and checks for the config right away (might not be loaded

To address this I'd have to rework the mod exclusions to use suppliers instead of TargetedMods directly. If someone needs a supplier, we can add that later, but as written it does exactly what I need it to.

And you can already do that with the current system by simply extending the class and adding your method

Yes, that's obviously possible. It's also irrelevant, as the entire system could be (and used to be) implemented in a separate mod. I don't see any reason why these shims shouldn't be in Unimixins itself.

@FalsePattern
Copy link
Member

FalsePattern commented Jul 19, 2025

I've modified the mod exclusions on a temporary branch to use BooleanSupplier instead of a raw boolean, this allows the condition to be evaluated during the shouldLoad(Set<ITargetedMod>) call. See:
https://github.com/LegacyModdingMC/UniMixins/tree/decisions-v2
Regular boolean arguments are not supported for this because those can already be done using an if condition, and not calling addExcluded/RequiredMod on the builder if the branch condition is false.

@FalsePattern
Copy link
Member

Merged the BooleanSupplier re-implementation into this PR after some discussion in the Discord channel

https://discord.com/channels/741043720241152001/1168018260105637928/1396154842237505618

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants