Conditional mods & event bus subscribers

[wired-tomato]

Conditional class loading described in #201

@DependsOn("modid")
@Mod("testmod")
//class loaded if modid is present
public class TestMod {
    public TestMod() { }
}

@DependsOn({"modid", "modid2"})
@EventBusSubscriber(modid = "testmod")
//class loaded if modid and modid2 are present
public class TestModEvents {
    public void configEvent(ModConfigEvent event) {}
}

[neoforged-pr-publishing[bot]]

• [ ] Publish PR to GitHub Packages

[tmvkrpxl0]

Can you use @Dependency without wrapping it in @ChainDependency? I'm quite sure most use-cases would be just depending on one single modid.

[tmvkrpxl0]

Uh oh I accidentally mentioned someone

[shartte]

I don't like how this is modeled. dependencies is not the right term to do conditional loading, and I think the annotation nesting soup is getting a bit out of hand... It'll be some time until I can take a proper look at the impl too, but I think I'd prefer repeatable annotations separate from the top-level annotation used.

[Gaming32]

Looking at the beginning of the impl, it seems you can use dependencies = @ChainDependency(@Dependency("modid")). Though that's still too verbose in my opinion.

[wired-tomato]

Would a model like this be preferable?

@DependsOn(value = "neoforge", condition = Condition.ALL_PRESENT, operator = Operator.AND)
@DependsOn(value = {"othermod", "othermod2"}, condition = Condition.AT_LEAST_ONE_PRESENT, operator = Operator.OR)
@DependsOn("othermod3")
@Mod("testmod")
public class TestMod {
    public TestMod() {
        //called when neoforge and one of othermod or othermod2 are present or when othermod3 is present
    }
}

The least verbose one dependency would be @DependsOn("dependency") This model also avoids changing the definitions of @Mod and @EventBusSubscriber

[shartte]

Why do you need such a complex expression language beyond simple AND/OR which could be expressed through a repeatable annotation / value array attribute?

[Technici4n]

I think that OR conditions are quite suspicious if the goal is to avoid crashes because of missing classes.

[wired-tomato]

The new model is comprised of AND/ORs and their respective negations ORs are included because there are cases in which multiple mods provide the same public API Negations are included because there are cases in which mod conflictions end up changing certain behaviors

@DependsOn("neoforge")
@DependsOn(value = { "othermod", "othermod2" }, condition = DependsOn.Operation.OR)
@DependsOn("othermod3")

@DependsOn.List({
        @DependsOn("neoforge"),
        @DependsOn(value = { "othermod", "othermod2" }, condition = DependsOn.Operation.OR),
        @DependsOn("othermod3")
})

Both of the preceding annotations will behave the same

Refer to the @DependsOn annotation class for further details

[Technici4n]

The main goal of checking for dependencies in the annotation like this, is to prevent accidentally loading code that references nonexisting classes. If you add NOT or OR dependencies, that flies out of the window, and you might as well just add the following code instead of writing convoluted annotations:

if (ModList.get().isModLoaded("a") && !ModList.get().isModLoaded("b")) {
    bus.register(AButNotBEventHandler.class);
}

[wired-tomato]

Removed ORs and NOTs, @DependsOn now only takes an array of modids that must all be present for the class to be loaded

[neoforged-automation[bot]]

@wired-tomato, this pull request has conflicts, please resolve them for this PR to move forward.