Skip to content
This repository has been archived by the owner on Jan 22, 2021. It is now read-only.

Glass cutter #48

Merged
merged 14 commits into from
Sep 7, 2020
Merged

Glass cutter #48

merged 14 commits into from
Sep 7, 2020

Conversation

NCBPFluffyBear
Copy link
Contributor

@NCBPFluffyBear NCBPFluffyBear commented Sep 5, 2020

Short Description

Added glass cutter, an item that instantly breaks glass.

Additions/Changes/Removals

Registered and added GLASS_CUTTER

Related Issues

N?A

Checklist

  • I have fully tested the proposed changes and promise that they will not break everything into chaos.
  • I have also tested the proposed changes in combination with base Slimefun and made sure nothing breaks/unexpected happens.
  • I followed the existing code standards and didn't mess up the formatting.
  • I did my best to add documentation to any public classes or methods I added which may not be obvious to maintainers.
  • I have added Nonnull and Nullable annotations to my methods to indicate their behaviour for null values

FluffyBear added 2 commits September 4, 2020 20:01
Signed-off-by: FluffyBear <unconfigured@null.spigotmc.org>
Signed-off-by: FluffyBear <unconfigured@null.spigotmc.org>
@NCBPFluffyBear
Copy link
Contributor Author


@EventHandler
@SuppressWarnings("ConstantConditions")
public void onGlassCut(PlayerInteractEvent e) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why don't you just use the ItemUseHandler

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there an advantage to using an item handler than an event except neatness

Copy link
Collaborator

Choose a reason for hiding this comment

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

it saves code and not having to run the same shit twice for no reason

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems there no left click interact event, only right click

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right click has slower interaction speed, so no fun

Copy link
Collaborator

Choose a reason for hiding this comment

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

Still maintain this should be in w handler. Right click makes more sense to begin with and it saves doing isItem

Signed-off-by: FluffyBear <unconfigured@null.spigotmc.org>
@WalshyDev
Copy link
Collaborator

We follow Google Code Style by the way so please do follow that. Especially regarding formatting and line-length.

Signed-off-by: FluffyBear <unconfigured@null.spigotmc.org>
@J3fftw1
Copy link
Owner

J3fftw1 commented Sep 5, 2020

Make it an electric tool im not sure if i want normal tools in lx

Signed-off-by: FluffyBear <unconfigured@null.spigotmc.org>
@J3fftw1
Copy link
Owner

J3fftw1 commented Sep 5, 2020

do you have a vid?

@NCBPFluffyBear
Copy link
Contributor Author

https://streamable.com/xut2ih actually found a bug while doing this ill update in a bit

…ass.

Signed-off-by: FluffyBear <unconfigured@null.spigotmc.org>
@NCBPFluffyBear
Copy link
Contributor Author

Also do u think i should make it glowy

@WalshyDev
Copy link
Collaborator

You can break like 85 glass with one charge there. That's quite a lot.

@NCBPFluffyBear
Copy link
Contributor Author

Yes you can break 600 glass with 1 charge... breaking 1 stack kinda small

Signed-off-by: FluffyBear <unconfigured@null.spigotmc.org>
Copy link
Owner

@J3fftw1 J3fftw1 left a comment

Choose a reason for hiding this comment

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

does it cancel shearing of leaves and cobwebs etc etc?

@NCBPFluffyBear
Copy link
Contributor Author

No need to cancel? Its a ghast tear not a shear

@J3fftw1
Copy link
Owner

J3fftw1 commented Sep 5, 2020

My bad

FluffyBear and others added 4 commits September 5, 2020 15:00
Signed-off-by: FluffyBear <unconfigured@null.spigotmc.org>
Signed-off-by: NCBPFluffyBear <31554056+ncbpfluffybear@users.noreply.github.com>
Signed-off-by: NCBPFluffyBear <31554056+ncbpfluffybear@users.noreply.github.com>
src/main/java/dev/j3fftw/litexpansion/Items.java Outdated Show resolved Hide resolved

@EventHandler
@SuppressWarnings("ConstantConditions")
public void onGlassCut(PlayerInteractEvent e) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Still maintain this should be in w handler. Right click makes more sense to begin with and it saves doing isItem

@NCBPFluffyBear
Copy link
Contributor Author

The problem is that right click interaction is slower than left click.. maybe 5x or more

Signed-off-by: NCBPFluffyBear <31554056+ncbpfluffybear@users.noreply.github.com>
@WalshyDev
Copy link
Collaborator

The problem is that right click interaction is slower than left click.. maybe 5x or more

You could hold right click and run along a line of glass, it'd get them all. I don't see why you'd have an issue with that

@NCBPFluffyBear
Copy link
Contributor Author

Its very slow. Right clicking has a set delay no matter if you hold it or not, left click triggers as soon as you are looking at a new block face. If you want, I can make a video showing the speed difference. I saved the right click version I made a while ago

@NCBPFluffyBear
Copy link
Contributor Author

Actually, the wrench accepts both left and right click. Ill demonstrate using that. https://streamable.com/gtzgi2

@NCBPFluffyBear
Copy link
Contributor Author

I could also have i accept both left and right clicks so that players can break glass slowly or quickly, thats part of the reason why that exists for the wrench

@J3fftw1
Copy link
Owner

J3fftw1 commented Sep 6, 2020

Make sure to check for sf machines that are glass that those doesnt get broken

@NCBPFluffyBear
Copy link
Contributor Author

Already does

NCBPFluffyBear and others added 2 commits September 6, 2020 17:22
Signed-off-by: NCBPFluffyBear <31554056+ncbpfluffybear@users.noreply.github.com>
@NCBPFluffyBear
Copy link
Contributor Author

Tested and recorded it working too

Copy link
Collaborator

@WalshyDev WalshyDev left a comment

Choose a reason for hiding this comment

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

Gonna merge today, just small changes.

@EventHandler
@SuppressWarnings("ConstantConditions")
public void onGlassCut(PlayerInteractEvent e) {
Block block = e.getClickedBlock();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Final

Block block = e.getClickedBlock();
if (e.getAction() == Action.LEFT_CLICK_BLOCK && isItem(e.getItem())
&& SlimefunPlugin.getProtectionManager().hasPermission(e.getPlayer(),
block.getLocation(), ProtectableAction.BREAK_BLOCK)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Closing bracket and brace on new line

block.getLocation(), ProtectableAction.BREAK_BLOCK)) {
e.setCancelled(true);

SlimefunItem slimefunItem = BlockStorage.check(e.getClickedBlock());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Final


if ((block.getType() == Material.GLASS
|| block.getType().name().endsWith("_GLASS")
|| block.getType().name().endsWith("_PANE"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should really be _GLASS_PANE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wont work with clear glass pane @J3fftw1

Copy link
Owner

Choose a reason for hiding this comment

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

Already fixed it

@J3fftw1 J3fftw1 merged commit 83ea821 into J3fftw1:master Sep 7, 2020
J3fftw1 added a commit that referenced this pull request Sep 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants