MinecraftForge / Documentation

Read the docs MinecraftForge documentation
MIT License
460 stars 196 forks source link

DataProvider is implemented directly, no need for lambda #515

Closed Friendly-Banana closed 10 months ago

Friendly-Banana commented 10 months ago

The 1.19.x docs contain a lambda which is not only unnecessary but wrong. See e.g. docs/datagen/client/localization.md

@SubscribeEvent
public void gatherData(GatherDataEvent event) {
    event.getGenerator().addProvider(
        event.includeClient(),
        output -> new MyLanguageProvider(output, MOD_ID, "en_us")
    );
}

which should be

new MyLanguageProvider(event.getGenerator(), MOD_ID, "en_us"), at least for 1.19.2 and Forge 43.3.0.

This pattern can be observed in all places where a DataProvider will be passed as argument.

Jonathing commented 10 months ago

I can see why it's unnecessary, but why is it wrong?

Friendly-Banana commented 10 months ago

Because DataProvider requires two methods, run and get name. A lambda only implents one method

Jonathing commented 10 months ago

I am failing to follow. The implementation that this documentation example uses is merely a function that gets a data provider anyway. Using a lambda doesn't magically make it anything less than one. You can use a lambda to make an anonymous class of a data provider anyway to implement run and get name.

Friendly-Banana commented 10 months ago

Now I'm confused, the method takes a DataProvider, how would you make a lambda implement that? And why not directly an anonymous class/a normal class? I copied the code from the docs and compiling failed with an error: multiple abstract methods not implemented. I will post the exact one later.

LexManos commented 10 months ago

And this is why having code in documentation is a bad idea. It gets outdated fairly quickly. The docs arnt meant to be copy/pasteable functional code. They are meant to point you in the right direction. No idea what the current state of the docs are as I leave them for the community to deal with. What you should get from the docs is that there is a GatherDataEvent which is used to gather the data providers. And the specific implementation of that event is up to you the mod developer.

If you want to fix the code to be compileable then go for it I don't care. But this is a prime example of why having code in docs for Minecraft is a bad thing.

ChampionAsh5357 commented 10 months ago

The 1.19.x docs are for the latest 1.19 version, which is 1.19.4. There is a separate branch for 1.19.2, but it does not have the code example, meaning there is no issue with the docs as described.

ChampionAsh5357 commented 10 months ago

I will mention that the code in the 1.19.4 version doesn't compile as-is due to an ambiguation in the lambda provided, as it does not know whether the lambda is acting as a DataProvider or DataProvider$Factory. This can be fixed by wrapping the lambda in a delegate method or performing a cast. Still, I will keep this closed as it doesn't have to do with the issue itself.