Byteflux / CombatTagPlus

Stop those dirty combat loggers!
17 stars 72 forks source link

Teleporting issue #85

Closed christiaanbruijnes closed 7 years ago

christiaanbruijnes commented 8 years ago

The operator requested to reopen an issue if the previous issue hasn't been fixed which is the case now.

I am using the latest dev build and the issue is the following:

While a player is in combat you can't teleport them away with any plugin (Skript, Essentials etc..).

ProgrammerDan commented 8 years ago

This is a configuration option now: https://github.com/MinelinkNetwork/CombatTagPlus/blob/master/CombatTagPlus/src/main/java/net/minelink/ctplus/Settings.java#L418

Simply add untag-on-plugin-teleport: true to your config.

christiaanbruijnes commented 8 years ago

They added that option because of my previous issue, however this didn't fixed it. That is why I openend this issue.

That config option doesn't work and that is the exact problem.

ProgrammerDan commented 8 years ago

So, initially it was added and wasn't behind a config option. This broke my plugin teleports (World Border, etc. :D ) and created PVP exploits (easy way to untag self).

I mention just in case -- always worth doublechecking the config first as you might not have caught that it's now behind a config option instead of default on.

Back to the issue -- none of the plugins you use can teleport a player while in Combat? Is there a stack trace on your console or any other indication of failure? Or simply -- nothing -- no action, no error?

Techcable commented 8 years ago

Does any plugin work at all, or is it only certain comands? Does vanilla work?

christiaanbruijnes commented 8 years ago

I tested this on an empty server with newest Spigot and CTP.

I tried to teleport the player away in combat with Essentials and with Skript, but simply nothing. No action, no error!

Everything works and everything is good expect that I can't teleport a player away while in combat which is needed for some cases (example, in a duel.. I want to teleport them to spawn).

Techcable commented 8 years ago

Does it work with vanilla commands?

christiaanbruijnes commented 8 years ago

Which vanilla command?

Techcable commented 8 years ago

Try any

christiaanbruijnes commented 7 years ago

I tried /minecraft:tp Chris_GSPvP and that worked!

Techcable commented 7 years ago

So only the vanilla commands work, but not plugin ones? Could you please gist your config file?

christiaanbruijnes commented 7 years ago

Correct.

https://gist.github.com/chrisbruijnes/187d23da6c7fe86ac4bf93d7a2bbc4cf

I already tried the untag-on-plugin-teleport option.

christiaanbruijnes commented 7 years ago

I have found the issue on file listener > PlayerListener on line 286.

https://gyazo.com/3c74b56294b24ca26f6753387fc652e2

The CASE: PLUGIN is empty and should be filled with the same code below.

christiaanbruijnes commented 7 years ago

However, that won't fix the MAIN issue though, the server itself should be able to teleport players away while in combat.

ProgrammerDan commented 7 years ago

@SpiroMarshes CASE are fall-through -- since there is no break; on the PLUGIN case, it also executes the same code as in UNKNOWN. Your patch is unnecessary as it doesn't change anything, the code is already getting run.

@chrisbruijnes This is the code I pointed you to several months back, as it's tied to the untag-on-plugin-teleport option.

If different behavior is needed between UNKNOWN and PLUGIN, then perhaps another configuration option needs adding. I'm still not clear why the existing code doesn't behave the way you're wanting.

christiaanbruijnes commented 7 years ago

That config option doesn't work, it is simple: You can't teleport people away while they are in combat. This is a really urgent requirement for most servers.

I've tried everything you told me to do, nothing works. Please find a proper solution for this.

ExoticCoding commented 7 years ago

@ProgrammerDan @chrisbruijnes brought up a good point by bringing up the "Case" issue but touched on the wrong part of it, he's using essentials to run the teleport however when essentials teleports the player it runs them with the enum cause "Command" which was not added so you should add in the "Command" as another fallthrough cause along with "Plugin" https://hub.spigotmc.org/javadocs/bukkit/org/bukkit/event/player/PlayerTeleportEvent.TeleportCause.html#COMMAND

I've tested the response from running the tp command with Essentials and it responded with "COMMAND", which is how I know how to fix this, just by the way.

It's a super simple 1 line fix, I have no idea how to commit to fixes but here's the code which would fix it: http://hastebin.com/ozegexinow.java

christiaanbruijnes commented 7 years ago

Awesome! @ProgrammerDan Can you add "case COMMAND:" into the newest version and push it to the newest dev build?

ProgrammerDan commented 7 years ago

Someone else will probably beat me to it, but sure, that's something easily contributed in a PR -- thanks @ExoticCoding for pointing out the issue, wasn't aware a CASE was missing, good catch; it'd be bound by default behavior.

christiaanbruijnes commented 7 years ago

Will this be added/fixed in a new dev build? If so, when?

Waiting for this for months now :D

ExoticCoding commented 7 years ago

@ProgrammerDan I'm not really sure how to make PRs, I also don't think I can due to my internet limitations but if I could do it, I would.

ProgrammerDan commented 7 years ago

@chrisbruijnes http://ci.minelink.net/job/CombatTagPlus/ -- Build is live!

christiaanbruijnes commented 7 years ago

The bug hasn't been fixed, however we are getting closer!

What I am trying to do is: /spawn (while he is in combat) What happens: I am getting teleported away to spawn for a second and then I get teleported back. How is this possible?: I've came to the conclusion that teleporting to spawn doesn't work since it is a region (with PvP deny, not sure if this counts), I tried to teleport myself to a player outside spawn and it worked, however it doesn't work when I teleport to the player while he is in spawn. This means it has something to do with the region.

If the config value "untag-on-plugin-teleport" just untags the player first before teleporting, it will be fixed I guess.

Maybe because of the "setCancelled(true)": https://gyazo.com/8830408501f5e6d8e3ac486490e00323

ProgrammerDan commented 7 years ago

If the config value "untag-on-plugin-teleport" just untags the player first before teleporting, it will be fixed I guess.

Indeed that is what it does. Assuming untag-on-plugin-teleport is set to true, the setCancelled(true) you point to will not execute, when using /spawn or other essentials teleport commands.

christiaanbruijnes commented 7 years ago

How come that it doesn't work then?

Oh by the way, is another restart needed for that config value to take affect? Because I used /ctreload for that one.

ProgrammerDan commented 7 years ago

It sounds like something else is reversing the teleport -- not cancelling it. A cancelled teleport never happens, whereas what you are describing is a reversal -- you arrive, only to be sent back -- and that implies some other plugin at work.

/ctreload

I am not certain, if doubt remains concerning its activation, restart to be sure.

christiaanbruijnes commented 7 years ago

Okay, I did some research and tested some couple things and came to the following conclusion:

I deleted all of my plugins except Worldguard and the bug was still here, removing WorldGuard did fix the bug. The cause of the bug is the flag "pvp: deny", that stops you from teleporting because there is a PvP zone there.

Hope this makes things more clear and it can be fixed.

(The restarting didn't fixed it by the way).

ProgrammerDan commented 7 years ago

Based on your investigation, I think we've "fixed" the issue as much as possible within CTP; if worldguard is preventing behavior that CTP now supports, you will probably need to open a companion issue with WorldGuard to see if they can help you out.

christiaanbruijnes commented 7 years ago

Isn't there a way to just force the teleport? There must be a solid way of fixing this issue, since a lot of people will be teleporting a player away into a No-PvP zone.

Let's be honest, Worldguard is not going to spend their time to fix small issues like this, when this is a major problem for CTP.

Aren't there other way-arounds to fix this issue?

ProgrammerDan commented 7 years ago

Well, regardless, we are at a limit of my knowledge. I am entirely unfamiliar with WorldGuard -- perhaps someone else knows it better and could propose a solution.

Good luck :)

To anyone with actual familiarity with WorldGuard, perhaps the hook needs updating: https://github.com/MinelinkNetwork/CombatTagPlus/tree/master/CombatTagPlusWG-v6

christiaanbruijnes commented 7 years ago

Is there anyone who might have a proper solution? I am waiting for this for months now.

The issue right now: I can't teleport players away in combat to a Non-PvP zone.

christiaanbruijnes commented 7 years ago

Re-opn this please @Techcable

ghost commented 7 years ago

Will this be ever fixed?

christiaanbruijnes commented 7 years ago

UPDATE:

I've finally found a way to fix this by enabling disable-teleportation, and it works now!

However, because of this we have another issue that people get untagged because of plugins teleporting them (https://github.com/MinelinkNetwork/CombatTagPlus/commit/b7ceaf9579a4a4f8ea90489418fadda27297133a)

Can you only add the case for Essentials and remove the others? A proper and solid way of fixing this issue is by adding an untag or teleport command.

@Techcable @ProgrammerDan @ExoticCoding

ghost commented 7 years ago

@chrisbruijnes Thank you so much for that :p

ghost commented 7 years ago

But why does it block teleportation when it's disabled? That's the real issue.

christiaanbruijnes commented 7 years ago

Using the function to untag on teleport is not smart to use right now, there are appearing many new bugs.. I just prefer a new command to untag or teleport to solve all issues. How hard could it be?

ghost commented 7 years ago

Any update on this??

christiaanbruijnes commented 7 years ago

Still nothing, I guess the smartest solution to this is an untag command: /untag .

ghost commented 7 years ago

@Techcable @Byteflux @ProgrammerDan @ExoticCoding

Why does the plugin stop teleporting, even with the option disabled in the config anyways?

Byteflux commented 7 years ago

It doesn't. I personally use it on 2 different servers with the option disabled.

christiaanbruijnes commented 7 years ago

Whenever you teleport someone to a non-pvp zone, it won't work.

(if you have teleportion disabled and the untag value).

That's why a /untag command will be a fix to all issues.

christiaanbruijnes commented 7 years ago

@ProgrammerDan @Techcable @Byteflux @ExoticCoding

Can you commit the following code? It is an untag command, can you also register it in the config.yml file?

http://pastebin.com/nvJCsZPK

christiaanbruijnes commented 7 years ago

@Techcable Thanks a ton, maybe add a second delay after this, so they can't get tagged after the command got executed for 1 second?

This because of I will first untag them with a command and then teleport them away, but what if someone hits them on the exact same time?

christiaanbruijnes commented 7 years ago

I tried to commit a fix, however I do not have enough permissions.

All what is needed is the following: https://gyazo.com/bb7a625d5e9f5bf829a04a2ef31fcf98

Can one of you PLEASE commit that? @ProgrammerDan @Byteflux @Techcable

Bonista commented 7 years ago

@chrisbruijnes just fork the repo and do it yourself

christiaanbruijnes commented 7 years ago

Okay, I've made the fix: https://github.com/MinelinkNetwork/CombatTagPlus/pull/100

This will fix bug #99 and the bugs from above, maybe a second delay should be added aswell (see 2 comments above).

@Byteflux @Techcable