Why is the method canVillagerPickupItem in EntityVillager written this way?

[MrDireball]

While looking at the code in EntityVillager, I found something that has caught my eye. In the canVillagerPickupItem method, rather than loop through a static array of items that it can pick up, it uses an if-statement with six ||s. The problem with this is that whatever the villager is allowed to pick up is hardcoded into the statement. With an array, you could just go to where the array is initialized and add it in. Is there any particular way why it was done like this? I know that the final result is still the same, but you could say that about pretty much any good or bad practice.

 

    private boolean canVillagerPickupItem(Item itemIn)
    {
        return itemIn == Items.BREAD || itemIn == Items.POTATO || itemIn == Items.CARROT || itemIn == Items.WHEAT || itemIn == Items.WHEAT_SEEDS || itemIn == Items.BEETROOT || itemIn == Items.BEETROOT_SEEDS;
    }

 

Edited January 18, 2019 by MrDireball

[Laike_Endaril]

This is probably just a legacy code thing.  Minecraft wasn't originally made for modding; mod support was added later, so you'll be finding a lot of things that are not as easy to mod as they could be.

 

But yeah, that's a pretty annoying one.  I don't have a good solution for you there, because...

...you can't alter the method directly without a bytecode edit

...you can't override the method unless you use an AT and extend the villager, and even if you do this, it will cause all kinds of incompatibility with other mods

 

Actually, I can think of one way to do this; you can write your own completely separate pick-up code somewhere else in a tick event, which checks if a villager is near your chosen item(s), and if so, makes them pick it up.  This is probably the easiest, most flexible, and most compatible solution, though not necessarily the most efficient at runtime.

 

You can minimize the performance impact by eg. using rectangular distance checks instead of circular distance checks (direct x and z comparisons instead of actual distance)

[Cadiboo]

44 minutes ago, Laike_Endaril said:

This is probably just a legacy code thing.  Minecraft wasn't originally made for modding; mod support was added later, so you'll be finding a lot of things that are not as easy to mod as they could be.

Minecraft has never supported modding on java, it has become accepted over time though. Minecraft (java) makes no provision for mods in its code

[MrDireball]

Even if Minecraft wasn't meant to mod, it still would have been better practice to cycle through an array at this point, or even better, use a dictionary.

 

I've heard that they pretty much completely rewrote everything a few years ago, so I'm surprised that they didn't add mod support considering that Minecraft among the most modded video games in existence.

[Cadiboo]

To clarify what diesieben said, the minecraft code Modders work with is decompiled from minecrafts obfuscated jar. It doesn’t resemble the actual source code of Minecraft that Mojang has

[MrDireball]

So the code was auto-generated from different code that does the same thing?

[DaemonUmbra]

What happens is when Mojang release a version or snapshot they run it through an obfuscator, this renames everything to what looks like gibberish and no longer has the same organization system (the hierarchy of files and folders is flattened) but is still valid code, some obfuscators go a step further and make slight tweaks to make things more efficient, an example of this is it might remove code that is never accessed (This is what creates the difference between the client and server with code that both use).

[Cadiboo]

9 hours ago, DaemonUmbra said:

some obfuscators go a step further and make slight tweaks to make things more efficient

An example of this is inlining constants and another example could be to turn an array that gets looped over and ORed into a constant OR expression. If you want to look at what the obfuscator actually does, google ProGuard as diesieben mentioned