Skip to content
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

Add blacklist of friendly mobs that hostile potion abilities don't affect. #60

Merged
merged 2 commits into from
Nov 25, 2017
Merged

Conversation

CplPibald
Copy link
Contributor

Fix for #41 as discussed.

Copy link
Member

@rubensworks rubensworks left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!
Just a couple of small comments.

}
}
}
}

private static java.util.Set<ResourceLocation> friendlyMobs = null;
Copy link
Member

Choose a reason for hiding this comment

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

Any reason why you aren't just importing Set?

static boolean isFriendlyMob(EntityLivingBase mob, EntityPlayer player) {

if (friendlyMobs == null) {
friendlyMobs = new java.util.HashSet<ResourceLocation>();
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

private static java.util.Set<ResourceLocation> friendlyMobs = null;

static boolean isFriendlyMob(EntityLivingBase mob, EntityPlayer player) {

Copy link
Member

Choose a reason for hiding this comment

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

Empty line.

/**
* Mobs that are immune to hostile potion effect abilities
*/
@ConfigurableProperty(category = ConfigurableTypeCategory.GENERAL, comment = "These mobs will not be affected by hostile area potion effects such as poison or weakness.")
Copy link
Member

Choose a reason for hiding this comment

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

requiresMcRestart should be set to true, due to how you have implemented config value synchronization.

Alternatively, you could set a changedCallback so that this list can be changed at runtime. (EvilCraft does this, if you need an example).

The second option would be nicer, but not required.

@rubensworks rubensworks added this to Accepted (To Do) in Features via automation Nov 23, 2017
@rubensworks rubensworks moved this from Accepted (To Do) to In Progress in Features Nov 23, 2017
@rubensworks
Copy link
Member

@CplPibald Also, could you sign the CLA? (It's something I just enabled)

@CplPibald
Copy link
Contributor Author

CplPibald commented Nov 24, 2017

I approve of the CLA and I agree with the terms presented in the Cyclops Contributor License Agreement.

However, when I try to click the agree link, the app requests way too many permissions for me to be comfortable with, such as reading and writing to all of my repos and public and private gists. See cla-assistant/cla-assistant#243 and cla-assistant/cla-assistant#78

@rubensworks
Copy link
Member

@CplPibald Did you try directly going to https://cla-assistant.io/CyclopsMC/CyclopsCore as someone in one of those issues suggested? Haven't checked myself, but that may require less permissions. (No problem if that's not the case)

@rubensworks rubensworks merged commit a4b2e0f into CyclopsMC:master-1.12 Nov 25, 2017
Features automation moved this from In Progress to Done Nov 25, 2017
@rubensworks
Copy link
Member

Great, thanks!

@CplPibald CplPibald deleted the feature41 branch November 25, 2017 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Features
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants