New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactored StorageChannel enum into an interface #3138

Merged
merged 7 commits into from Oct 8, 2017

Conversation

Projects
None yet
2 participants
@yueh
Member

yueh commented Oct 6, 2017

This replaces the static enum with a more dynamic interface providing factory methods for handling network storage.

The intention is to allow for more storage types than the current item and fluid ones and no longer forces addons to abuse an existing channel for something disjunct.

A regression would be that the crafting grid is not more strict about using only IAEItemStack and no longer IAEStack. But in the long run this change and some other might allow us to open up the crafting calculation for different types.

This is still tentative and might not even be merged.

@@ -1183,7 +1189,7 @@ public static long nanoTime()
if( itemToAdd < input.getStackSize() )
{
final long original = input.getStackSize();
final StackType split = (StackType) input.copy();
final T split = (T) input.copy();

This comment has been minimized.

@orod-org

orod-org Oct 6, 2017

MINOR Remove this unnecessary cast to "T". rule

@orod-org

orod-org Oct 6, 2017

MINOR Remove this unnecessary cast to "T". rule

@@ -823,7 +830,7 @@ public void onListUpdate()
}
}
private class ChestMonitorHandler<T extends IAEStack> extends MEMonitorHandler<T>
private class ChestMonitorHandler<T extends IAEStack<T>> extends MEMonitorHandler<T>

This comment has been minimized.

@orod-org

orod-org Oct 6, 2017

MINOR Remove this use of "MEMonitorHandler"; it is deprecated. rule

@orod-org

orod-org Oct 6, 2017

MINOR Remove this use of "MEMonitorHandler"; it is deprecated. rule

}
else if( channel == AEApi.instance().storage().getStorageChannel( IFluidStorageChannel.class ) && this.fluidCell != null )
{
if( this.fluidCell == null )

This comment has been minimized.

@orod-org

orod-org Oct 6, 2017

MAJOR Change this condition so that it does not always evaluate to "false" rule

@orod-org

orod-org Oct 6, 2017

MAJOR Change this condition so that it does not always evaluate to "false" rule

ih.setPriority( this.priority );
final MEMonitorHandler<StackType> g = new ChestMonitorHandler<StackType>( ih );
final MEMonitorHandler<T> g = new ChestMonitorHandler<T>( ih );

This comment has been minimized.

@orod-org

orod-org Oct 6, 2017

MINOR Remove this use of "MEMonitorHandler"; it is deprecated. rule

@orod-org

orod-org Oct 6, 2017

MINOR Remove this use of "MEMonitorHandler"; it is deprecated. rule

}
return null;
}
private <StackType extends IAEStack> MEMonitorHandler<StackType> wrap( final IMEInventoryHandler h )
private <T extends IAEStack<T>> MEMonitorHandler<T> wrap( final IMEInventoryHandler<T> h )

This comment has been minimized.

@orod-org

orod-org Oct 6, 2017

MINOR Remove this use of "MEMonitorHandler"; it is deprecated. rule

@orod-org

orod-org Oct 6, 2017

MINOR Remove this use of "MEMonitorHandler"; it is deprecated. rule

yueh added some commits Oct 6, 2017

Refactored StorageChannel enum into an interface
This replaces the static enum with a more dynamic interface providing
factory methods for handling network storage.
@@ -85,15 +88,15 @@ public static IAEItemStack loadItemStackFromNBT( final NBTTagCompound i )
return null;
}
final AEItemStack item = AEItemStack.create( itemstack );
final AEItemStack item = AEItemStack.fromItemStack( itemstack );
item.setStackSize( i.getLong( "Cnt" ) );

This comment has been minimized.

@orod-org

orod-org Oct 7, 2017

MAJOR A "NullPointerException" could be thrown; "item" is nullable here. rule

@orod-org

orod-org Oct 7, 2017

MAJOR A "NullPointerException" could be thrown; "item" is nullable here. rule

@@ -120,7 +123,7 @@ public static IAEItemStack loadItemStackFromPacket( final ByteBuf data ) throws
return null;
}
final AEItemStack item = AEItemStack.create( itemstack );
final AEItemStack item = AEItemStack.fromItemStack( itemstack );
item.setStackSize( stackSize );

This comment has been minimized.

@orod-org

orod-org Oct 7, 2017

MAJOR A "NullPointerException" could be thrown; "item" is nullable here. rule

@orod-org

orod-org Oct 7, 2017

MAJOR A "NullPointerException" could be thrown; "item" is nullable here. rule

@@ -78,7 +79,7 @@ public PatternHelper( final ItemStack is, final World w )
this.canSubstitute = this.isCrafting && encodedValue.getBoolean( "substitute" );
this.patternItem = is;
this.pattern = AEItemStack.create( is );
this.pattern = AEItemStack.fromItemStack( is );
final List<IAEItemStack> in = new ArrayList<>();
final List<IAEItemStack> out = new ArrayList<>();

This comment has been minimized.

@orod-org

orod-org Oct 7, 2017

MAJOR Move the declaration of "out" closer to the code that uses it. rule

@orod-org

orod-org Oct 7, 2017

MAJOR Move the declaration of "out" closer to the code that uses it. rule

}
public static IAEItemStack loadItemStackFromPacket( final ByteBuf data ) throws IOException
public static IAEItemStack fromPacket( final ByteBuf data ) throws IOException

This comment has been minimized.

@orod-org

orod-org Oct 7, 2017

MINOR Remove the declaration of thrown exception 'java.io.IOException', as it cannot be thrown from method's body. rule

@orod-org

orod-org Oct 7, 2017

MINOR Remove the declaration of thrown exception 'java.io.IOException', as it cannot be thrown from method's body. rule

yueh added some commits Oct 7, 2017

@orod-org

This comment has been minimized.

Show comment
Hide comment
@orod-org

orod-org Oct 8, 2017

SonarQube analysis reported 78 issues

  • BLOCKER 7 blocker
  • CRITICAL 3 critical
  • MAJOR 41 major
  • MINOR 20 minor
  • INFO 7 info

Watch the comments in this conversation to review them.

Top 10 extra issues

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

  1. BLOCKER CraftingCPUCluster.java#L466: Refactor this method to not always return the same value. rule
  2. BLOCKER CraftingCPUCluster.java#L847: Refactor this method to not always return the same value. rule
  3. BLOCKER CraftingCPUCluster.java#L961: Refactor this method to not always return the same value. rule
  4. BLOCKER NetworkInventoryHandler.java#L138: Refactor this method to not always return the same value. rule
  5. BLOCKER SecurityStationInventory.java#L73: Refactor this method to not always return the same value. rule
  6. BLOCKER SecurityStationInventory.java#L90: Refactor this method to not always return the same value. rule
  7. BLOCKER Platform.java#L1406: Refactor this method to not always return the same value. rule
  8. CRITICAL AEBaseContainer.java#L224: The Cyclomatic Complexity of this method "setTargetStack" is 11 which is greater than 10 authorized. rule
  9. CRITICAL ContainerMEMonitorable.java#L130: Refactor this code to not nest more than 3 if/for/while/switch/try statements. rule
  10. CRITICAL AppEngInternalAEInventory.java#L148: The Cyclomatic Complexity of this method "insertItem" is 11 which is greater than 10 authorized. rule

orod-org commented Oct 8, 2017

SonarQube analysis reported 78 issues

  • BLOCKER 7 blocker
  • CRITICAL 3 critical
  • MAJOR 41 major
  • MINOR 20 minor
  • INFO 7 info

Watch the comments in this conversation to review them.

Top 10 extra issues

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

  1. BLOCKER CraftingCPUCluster.java#L466: Refactor this method to not always return the same value. rule
  2. BLOCKER CraftingCPUCluster.java#L847: Refactor this method to not always return the same value. rule
  3. BLOCKER CraftingCPUCluster.java#L961: Refactor this method to not always return the same value. rule
  4. BLOCKER NetworkInventoryHandler.java#L138: Refactor this method to not always return the same value. rule
  5. BLOCKER SecurityStationInventory.java#L73: Refactor this method to not always return the same value. rule
  6. BLOCKER SecurityStationInventory.java#L90: Refactor this method to not always return the same value. rule
  7. BLOCKER Platform.java#L1406: Refactor this method to not always return the same value. rule
  8. CRITICAL AEBaseContainer.java#L224: The Cyclomatic Complexity of this method "setTargetStack" is 11 which is greater than 10 authorized. rule
  9. CRITICAL ContainerMEMonitorable.java#L130: Refactor this code to not nest more than 3 if/for/while/switch/try statements. rule
  10. CRITICAL AppEngInternalAEInventory.java#L148: The Cyclomatic Complexity of this method "insertItem" is 11 which is greater than 10 authorized. rule

@yueh yueh merged commit 6e81f69 into rv5-1.12 Oct 8, 2017

3 of 4 checks passed

sonarqube SonarQube reported 78 issues, with 3 critical and 7 blocker
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
jenkins Success
Details

@yueh yueh deleted the feature-storagechannel-rework branch Oct 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment