Skip to content

Grinder Block - #96

Open
tanvirt wants to merge 19 commits into
devfrom
grinder-block
Open

Grinder Block#96
tanvirt wants to merge 19 commits into
devfrom
grinder-block

Conversation

@tanvirt

@tanvirt tanvirt commented Mar 27, 2017

Copy link
Copy Markdown
Owner

The Grinder Block grinds items that are specified to have a grinding recipe. Still needs proper models for rendering.

grinder

grinder gui

register(new BlockInfiniteProducer());
register(new BlockPowerAnalyzer());

initBlockGrinder();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why have a function called initBlockGrinder? it turns something that could be one line into 4 lines now. In the future I could see having an initMachines but there really isn't a reason to put the actual register itself in its own function.

@tanvirt tanvirt Mar 27, 2017

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reasoning was that I imagined there would be some extra stuff I would have to do to fully initialize the grinder, which will actually be the case when the object is made generic. Like:

private void initBlockGrinder() {
  int numInputs = 1;
  int numOutputs = 1;
  String modelResourceLocation = "some/file/path/grinder.json";
  String guiLocation = "some/file/path/gui.png";
  String name = "blockGrinder";
  register(new SimpleIOMachine(numInputs, numOutputs, modelResourceLocation, guiLocation, name));
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright that works for me then we can revisit this once you make it generic

}

@Override
public void onBlockAdded(World parWorld, BlockPos parBlockPos, IBlockState parIBlockState) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick up for discussion: Why 'par'? I assume it stands for 'parameter' but why not just do something like:

(World world, BlockPos position, IBlockState state) for example?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I didn't write the par part myself. That's how it is populated when you create the class and all the unimplemented methods for the interface are created by IntelliJ. I do agree it would look better w/o the par though. I'll make the change.

}

@Override
public IBlockState getStateForPlacement(World world, BlockPos pos, EnumFacing facing,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you have an inconsistency now because you were doing World parWorld styled variable names and now they are the World worldstyle. I prefer the World world but we definitely have to be consistent.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refer to my prior blame of IntelliJ and Forge

}

@Override
public boolean onBlockActivated(World parWorld, BlockPos parBlockPos,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

private int ticksGrindingItemSoFar;
private int ticksPerItem;

public ContainerGrinder(InventoryPlayer parInventoryPlayer, IInventory parIInventory) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parameter nit again

}

@Override
public boolean canExtractItem(int parSlotIndex, ItemStack parStack, EnumFacing parFacing) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parameter nit

return grinderItemStackArray[slotEnum.INPUT_SLOT.ordinal()] != null;
}

private int timeToGrindOneItem(ItemStack parItemStack) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parameter nit

private final InventoryPlayer inventoryPlayer;
private final IInventory tileGrinder;

public GuiGrinder(InventoryPlayer parInventoryPlayer, IInventory parInventoryGrinder) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parameter nit

}

@Override
protected void drawGuiContainerForegroundLayer(int mouseX, int mouseY) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly make some of these values below private static final ints to make some values more clear

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Careful with using private static final too much. It weighs the class down with variables for no reason if the values aren't used in more than one location. But yea I could definitely use more named local variables.

import net.minecraft.world.World;

import javax.annotation.Nullable;
public abstract class BlockContainerTileEntity<TE extends TileEntity>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

insert space above class

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.

2 participants