Magic

Magic

190k Downloads

[REQUEST] Spell effects spectator list

Remodactyl opened this issue · 28 comments

commented

Spell effects currently have a parameter 'visibility', and its sound effects have a parameter 'sound_broadcast',

I propose that the spells maintain a list of spectator players (players who can perceive the spell) that is usable in the MagicAPI so that spell logic changing caster and target does not influence those players from being able to see the spell.
Through MagicAPI we could then call say 'spell.AddSpectator(Player player)' and 'spell.removeSpectator(Player player)'

the parameter visibility would have an additional option besides 'target' and 'caster', they could now also have the option 'spectators',
or optionally the spell keeps the same options and spectators will always see the spell effects by default.
sound_broadcast when turned false would then also play the sound to all spectators in addition to target and caster as it did before.

Why?
I am currently building a quest plugin that relies heavily on packets and ProtocolLib. When animations and cinematics in the quests play out for the players they are completely invisible to people who are not currently involved in the quest (this allows multiple people to play the same storyline at the same time in specific locations without interfering with eachother).
My NPCs can cast Magic plugin spells during these animations (in chase scenes or animated magic battles), however these spells are very difficult to get functioning while also being only visible to the player watching the cinematic. This is currently the final piece I have to solve in my animations.

This, for me, is a very technical reason for wanting the feature, but I can imagine it would have many uses for other developers if spectators is also accessible outside MagicAPI and through regular Magic commands or spell configuration, potentially allowing people to set up player filters for spells?

Finally, a big thank you for keeping this plugin updated and functioning!
The game I'm working on is benefitting amazingly from the features in this plugin.

commented

Darn that was fast!

commented

Hello again! Thank you for your kind words.

This seems doable so long as there is no performance impact (e.g. lots of map checks for every spawned particle), but I think as long as everything short-circuits when the spectator list is empty it should be fine, and hopefully easy to implement.

So let me make sure I have it straight- you are using the API to cast spells programmatically, like from NPCs or whatever other event or thing in quest.

You'd like to be able to manage a list of spectators per spell (cast?), any player on the spectator list will always hear the sounds and see the particles of those spells, even if broadcasting is off and visibility is not set to "all"?

commented

Yes, a way for me to modify the visibility filter of an instance of a spell being cast.
Example: I'll have say an NPC running down a hallway shooting missile|2 at a player who approaches, meanwhile a different player is not quite so far along in the quest, and they're still just talking to their version of that same NPC at the beginning of that hallway. The second player of course cannot see the effects of the first player's NPC.
(Preferably not get hit by that player's npc's spells either but I imagine that's possibly tougher to implement?)

I figured a spell having a list of 'spectators' will at least allow for any further implementations of such visibility to be easier to create. Any further features such as sounds being audible by that filter, or effects being visible for that filter are then based on that.

commented

I am currently using the CitizensAPI to create and manage my NPCs and that plugin has a tie-in with Magic. My plugin gives me a way to easily write dialogue scripts and animation scripts for NPCs and quests:

image

image

image

You don't need any of these images to back up this feature request, this is just me being overexcited about what we've already accomplished for the game :P

commented

That all looks really cool! I love the idea of integrating Magic with quests and cinematic events and such.

Ok, I implemented it but didn't have any way to test, so please let me know if it works!

Also let me know if you see any strange behavior, this took a little more change than I'd hoped (including to EffectLib)

If it's all working, please let me know that, too, I'll include it in the next release.

https://github.com/elBukkit/MagicPlugin/blob/master/MagicAPI/src/main/java/com/elmakers/mine/bukkit/api/spell/Spell.java#L146

I chose "setObservers" so as not to be confused with spectator mode.

You will need to update your plugin to use MagicAPI 10.8.12-SNAPSHOT which won't be available in the Maven common repo, not sure how you're referencing the API now but let me know if that's an issue and we'll work something out (later).

commented

I am currently using Maven, but I will give testing a go tonight! I'll let you know how everything works as soon as I can. I believe I downloaded the API jar in a local archive so I don't think Maven will be a problem? I'll keep you up to date!

commented

I just realised this might be more complicated on my side than I initially thought, since I am setting the magic spell casting traits through Citizens API at this moment, I'll likely need to rework that code to use MagicAPI directly or Citizens will have to implement something for it.
Unless the observers can already be configured with spell parameters like here:
image
This might take a while longer so I'll probably have to get back to you sunday or monday.

commented

Turns out, I made a very silly mistake.
I assumed the Magic integration with Citizens plugin was done by the Citizens developers. I asked them about potentially integrating the new feature into the MagicCitizensTrait, turns out that integration was also set up by you!
Would it be possible to quickly add a small extension of this new setObservers feature to the MagicCitizensTrait?
This would work something like:

npc.getOrAddTrait(MagicCitizensTrait.class).setObservers(players);

With that I can instantly test the feature tomorrow evening and it won't require a full rebuild of the spell casting actions.

commented

No problem and yea, I took a shot at downloading the source code on git to see if I could quickly add it in and test it out, but that was aiming slightly too high for one evening to set that all up correctly in IntellIJ :P
I did see that the NPC trait casts the spell using MagicAPI.cast which doesn't allow a list of observers yet, but perhaps an overload method with an extra parameter for the observer list will work too! Let me know when you've got the Trait set up, I'll then test it as soon as I can!
If you're interested we'd love to show off what we're using your plugin for later as well! You've actually joined us a couple of years ago when we were first making this game back in 1.11 ;)

commented

Sorry I haven't been able to get to this, but I think it should be doable just fine- assuming you want the NPC to use that observer list for each spell they cast.

I'll hopefully have some time for this tomorrow.

commented

It doesn't seem to be working yet, I am adding the target players to the MagicCitizensTrait observer, but other people can still witness the spell effects. Does it require setting visibility to observers in the effects or anything along those lines? Any kind of config changes I need to use before this works?

commented

The latest dev build has MagicCitizensTrait.setObservers(@Nonnull Collection<Player> players)

Let me know how it goes!

commented

I will try it at once!

commented

Implemented use of the feature, now to wait for one of our loyal test subjects to show up :)

commented

No, it shouldn't have required anything else, it ought to bypass any broadcast or visibility settings and just show effects to your list of observers.

Unfortunately I wasn't able to test any of it myself so it's probably a bug somewhere :(

I can try to make my own test plugin so I can do some debugging, but not sure when I can get to that.

commented

So instead of a test plugin I added some commands:

/mtrait observers <player name>
/mtrait observers none (will make it visible to no one)
/mtrait observers clear (will reset to default, no observer list)

And you can see the observer list with /mtrait, if set

In using these it seems to work- if I set myself as an observer, I see the effects. If I set observers to "none" I don't see them.

I don't have another player account to test with, but maybe you can use these commands (latest dev build?) to try and debug why it's not working for you ?

commented

I've tried the commands you've mentioned, they worked command-wise, and maintained an observer list for a magic npc successfully but all players could still witness spell effects, so the actual filtering afterwards does not appear to happen correctly. In my case, i tried with the basic missile spell, frost, push, and some custom spells i made before, both with observers set to none, undefined and specific players on the server. What i did notice is that occasionally one effect of the spell seems to be cancelled out, like a specific sound that isn't played. Could it be that not all effects in the spell are filtered through the observer list? (perhaps only the first the spell comes across?)

I am also open to working together on solving this issue through a discord call if necessary. Vacation is just starting so name a day and a time and I'll likely be available to help debug this problem in real time.

commented

I think I see the problem, I had only tested with the "day" spell.

But spells with repeating non-effectlib effects (like push) wouldn't work, only the first batch of effects uses the observers.

The PlayEffects action also wasn't covered (used by missile)

I've fixed those two cases (latest dev build) and tested with missile, push and frost. (Keep in mind block changes like from frost will be seen by everyone)

It seems fine now with those spells at least. It's possible I've missed other cases, let me know how it goes.

I will be trying to stay off the computer over the holidays so hopefully we can get this working before then :)

commented

Let me test the new version right away!

commented

That definitely made a huge difference using the commands already, I still see this particular effect by the looks of it:
image

The Helix effect with the portal particle. Perhaps there are still some more edge cases in here?
I'm gonna try out a ton of different spells and list some I find along the way, see if we can make this whole thing foolproof for weird edge cases.
I do get that block updates and entities altered/summoned cannot be hidden from other players. I am mainly looking to get at least particles and sounds to work under most circumstances! :)

commented

I should note that the very vast majority of spells successfully had their effects removed from running the spell through an npc with the mtrait commands!

commented

So far spells that still show me some effects:
Boom
chainlightning
homing
kill
music
neutron
sticky
tornado (cant change block damage but I can still use the spell in protected regions, tornado effect does still play)
torture

I certainly haven't tested all, mostly I've gone through a lot that have effects and don't edit blocks or use entities.
I expect most of these spells actually all use the same sort of action that still doesn't inherit the observer list.
Hope this helps!

commented

And finally I can now confirm that even with direct use by my plugin during cutscenes and animations, the spells are also using the same behaviour as with mtrait commands. So this feature is now almost fully working apart from just those spells I listed above! This is already very very useful! Let me know if you can fix the remaining spells, this might very well boil down to 1 single spell action.

commented

Are they just showing firework effects?

Unfortunately those can't be hidden since it's just spawning a firework entity .. I think I used to use packets but it was messy, it may be worth going back to that since per-player fireworks would be cool.

In the meantime, I think you have to avoid those.

Similarly, the PlayRecord action is special since it's faking a jukebox, playing a sound from a specific block in the world- all players will be able to hear that, too.

The AreaOfEffectCloud action (neutron) is also off-limits since it spawns an AOE entity.

So anything except for basic particle, effect lib and sound effects won't work with observers.

That all said, I found a bug that I think accounts for the others in your list, like torture, homing and sticky. Please try the latest dev build when you can 🤞

commented

Alright, yea I figured fireworks wouldn't work which is why I left flare off the list, AOE entities makes sense too, I think the number of spells covered by that bug fix probably accounts for most of them. I will try this shortly, and if this makes that difference I think we can consider the feature finished!

commented

Looks like all that are left are Boom (particles from fireball)
music (notes and sound)
and neutron (area of effect entity)
and all three of these will not be possible to fix considering how they work, so hereby I believe this issue is resolved!

commented

A big thank you for the swift work, when I have some time available I'll put up a little video with a result cutscene that I've been making to test out the new feature :)

commented

Through the last couple days, I've set up a cutscene while testing this new feature, this is the result!

Magic & Quests cutscene example
https://youtu.be/gzPoUFjY3fc

Pretty much all things happening in the cutscene in this video are invisible to players who are not viewing the same scene.
This includes barriers, monsters, magic spells, and NPCs.
This is a small fragment of the possibilities I have now thanks to this feature!