[SOLVED] Wrench fastest implementation

[quaiby]

Disclaimer: I know that the question is more Java-related than Forge-related.

I know 2 ways to implement wrench like in IC2 (needed to break machine): using interface or using annotation.

I.e. using interface:

interface IWrenchable {}

class MyBlock extends Block implements IWrenchable {}

class WrenchItem extneds Item {
  	@Override
	public EnumActionResult onItemUse(looong args list) {
        if (!worldIn.isRemote) {
            if (player.isSneaking() && worldIn.getBlockState(pos).getBlock() instanceof IWrenchable) {
                worldIn.destroyBlock(pos, true);
                player.getHeldItem(hand).damageItem(1, player);
            }
        }
        return super.onItemUse(player, worldIn, pos, hand, facing, hitX, hitY, hitZ);
    }
}

Using annotation:

@Retention(RetentionPolicy.RUNTIME)
@interface Wrenchable {}

@Wrenchable
class MyBlock extends Block {}

class WrenchItem extneds Item {
  	@Override
	public EnumActionResult onItemUse(looong args list) {
        if (!worldIn.isRemote) {
            if (player.isSneaking() && worldIn.getBlockState(pos).getBlock().getClass().isAnnotationPresent(Wrenchable.class)) {
                worldIn.destroyBlock(pos, true);
                player.getHeldItem(hand).damageItem(1, player);
            }
        }
        return super.onItemUse(player, worldIn, pos, hand, facing, hitX, hitY, hitZ);
    }
}

 

Which way is better and faster?

Edited April 29, 2018 by quaiby

[quaiby]

  On 4/29/2018 at 1:32 PM, diesieben07 said:

Only one is possible, since an annotation does not allow the following, which is necessary:

interface Wrenchable {
    boolean isWrenchable(World world, BlockPos pos, IBlockState state, EntityPlayer player)
}

 

Tying the entire block to "yes" or "no" is terrible.

 

If you are worried about performance, an instanceof check is blazing fast. Annotation reflection is not.

Expand  

Thank you! But this method is not necessary for me, my mod allow players to break machines not only using wrench. Wrench just makes it easier

Edited April 29, 2018 by quaiby

[quaiby]

  On 4/29/2018 at 1:40 PM, diesieben07 said:

I am not sure how that is relevant. I assume this is for some kind of API of your mod, allowing other mod's blocks to be wrenched? If so, yes, you need this method.

Expand  

Nope, that's part of the internal mixin needed to interact with machines from my mod

Edited April 29, 2018 by quaiby

[jabelar]

I'm not really sure why you're so concerned about this. There are dozens of legitimate ways to achieve the result you're after. Each may have different advantages, and some may feel "cleaner" to implement, but as long as it works it is valid. Performance shouldn't be an issue in any reasonable implementation because this will only happen very occasionally in the game.

 

To me the biggest question is whether you might want to use your wrench on machines from vanilla and from other mods. In that case I would take the approach of simply creating a collection (List, Set, Array, whatever suits best) of blocks that are "wrenchable" and then just check if block is member of the list when using the wrench. To me that is the most extendable approach.

[quaiby]

  On 4/29/2018 at 3:12 PM, jabelar said:

I'm not really sure why you're so concerned about this. There are dozens of legitimate ways to achieve the result you're after. Each may have different advantages, and some may feel "cleaner" to implement, but as long as it works it is valid. Performance shouldn't be an issue in any reasonable implementation because this will only happen very occasionally in the game.

 

To me the biggest question is whether you might want to use your wrench on machines from vanilla and from other mods. In that case I would take the approach of simply creating a collection (List, Set, Array, whatever suits best) of blocks that are "wrenchable" and then just check if block is member of the list when using the wrench. To me that is the most extendable approach.

Expand  

Though check happens rarely, big list of blocks can cause lag spike anyway because java will iterate over all of the entries in the list. Also using interface allows to reduce amount of code. Compare: one word in the class signature plus 5 lines plain check vs list of blocks that contain 20+ entries and 10+ lines iterator

Edited April 29, 2018 by quaiby

[quaiby]

I'm just very crazy about any amount of possible performance 

[Draco18s]

  On 4/29/2018 at 12:26 PM, quaiby said:

worldIn.getBlockState(pos).getBlock() instanceof IWrenchable

Expand  

Why do this? This code is already running in the block at that position. It is equivalent to this instanceof IWrenchable

Which you already know is true. 

[jabelar]

  On 4/29/2018 at 3:38 PM, quaiby said:

I'm just very crazy about any amount of possible performance 

Expand  

That actually is not the best strategy from a programming perspective. You should always try to do the most logical approach (i.e. most likely bug-free and most maintainable, which usually implies being simple and readable). Trying to be clever for performance should only be undertaken when you know performance is actually a problem (you should profile your code).

 

Also, your statements about perceived performance are not really true. You said that looping through a lot of blocks can cause lag, but that is not true for this case. Looping through a lot of block POSITIONS can cause lag because the numbers really add up. In a single chunk there are 65k block positions. There are literally millions of block positions in a Minecraft world. However, looping through block TYPES is unlikely to be more than a couple hundred. And in your case your item only works on machines so your list is only going to be like 10 entries long.  Also, Java collections are already compiled extremely efficiently. So doing something like a contains() method to check against a set of 10 block types is going to be instantaneous.

 

Regarding lines of code, that shouldn't be a factor in coding in most situations. Being bug-free is the most important thing and you have ability to cut and paste so any repetitive code doesn't really take any extra time (in fact is usually faster than trying to be clever with loops and special conditions).

 

Basically in modern computers you rarely should worry about these things (performance or code size). Performance is only a concern with algorithms that grow exponentially (like searching regions of block positions, pathfinding, and so forth).

 

 

Edited April 29, 2018 by jabelar

[quaiby]

  On 4/29/2018 at 9:16 PM, Draco18s said:

Why do this? This code is already running in the block at that position. It is equivalent to this instanceof IWrenchable

Which you already know is true. 

Expand  

I cannot understand what you mean. This code is inside Item class, not block. So it should check if the wrench is being used on a machine and not on a grass, for example.

[quaiby]

  On 4/29/2018 at 9:39 PM, jabelar said:

...You should always try to do the most logical approach (i.e. most likely bug-free and most maintainable, which usually implies being simple and readable).

Expand  

I already try to write plain and understandable code. But the interface and type check way seems to me more right than list. Minifying amount of the code also makes it more understandable.

  Quote

...Also, your statements about perceived performance are not really true...

Expand  

Yes, you're right. I just wasn't been able to imagine more suitable example and I got what I got .

  Quote

...Being bug-free is the most important thing and you have ability to cut and paste so any repetitive code doesn't really take any extra time.

Expand  

But entries can be added dynamically. And dynamic things are often less bug-free, than hard-coded things, aren't they?

  Quote

...Basically in modern computers you rarely should worry about these things.

Expand  

This way is applicable for mods, but what about smt else? It is better if you always care, even if it is not neccessary in most of the cases, than if you never care.

[jabelar]

  On 5/2/2018 at 9:42 AM, quaiby said:

This way is applicable for mods, but what about smt else? It is better if you always care, even if it is not neccessary in most of the cases, than if you never care.

Expand  

 

The only situation where you have to by default worry about performance and code size is in "embedded" programming -- code that goes into an inexpensive device like a toy or a networking router. 

 

Otherwise, when coding for a full-size computer the main place you have to worry about performance is in things that don't scale well like large databases, tree algorithms, recursive algorithms and convergent iteration. For example if you're trying to do "generative adversarial networks" in AI you would care about performance a lot.

 

In Minecraft there are only a few cases where people get in trouble with performance. The main one is by not understanding the sheer number of block positions there are in even a seemingly small area -- structure generation, block pattern detection, alternate pathfinding algorithms, all fall in this category.

[Draco18s]

  On 5/2/2018 at 8:53 AM, quaiby said:

I cannot understand what you mean. This code is inside Item class, not block. So it should check if the wrench is being used on a machine and not on a grass, for example.

Expand  

Ah, you are right. I saw the class BlockWhatever implements IWrenchable at the top of a code block and didn't see that it was an empty class with an item definition right after it.