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

Added arrow shooting prevention flag #276

Closed
wants to merge 12 commits into from
Closed

Added arrow shooting prevention flag #276

wants to merge 12 commits into from

Conversation

Kruithne
Copy link

Add basic flag to prevent entities firing a bow in a certain region. Mostly intended for global configuration to prevent bows in general for two main reasons:

1: Prevent mass build-up of arrows without the need for another plug-in to do so.
2: Prevent people firing bows with special effects (such as TNT from another plug-in) in protected regions.

IntelliJ automatically did some re-factoring to the imports here as-well, not necessary but not harmful either.

@DarkArc
Copy link
Member

DarkArc commented Jul 10, 2013

This is a config setting, not a flag. I'm also not sure how useful this would be. Additionally, your choosen event only helps with entity fired arrows, it ignores dispenser fired arrows for example, despite its wording which implies otherwise.

@TomyLobo
Copy link
Collaborator

if you use "import foo.*", you can no longer look for usages of class foo.Bar by searching for "foo.Bar" with a plain text editor. That's the harm in your IDE's unasked behaviour

@Kruithne
Copy link
Author

@DarkArc Config setting, not flag then, I apologize for my incorrect wording. The reason why I chose a config setting over a flag is because I really only see this being use on a per-world basis and not for smaller regions. It would be rather useful for the reasons I stated, not every server has a use for bows and disabling them if they lack another plug-in to do so would be beneficial.

The reason a server I work on requires such a thing is to prevent bows from another plug-in (with explosions and such) being used in a world that we'd rather them not. It's only a small light-weight inclusion and we deemed it more sensible to include here than create a new plug-in for. I've updated the code to include any arrows fired from a dispenser also, another wording error.

@TomyLobo I fail to see that as a problem but we will agree to disagree, I have reverted the change in my latest commit.

@sk89q
Copy link
Member

sk89q commented Jul 10, 2013

Perhaps the configuration should be disable-player-arrow-shooting due to the more limited scope.
Edit: Apparently I over-skimmed.

But otherwise, it looks OK to me.

@DarkArc
Copy link
Member

DarkArc commented Jul 10, 2013

This may be a better event http://jd.bukkit.org/rb/apidocs/org/bukkit/event/entity/ProjectileLaunchEvent.html.

Also there are a couple of minor formatting issues... I'll point them out real quick.

@@ -165,6 +165,7 @@
public boolean regionInvinciblityRemovesMobs;
public boolean disableDeathMessages;
public boolean disableObsidianGenerators;
public boolean disableArrowShooting;
Copy link
Member

Choose a reason for hiding this comment

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

The lines here are not properly spaced, make sure you are using spaces, and not tabs.

@Kruithne
Copy link
Author

I skimmed over the conventions used by accident as I am used to churning to a different convention (personally think Java conventions used here are terrible, ugly and generally harder to follow), so apologies there.

@Kruithne
Copy link
Author

I agree on the ProjectileLaunchEvent over EntityShootBow, however I've kept it limited to arrows only as that's what the intended restriction was for, unless it would rather be extended as a value to disable all projectiles from being fired?


if (wcfg.disableArrowShooting && event.getItem().getTypeId() == ItemID.ARROW) {
event.setCancelled(true);
}
Copy link
Member

Choose a reason for hiding this comment

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

The top two lines need moved over still, it should be something like:

    if (wcfg.disableArrowShooting && event.getItem().getTypeId() == ItemID.ARROW) {
        event.setCancelled(true);
    } 
}

@@ -894,6 +895,10 @@ public void onBlockDispense(BlockDispenseEvent event) {
}
}
}

if (wcfg.disableArrowShooting && event.getItem().getTypeId() == ItemID.ARROW) {
event.setCancelled(true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

return;

Copy link
Collaborator

Choose a reason for hiding this comment

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

So we don't forget to add it (and thus waste CPU) should we add something after it.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry yanked my comment out of there before you replied after discussing it with another developer, made a commit to fix it.

if (wcfg.disableArrowShooting && event.getItem().getTypeId() == ItemID.ARROW) {
event.setCancelled(true);
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

You know, this whole section probably doesn't need to be here. ProjectileLaunch should cover all cases. I don't know if this would save the item from being removed though (and the ProjectileLaunch event wouldn't), someone want to look into that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

In that case, also check whether projectilelaunch prevents an item in a
dispenser from being used up if it's blocked.

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.

None yet

5 participants