[1.12.2] Redstone lamp

[geqo]

I just copied the redstone lamp code and it works, but i don't know how to get my lamps instead redstone lamps, i've tryed make my own Blocks class but "Could not initialize class ru.geqo.lamps.init.Blocks"; Before it i tryed create LIT_WHITELAMP object right in WhiteLamp static method, but it doesn't help. What am I doing wrong?

Here's my code:

Main class

Spoiler

package ru.geqo.lamps;

import net.minecraftforge.fml.common.Mod;

@Mod(modid = Lamps.MODID, name = Lamps.NAME, version = Lamps.VERSION)
public class Lamps
{
    public static final String MODID = "lamps";
    public static final String NAME = "Lamps mod";
    public static final String VERSION = "0.1.0";
}

Registration handler

Spoiler

package ru.geqo.lamps;

import net.minecraft.block.Block;
import net.minecraft.client.renderer.block.model.ModelResourceLocation;
import net.minecraft.creativetab.CreativeTabs;
import net.minecraft.item.Item;
import net.minecraft.item.ItemBlock;
import net.minecraftforge.client.event.ModelRegistryEvent;
import net.minecraftforge.client.model.ModelLoader;
import net.minecraftforge.event.RegistryEvent;
import net.minecraftforge.fml.common.Mod;
import net.minecraftforge.fml.common.eventhandler.SubscribeEvent;
import ru.geqo.lamps.blocks.*;

@Mod.EventBusSubscriber(modid = Lamps.MODID)
public class RegistrationHandler
{
    public static Block whitelamp;
    public static Block lit_whitelamp;

    public static Item whitelampitem;

    static {
        whitelamp = new WhiteLamp(false).setRegistryName(Lamps.MODID, WhiteLamp.NAME).setCreativeTab(CreativeTabs.REDSTONE);
        lit_whitelamp = new WhiteLamp(true).setRegistryName(Lamps.MODID, "lit_whitelamp");
    }

    @SubscribeEvent
    public static void registerBlocks(final RegistryEvent.Register<Block> event)
    {
        event.getRegistry().register(whitelamp);
        event.getRegistry().register(lit_whitelamp);
    }

    @SubscribeEvent
    public static void registerItemBlocks(final RegistryEvent.Register<Item> event)
    {
        whitelampitem = new ItemBlock(whitelamp);
        whitelampitem.setUnlocalizedName(WhiteLamp.NAME);
        whitelampitem.setRegistryName(whitelamp.getRegistryName());

        event.getRegistry().register(whitelampitem);
    }

    @SubscribeEvent
    public static void registerModels(ModelRegistryEvent event)
    {
        ModelLoader.setCustomModelResourceLocation(whitelampitem, 0,
                new ModelResourceLocation(whitelamp.getRegistryName(), "inventory")
        );
    }
}

Lamp

Spoiler

package ru.geqo.lamps.blocks;

import net.minecraft.block.Block;
import net.minecraft.block.SoundType;
import net.minecraft.block.material.Material;
import net.minecraft.block.state.IBlockState;
import ru.geqo.lamps.init.Blocks;
import net.minecraft.util.math.BlockPos;
import net.minecraft.world.World;

import java.util.Random;

public class WhiteLamp extends Block
{
    public static String NAME = "whitelamp";
    private final boolean isOn;

    public WhiteLamp(boolean isOn) {
        super(Material.GLASS);
        setUnlocalizedName(NAME);
        setSoundType(SoundType.GLASS);
        setHardness(0.1F);
        setResistance(0.1F);

        this.isOn = isOn;

        if (isOn) {
            this.setLightLevel(1.0F);
        }

    }

    public void onBlockAdded(World worldIn, BlockPos pos, IBlockState state)
    {
        if (!worldIn.isRemote)
        {
            if (this.isOn && !worldIn.isBlockPowered(pos))
            {
                worldIn.setBlockState(pos, Blocks.WHITELAMP.getDefaultState(), 2);
            }
            else if (!this.isOn && worldIn.isBlockPowered(pos))
            {
                worldIn.setBlockState(pos, Blocks.LIT_WHITELAMP.getDefaultState(), 2);
            }
        }
    }

    public void neighborChanged(IBlockState state, World worldIn, BlockPos pos, Block blockIn, BlockPos fromPos)
    {
        if (!worldIn.isRemote)
        {
            if (this.isOn && !worldIn.isBlockPowered(pos))
            {
                worldIn.scheduleUpdate(pos, this, 4);
            }
            else if (!this.isOn && worldIn.isBlockPowered(pos))
            {
                worldIn.setBlockState(pos, Blocks.LIT_WHITELAMP.getDefaultState(), 2);
            }
        }
    }

    public void updateTick(World worldIn, BlockPos pos, IBlockState state, Random rand)
    {
        if (!worldIn.isRemote)
        {
            if (this.isOn && !worldIn.isBlockPowered(pos))
            {
                worldIn.setBlockState(pos, Blocks.LIT_WHITELAMP.getDefaultState(), 2);
            }
        }
    }

    @Override
    public int quantityDropped(Random random)
    {
        return 1;
    }
}

And blocks(just attempt, full copy net.from minecraft.init.Blocks)

Spoiler

package ru.geqo.lamps.init;

import com.google.common.collect.Sets;
import java.util.Set;
import javax.annotation.Nullable;
import net.minecraft.block.Block;
import net.minecraft.init.Bootstrap;
import net.minecraft.util.ResourceLocation;

public class Blocks
{
    private static final Set<Block> CACHE;
    public static final Block WHITELAMP;
    public static final Block LIT_WHITELAMP;

    /**
     * Returns the Block in the blockRegistry with the specified name.
     */
    @Nullable
    private static Block getRegisteredBlock(String blockName)
    {
        Block block = Block.REGISTRY.getObject(new ResourceLocation(blockName));

        if (!CACHE.add(block))
        {
            throw new IllegalStateException("Invalid Block requested: " + blockName);
        }
        else
        {
            return block;
        }
    }

    static
    {
        if (!Bootstrap.isRegistered())
        {
            throw new RuntimeException("Accessed Blocks before Bootstrap!");
        }
        else
        {
            CACHE = Sets.<Block>newHashSet();
            WHITELAMP = getRegisteredBlock("whitelamp");
            LIT_WHITELAMP = getRegisteredBlock("lit_whitelamp");
            CACHE.clear();
        }
    }
}

 

[Cadiboo]

1 hour ago, geqo said:

static { whitelamp = new WhiteLamp(false).setRegistryName(Lamps.MODID, WhiteLamp.NAME).setCreativeTab(CreativeTabs.REDSTONE); lit_whitelamp = new WhiteLamp(true).setRegistryName(Lamps.MODID, "lit_whitelamp"); }

Why are you initialising your blocks in a static initialiser?

[geqo]

21 minutes ago, Cadiboo said:

Why are you initialising your blocks in a static initialiser?

I don't know, is it bad?

[Cadiboo]

Recommend Forge convention

What: Not using static initialisers

Why: Using static initialisers does not allow you to control when your objects are created or the order in which your objects are created

Consequences: Using static initialisers prevents other mods from overriding your objects, prevents forge from being able to dynamically load/unload mods and can cause weird, unreproducible crashes.

How: Use the @ObjectHolder annotation if you need static references to your objects and create & register your objects in the appropriate registry event. (Note: before registry events Modders created and registered their items in preInit. This practice became out of date in 1.7 with the introduction of registry events, and is no longer encouraged or supported)

 

(Excuse the copy paste, it’s easier for me to do it, and it also ensures that I don’t forget to include some information)

Edited January 26, 2019 by Cadiboo

[geqo]

31 minutes ago, Cadiboo said:

Recommend Forge convention

What: Not using static initialisers

Why: Using static initialisers does not allow you to control when your objects are created or the order in which your objects are created

Consequences: Using static initialisers prevents other mods from overriding your objects, prevents forge from being able to dynamically load/unload mods and can cause weird, unreproducible crashes.

How: Use the @ObjectHolder annotation if you need static references to your objects and create & register your objects in the appropriate registry event. (Note: before registry events Modders created and registered their items in preInit. This practice became out of date in 1.7 with the introduction of registry events, and is no longer encouraged or supported)

 

(Excuse the copy paste, it’s easier for me to do it, and it also ensures that I don’t forget to include some information)

I need to create new instance of object block for every event then? Like this?

Spoiler

package ru.geqo.lamps;

import net.minecraft.block.Block;
import net.minecraft.client.renderer.block.model.ModelResourceLocation;
import net.minecraft.creativetab.CreativeTabs;
import net.minecraft.item.Item;
import net.minecraft.item.ItemBlock;
import net.minecraftforge.client.event.ModelRegistryEvent;
import net.minecraftforge.client.model.ModelLoader;
import net.minecraftforge.event.RegistryEvent;
import net.minecraftforge.fml.common.Mod;
import net.minecraftforge.fml.common.eventhandler.SubscribeEvent;
import ru.geqo.lamps.blocks.*;
import ru.geqo.lamps.init.Blocks;

@Mod.EventBusSubscriber(modid = Lamps.MODID)
public class RegistrationHandler
{
    @SubscribeEvent
    public static void registerBlocks(final RegistryEvent.Register<Block> event)
    {
        Block whitelamp = new WhiteLamp(false).setRegistryName(Lamps.MODID, WhiteLamp.NAME).setCreativeTab(CreativeTabs.REDSTONE);
        Block lit_whitelamp = new WhiteLamp(true).setRegistryName(Lamps.MODID, "lit_whitelamp");
        event.getRegistry().register(whitelamp);
        event.getRegistry().register(lit_whitelamp);
    }

    @SubscribeEvent
    public static void registerItemBlocks(final RegistryEvent.Register<Item> event)
    {
        Block whitelamp = new WhiteLamp(false).setRegistryName(Lamps.MODID, WhiteLamp.NAME).setCreativeTab(CreativeTabs.REDSTONE);
        ItemBlock whitelampitem = new ItemBlock(whitelamp);
        whitelampitem.setUnlocalizedName(WhiteLamp.NAME);
        whitelampitem.setRegistryName(whitelamp.getRegistryName());
        
        event.getRegistry().register(whitelampitem);
    }

    @SubscribeEvent
    public static void registerModels(ModelRegistryEvent event)
    {
        Block whitelamp = new WhiteLamp(false).setRegistryName(Lamps.MODID, WhiteLamp.NAME).setCreativeTab(CreativeTabs.REDSTONE);
        ItemBlock whitelampitem = new ItemBlock(whitelamp);
        whitelampitem.setUnlocalizedName(WhiteLamp.NAME);
        whitelampitem.setRegistryName(whitelamp.getRegistryName());
        
        ModelLoader.setCustomModelResourceLocation(whitelampitem, 0,
                new ModelResourceLocation(whitelamp.getRegistryName(), "inventory")
        );
    }
}

 

[Cadiboo]

50 minutes ago, Cadiboo said:

Use the @ObjectHolder annotation if you need static references to your objects

What: Using @ObjectHolder

Why: Using @ObjectHolder allows you to have static field references to your objects, allows other mods to override your objects without problems related to outdated field references. It manages your static references for you, without you having to manually manage and assign all your fields yourself 

Consequences: Not using @ObjectHolder can result in problems with mods overriding your objects, which can lead to undefined behaviour and/or crashes.

How: Put @ObjectHolder at the top of your object holding class (for example ModBlocks or ModItems). Have your modid as the parameter of the annotation. Have your field be public static final and null, and name them the same as the registry name of your object in capitals. After you register your objects (in the appropriate registry event) your fields will be automatically filled with the correct values.

For example:

@ObjectHolder(yourModId)

class ModBlocks {

    public static final Block YOUR_BLOCKS_REGISTRY_NAME_IN_CAPITALS = null;

}

50 minutes ago, Cadiboo said:

create & register your objects in the appropriate registry event

What: Using registry events

Why: Using registry events paves the way for Forge to dynamically load and unload mods without restarting minecraft. Registry events are dedicated events with support for mod-sorted execution and overriding objects in the registry and/or querying the registry. 

Consequences: Using static initialisers can result in weird & unreproducible crashes and creating & registering your objects in preInit is no longer supported.

How: In your event subscriber class subscribe to the appropriate registry event (Registry.Register<typeOfObject>)

10 minutes ago, geqo said:

I need to create new instance of object block for every event then?

I definitely did not say this

Edited January 26, 2019 by Cadiboo

[mirk]

Thanks for the info Cadiboo! I'm currently implementing this into my mod, as I realise I'm quite guilty of this.

 

Just a quick question: does the class being registered have to have a default constructor?

I have ItemBase and BlockBase classes, which take their names from their constructors and have no default constructor, how would I go about fitting these into the @ObjectHolder standard?

[Cadiboo]

16 hours ago, mirk said:

Just a quick question: does the class being registered have to have a default constructor?

No, I also never said anything about a default constructor. You have complete control over creating your items, you just have to do it at the right time.

16 hours ago, mirk said:

ItemBase and BlockBase

What: Not using an object base class

Why: Using an object base class (commonly called BlockBase or ItemBase) is unnecessary and is an anti-pattern. There is already a BlockBase class, it’s the minecraft Block class. Making a class just to have its children inherit default implementations of methods goes against the OOP principle of Composition over Inheritance

Consequences: Using a class like this stops you from extending other classes and because lots of minecraft code uses instanceof checks to specially handle logic, you are likely to encounter weird and hard-to-fix bugs.

How: Instead of putting all your common logic in 1 class and extending it, extract the logic to utility methods. For example: Instead of calling setRegistryName, setTranslationKey, etc. inside your object base’s constructor, extract it to a helper method and call it on every object when you create it. In this example "setup" calls the above methods. registry.register(setup(new CustomItem(), "custom_item"));

16 hours ago, mirk said:

which take their names from their constructors and have no default constructor, how would I go about fitting these into the @ObjectHolder standard?

You still have access to the registry name, just use the name you pass into the constructor

Edited January 27, 2019 by Cadiboo

[mirk]

Ah, excellent advice. Thank you again!

I'm going to go about changing all my stuff to better fit this pattern.

 

Is there a list of all of this advice somewhere?

If not, there should be! I'd love to read it all. Who knows how many mistakes I'm making, just picking up bad habits from other modders who have tried to make my life easier.

[Cadiboo]

21 hours ago, mirk said:

Is there a list of all of this advice somewhere?

 If not, there should be! I'd love to read it all. Who knows how many mistakes I'm making, just picking up bad habits from other modders who have tried to make my life easier.

The stuff I’ve posted is just from a list in my notes, you should read the EAQ and the Code Style post by diesieben, they have lots of good stuff

[mirk]

I'd love a copy of your notes, if you could PM me them or something. It looks like great advice!

And yeah, I should probably get out of this thread now and let you guys do your thing, as according to the EAQ.

 

Thanks so much!