search

Civcraft / JukeAlert

Do not open issues here; open them on the maintained fork @ DevotedMC
https://github.com/DevotedMC/JukeAlert
BSD 3-Clause "New" or "Revised" License
5 stars 15 forks source link

Teleporting now triggers snitches #48

Closed TealNerd closed 8 years ago

TealNerd commented 8 years ago

Previously teleporting into a snitch field (such as exiting a minecart and teleporting up because of humbug) did not trigger snitches. Now it will.

ttk2 commented 8 years ago

good idea, does this just catch the teleport event?

TealNerd commented 8 years ago

Yeah, uses the same logic as the player move event basically just on teleport instead

ttk2 commented 8 years ago

thoughts @rourke750 ?

TealNerd commented 8 years ago

@ttk2 this also has #49 in it as well if that's ok

ttk2 commented 8 years ago

might not be, we just need to complete discussion on both before merging.

TealNerd commented 8 years ago

Ok I can always revert one or the other

ProgrammerDan commented 8 years ago

Alright, two thoughts.

1) definitely pull #49 into its own pull request

2) Can we just directly fire the existing movement code, instead of recreating it here? Now we just have a double-code liability where if we need to change movement listener we have to remember to change it here too.

Otherwise, didn't even know this was an issue but great catch! Thanks for tackling :)

TealNerd commented 8 years ago

Yeah that's a good point, I fixed it.

ProgrammerDan commented 8 years ago

Awesome, looks good.

rourke750 commented 8 years ago

@ProgrammerDan Want to handle this?

TealNerd commented 8 years ago

@rourke750 merge so i can do the block break thing?

rourke750 commented 8 years ago

Any idea why all those lines got modified?

TealNerd commented 8 years ago

I think it was github getting confused about line numbers or something idk