Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Zero-length array allocation inspection (idea) #504

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

Open
RusTit wants to merge 103 commits into ForestryMC:master-MC1.12
base: master-MC1.12
Choose a base branch
Loading
from RusTit:memory_zero_Array

Conversation

@RusTit
Copy link
Contributor

@RusTit RusTit commented Apr 5, 2018

Text:
Reports on allocations of arrays with known lengths of zero. Since array lengths in Java are non-modifiable, it is almost always possible to share zero-length arrays, rather than repeatedly allocating new zero-length arrays. Such sharing may provide useful optimizations in program runtime or footprint. Note that this inspection does not report zero-length arrays allocated as static final fields, as it is assumed that those arrays are being used to implement array sharing.
And also I removed two methods. They are the same as those of the parent class.

RusTit added 16 commits April 5, 2018 22:35
@RusTit RusTit mentioned this pull request Apr 6, 2018
@Nedelosk Nedelosk requested a review from mezz April 6, 2018 13:53
Copy link
Contributor Author

RusTit commented Apr 7, 2018

@Nedelosk please review.

Copy link
Contributor Author

RusTit commented Apr 12, 2018

No problem, but it can take some time.

return itemStack;
}

private static final int MAX_ITEM_DAMAGE = 16387;
Copy link
Member

Choose a reason for hiding this comment

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

Any idea what the significance of this number is?
I thought it might be a power of 2 minus 1, but the nearest power of 2 is 2^14 (16384) which is also a weird number.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea, I was trying to find this value (binnie, forestry, forge) but no successed

Copy link
Contributor

@temp1011 temp1011 Apr 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Max seems to be OreDictionary.WILDCARD_VALUE at 32767 so this doesn't even have the right number of bits.

@Nullable
private static Pattern PATTERN;

private static Pattern GetPattern() {
Copy link
Member

Choose a reason for hiding this comment

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

Method names should start with a lowercase letter.

Actually I think this should just be created statically instead of using a singleton getter, since it is not very expensive to create and does not pull in lots of other classes.

private static Pattern PATTERN = Pattern.compile("_", Pattern.LITERAL);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case it is not. At first version I do like you describe. But after mc run, it crushed with NullPointException in

@Override
	public String toString() {
		return GetPattern().matcher(super.toString().toLowerCase(Locale.ENGLISH)).replaceAll(StringUtils.EMPTY);
}

Where is some issue with object creating (initial). At moment then toString invoked PATTERN will be null.

Copy link
Member

Choose a reason for hiding this comment

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

Hm enums are weird, I guess they are being constructed before static. This is fine then.

@SideOnly(Side.CLIENT)
public void onRenderBackground(int guiWidth, int guiHeight) {
Object texture = CraftGUITexture.BUTTON_DISABLED;
Object texture;
Copy link
Member

Choose a reason for hiding this comment

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

One nice use of final is for cases like this. You can make final Object texture; and the static analyzer will complain "variable 'texture' might not have been initialized" if there is a case where it is not set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I introduced the proposed changes, but it seems that the static analyzer worked correctly without them.

this.branchDescription.clear();
String desc = branch.getDescription();
if (desc == null || Objects.equals(desc, "") || desc.contains("for.")) {
if (desc == null || desc.length() == 0 || desc.contains("for.")) {
Copy link
Member

Choose a reason for hiding this comment

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

You can use org.apache.commons.lang3.StringUtils#isEmpty to check desc

for (int row = 0; row < rowCount; ++row) {
for (int column = 0; column < columnCount; ++column) {
final ControlSlot slot = new ControlSlot.Builder(this, column * 18, row * 18)
.assign(InventoryType.PLAYER,column + row * columnCount);
Copy link
Member

Choose a reason for hiding this comment

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

this line is using spaces for indentation, the one above is tabs

tooltip.add(slot.getName());
if (tooltipFlag.isAdvanced()) {
Collection<EnumFacing> inputSides = slot.getInputSides();
Collection<EnumFacing> inputSides = slot.getInputSides();
Copy link
Member

Choose a reason for hiding this comment

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

this line indents with spaces

tooltip.add(TextFormatting.GRAY + I18N.localise(ModId.CORE, "gui.side.insert", MachineSide.asString(inputSides)));
}
Collection<EnumFacing> outputSides = slot.getOutputSides();
Collection<EnumFacing> outputSides = slot.getOutputSides();
Copy link
Member

Choose a reason for hiding this comment

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

this line indents with spaces

public void addInformation(ItemStack stack, @Nullable World worldIn, List<String> tooltip, ITooltipFlag flagIn) {
super.addInformation(stack, worldIn, tooltip, flagIn);
IItemMiscProvider item = this.getItem(stack.getItemDamage());
IItemMiscProvider item = this.getItem(stack.getItemDamage());
Copy link
Member

Choose a reason for hiding this comment

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

this line indents with spaces

@Override
public String getItemStackDisplayName(final ItemStack stack) {
IItemMiscProvider item = this.getItem(stack.getItemDamage());
IItemMiscProvider item = this.getItem(stack.getItemDamage());
Copy link
Member

Choose a reason for hiding this comment

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

this line indents with spaces

if ((!FluidRegistry.registerFluid(bFluid)) || (FluidRegistry.addBucketForFluid(bFluid))) {
Log.error("Liquid registered incorrectly - {} ", fluid.getIdentifier());
}
}
Copy link
Member

Choose a reason for hiding this comment

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

this line indents with spaces

Copy link
Member

@mezz mezz left a comment

Choose a reason for hiding this comment

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

there are indentation issues in the latest commits

Copy link
Member

mezz commented Apr 16, 2018

Please do not add more commits on to this PR, it needs to be reviewed and merged but that gets harder with more commits.
You can create a new PR for more changes.

Copy link
Contributor Author

RusTit commented Apr 16, 2018

Ok, but to make clear, where is still some bugs:

  1. Item icons painting on second layer. (Some description is above).
  2. Event key passing in ComponentCompartmentInventory.java and WindowCompartment.java (read my dialog with Nedelosk).

Copy link
Contributor Author

RusTit commented Apr 16, 2018

Item icon painting bug appears not only in the search window.
You can also see it in the slider (in which custom tab settings are set).

Copy link
Member

mezz commented Apr 16, 2018

If you have specific fixes for the changes made in this PR, I think that is fine.
I am just worried that this PR is going out of control, it seems to be fixing too many different things.

Copy link
Contributor Author

RusTit commented Apr 16, 2018

Not yet. I do not know how to fix a bug with rendering. I will fix the error with the key a little later with separate commits (will be last in this PR)

Copy link
Contributor Author

RusTit commented Apr 16, 2018

Apparently there is no bug with keys.
I did debugging by checking the passed string-keys through the events, in everything is ok.
P.S. but the bug with the renderer I can not solve myself.

Copy link
Contributor

KorDum commented Apr 16, 2018

Tabs indent or spaces?

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

Reviewers

@Nedelosk Nedelosk Nedelosk left review comments

@mezz mezz mezz requested changes

+1 more reviewer

@temp1011 temp1011 temp1011 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /