Suggested by @Dr_Blamo, if the plugin does not already have an option for this (need to check, honestly don't remember) it would be good for spells to not target vanished players.

Yes, I don't think that there is an option allowing to do that currently.

I remember looking for this in the past in magic but not finding it.
Anyways, depending on what plugins should be supported, all it would take is something like this:

boolean isVanished() {
  for (MetadataValue meta : p.getMetadata("vanished")) {
    return meta.asBoolean();
  }
  return false;
}

I just haven't gotten around to do this myself. (As I'm not really confident with this part of the codebase it would probably be on the "eventually" time schedule ;). )

Thanks for the tip!

Odd, I thought vanish was part of the Spigot API for some reason...

The most recent dev build has a target_vanished parameter for spells. It is false by default, meaning spells will no longer target vanished players.

Untested, though, please let me know if you find any problems.

That's true, but it would not work for non-player mages as it requires a player to observe from.
Example: caster.getPlayer().canSee(target.getPlayer())
API: https://hub.spigotmc.org/javadocs/spigot/org/bukkit/entity/Player.html#canSee(org.bukkit.entity.Player)

Ah, yes, that's what I was thinking of! Thanks.

I used the metadata approach you (and mbaxter) suggested, I'm hoping Essentials complies. I kind of wish I could just check for metadata and assume it's true if it's there, would be much more efficient.

Also slightly off topic as I was looking at your patch:
Why have if (entity == null) return false;?
Wouldn't it make more sense to just let it blow up in your face so you know there is something wrong early on? (Before it becomes a problem)

Hm .. good question .. I think I'm in that habit to account for non-entity mages (Automata, Elementals) but not really sure if it'd make it this far anyway.

Personally I would just annotate the API with @Nonnull and call it undefined behaviour ;)

@NathanWolf The last problem is that I'm still on 1.8.8 version.

Ha ... wah-wahhhhhh

So, that is going to be a big problem. I'm not sure I can backport anything anymore, the master branch has diverged too far from the 1.8 branch. (killme, sometimes I still semi-regret the multi-module change! It is mostly nice though...)

I will see about re-doing that change by hand at some point, but honestly the 1.8 build is not going to get much attention anymore.

The problem is that I can't upgrade to 1.9.
Some plugins are outdated and not compatible with 1.9 ...
So thanks if you can backport it.

I'll see what I can do, but you have to realize that 1.8 is over 2 years old and at some point I'm really not going to support it anymore.

@NathanWolf Thanks and sorry for this inconvenience.

So it's not possible ? :/

No it's possible. It's just a pain. The 1.8 branch has diverged, meaning any changes I make to the current version I have to redo basically by hand (I could maybe use patches...) if I want to backport them to 1.8.

So I don't really want to backport anything to the 1.8 version anymore, or really touch it at all. It's like super duper dead to me :D

That said, try this dev build please, spells should no longer target vanished players:

http://jenkins.elmakers.com/job/MagicPlugin-1.8/42/com.elmakers.mine.bukkit.plugins$Magic/

It seems to work for us (AMC) on the master version.
We're using Vanish No Packet though (it does set the meta data https://github.com/mbax/VanishNoPacket/blob/6d8d91c1aad959dc16fb585cbf5467a37d7bec39/src/main/java/org/kitteh/vanish/VanishPlugin.java#L334 ).
What plugin are you using for vanishing?
As mentioned before player.canSee won't work for non-player mages.

I'm using Essentials (EssentialsX).

@NathanWolf The spells are still target vanished players with the last version.
Is there a configuration ?

Doh- no there is not, it's supposed to be off by default (meaning spells don't target vanished players)

Honestly though I've no idea if this technique actually works- it relies on Essentials (or whatever) setting some metadata on a vanished player, and I don't actually know if it does that. Is it Essentials you're using for vanish?

Unfortunately this might be really hard to me to test out, I'll have to get a second computer and account... or maybe I can rig something up with cmd blocks to target me. In any event if it's not working I may not have a quick fix for you.

You're using this build, right?

http://jenkins.elmakers.com/job/MagicPlugin-1.8/42/com.elmakers.mine.bukkit.plugins$Magic/

Oh 🤒
Do you tried : player.canSee(Player) ?
Yes I'm using this build.

=> I can help you for your test if you want.

EssentialsX in does not actually set any meta data :( (https://github.com/drtshock/Essentials/blob/db47460a7949c5686222b16feb3c5f4be2a8376f/Essentials/src/com/earth2me/essentials/User.java#L684). Perhaps consider requesting them to add that? It's just a two line change,

Ok, @Blamo27 I've added an additional check using canSee, this will only work for player-to-player targeting. I am keeping this only in the 1.8 branch for now:

http://jenkins.elmakers.com/job/MagicPlugin-1.8/43/com.elmakers.mine.bukkit.plugins$Magic/

@killme Out of curiosity do you know if VanishNoPacket removes the "vanished" metadata when a player un-vanishes? Or does it leave it on there but set it to false? I think if I could use a hasMetadata call it'd be a lot more efficient. I'm a little unhappy about having to iterate over metadata for every potential target.

It uses a non-cached lazily computed meta data value (VanishPlugin.java / VanishCheck.java).
So, it's more or less a getter rather than a fixed value. Iterating will therefor always be needed.

Assuming there is only a constant amount of vanished meta values, iteration should not be asymptotically slower though. It would basically be: !HashMap#get().isEmpty() vs for(meta : HashMap#get()) { (boolean) meta.get(); }