elBukkit / MagicPlugin

A Bukkit plugin for spells, wands and other magic
http://mine.elmakers.com
MIT License
239 stars 148 forks source link

Crash Parameter Suggestions (RideEntity) #1289

Open NightScythe1 opened 8 months ago

NightScythe1 commented 8 months ago

Just a couple of suggestions I had for the crash parameters for the RideEntity action, they're quite niche for my usecase, and i'm unsure if they're possible, but any help is appreciated!-

1) crash_entity_speed / crash_entity_dismount_speed At the moment, there are 'entity' versions of most of the crash parameters, but these two are absent. Currently this is slightly abuseable in my usecase, as players are crashing whenever an entity is near them at the lowest speeds. This also means they can very slowly walk/ride towards mobs or hordes of mobs and continuously push them away and damage them.

2) crash_cooldown / crash_entity_cooldown Another slight problem is that crash effects seem to spam/repeat constantly if the player is stuck against/inside a block or entity. This can result in deafening noise spam and particles if there are crash effects enabled. It would be handy to have a parameter where we can set a cooldown, where players cannot crash again within this timeframe.

NathanWolf commented 8 months ago

crash_entity_speed and crash_entity_dismount_speed added for next dev build.

Unlike crash_dismount_speed, the behavior for crash_entity_dismount_speed is to not dismount by default, to match current behavior.

NathanWolf commented 8 months ago

Also adding the cooldown parameters :)

I wasn't able to test any of this, so please let me know if it's not working.

NightScythe1 commented 8 months ago

Thanks for adding these! Just gave them all a quick test- crash_entity_speed and crash_cooldown both seem to work exactly as intended! crash_entity_dismount_speed does technically work, but it seems to trigger and dismount you as soon as you hit the speed determined within this parameter. I'm wondering if it's because it's accidentally targeting the mount or rider? crash_entity_cooldown unfortunately doesn't seem to work, and it's still possible to creep forward at a mob continuously damaging/crashing into it.

A couple other things as well- could crash_entity_braking potentially be another parameter? That way players can configure whether motion stops when crashing into blocks or entities independently & the Velocity action doesn't seem to work on a player that is currently using RideEntity anymore- It used to on one of my old dragon spells in HTTYD, but it will no longer apply velocity if the player is moving with that spell. It will however, apply velocity if the player is using RideEntity but is currently stationary/not moving.

NightScythe1 commented 8 months ago

Oh, I did have one more request, sorry to ask for so much! Could we get a jump_cast_cooldown as well? Currently there is no buffer/click cooldown for when the player presses space, so they are immediately spammed with the spell's cooldown sound effect and message every tick if they tap or hold spacebar for longer than a millisecond.

NightScythe1 commented 8 months ago

Another update- unfortunately, crash_speed doesn't seem to be working for me anymore, and no matter the value it is set to, crash effects will run on a collision with a block. crash_dismount_speed also seems to be working differently, as now you will be dismounted immediately at 0 speed if the value is not manually set to something higher.

When removing all crash parameters from a RideEntity spell, it seems the player will immediately crash and dismount if the Entity starts the spell on a solid block. This can be circumvented by setting crash_dismount_speed to a value over 0, but crash effects will still run whenever the player collides with a block at any speed no matter what crash_speed is set to (but this does still respect crash_cooldown)

NathanWolf commented 8 months ago

Oof- so sounds like I broke something? And then released it... :sob:

NightScythe1 commented 8 months ago

Potentially, although I'm a little confused as this happened upon updating magic from the first crash parameters dev built to the new full release 😅 It seemed normal when I was on Paper 1.20.1 and the dev build, but I only noticed this behaviour upon updating to Paper 1.20.4 and using the full 10.8.12 Magic release... Though I may have simply not noticed it prior to updating...

NathanWolf commented 8 months ago

Oh, I did have one more request, sorry to ask for so much! Could we get a jump_cast_cooldown as well? Currently there is no buffer/click cooldown for when the player presses space, so they are immediately spammed with the spell's cooldown sound effect and message every tick if they tap or hold spacebar for longer than a millisecond.

I think for this one you could/should just remove cooldown FX for such spells- is that an ok workaround? I don't really want to track separate cooldowns for different triggers, feels like a slippery slope.

I am looking at all the problems with crashing you've described but can't see what I broke in the code. I am getting tempted to revert all of the changes if I can't figure it out quickly but I will keep you updated.

NightScythe1 commented 8 months ago

That's understandable, no worries! Removing cooldown fx should work fine for that :) I'm not 100% sure what could have broken/changed either with the crash triggering, I see you pushed a new dev build so I'll give that a couple tests to see if anything changed! If not, no worries about reverting it- I doubt the addition was getting much use outside of my own request anyway :sweat_smile:

NathanWolf commented 8 months ago

Well I was worried I completely broke vehicles or crashing, but I tested broomsticks and cars and they seem ok.

I did find a couple of issues with the new parameters, that's what the dev build was about- but nothing I expect to fix the stuff you're seeing.

If you still have trouble and can provide a simplified test case that'd help a lot!

NightScythe1 commented 8 months ago

Just checked my server with the new build and still getting the same result unfortunately... I compiled a simple spell and mob that use the same configs/parameters as the actual ones I use, as you said. Hopefully that might help narrow it down! (Had to change filetype from yml to txt to attach) (the mob has a lot of stuff that can be safely removed/ignored i think, I just put it there as it's how the spell is usually activated)

crash_testmob.txt crash_testspell.txt

NightScythe1 commented 7 months ago

Update to this: I fear the release may have broken brooms for a few people. We've had a few reports in Discord of brooms no longer working in 10.8.12 - Perhaps reverting the change may be necessary :(

NathanWolf commented 7 months ago

:( I'll try and find some time to look at this today. I really don't know what could've gotten broken (just reviewing the code several times), and they're working fine on my demo server.

NathanWolf commented 7 months ago

Oh I think that issue from Discord is a different problem, fortunately. That's a general problem summoning entities from an internal Spigot change.

NathanWolf commented 7 months ago

I was able to reproduce the "instant dismount" behavior with the configs you provided.

But, interestingly, the problem is the same in build#4253 which doesn't have any of the new parameters or other changes in it.

So I'm not sure what's going on here- it seems like the spell thinks the target mob is inside of the block underneath it, which is why there is an instant-crash.

Do you normally use parrots as mounts without issue? Not sure if it could be something specific to them ...

I'll keep testing and see if I can figure out what's going on, but I'm hoping this means all the new parameters may be working ok.

NightScythe1 commented 7 months ago

That's super weird... In that case I honestly can't think of anything that might've changed other than me tinkering with my own configs... I've been using parrots as the base for all my dragons since around May last year with no issues, and the instant crashes sort of blindsided me as I was testing these new parameters- so it probably is something within those configs that's tripping me up

NathanWolf commented 7 months ago

Huh that is weird! I can't think of how it would've been avoiding the crash before. Possibly something in a Minecraft/Spigot update?

What I'm seeing in the debugger is that mounting a parrot causes the player to drop down so their Y-coordinate is underground. This causes an instant-crash.

What I can offer:

  1. I could add a parameter to bypass this check, but I would caution against that. There are a lot checks in there meant to prevent exploits, like players glitching through walls or ceiling.
  2. I was able to fix it by teleporting the parrot up one block before mounting. This required some tweaks to the Teleport action (so new dev build) and these changes:
    cast:
    - class: Cancel
    - class: ChangeContext
      target_offset: 0,1,0
      actions:
      - class: Teleport
        teleport_target: true
        direct: true
      - class: RideEntity
NightScythe1 commented 7 months ago

Thanks so much for providing this fix!! Teleporting the parrot definitely seems like the best bet- I'll give the new dev build and teleportation a try now! (I'll also check if the constant crash effects persist)

NightScythe1 commented 7 months ago

Upon testing the new build- The teleportation works great and allows the player to get off the ground! However, if the player touches a block they are still immediately dismounted if crash_dismount_speed is not set. Could this be defaulting to 0 and causing crashes at speeds of 0 and above? I'm unsure if this was already the default behavior though, so may be nothing to worry about... crash_speed still doesn't seem to be working properly though unfortunately, the behavior is similar to crash_dismount_speed where it appears to be defaulting to 0 and triggering at any/all speeds, but unlike crahs_dismount_speed this doesn't seem to be chanegable/fixable by simply setting the parameter manually, and giving the parameter a value doesn't seem to have any effect at all