Compatibility w/ OldCombatMechanics

[lowercasebtw]

Is this compatible with the OldCombatMechanics plugin?

Side Note: I noticed that gravity was hardcoded in the math util class, why doesn't it use/take into count the server entity gravity attribute as gravity is a entity attribute now

[CASELOAD7000]

I have not tested the compatibility with the mentioned plugin, you're free to do so yourself. the gravity attribute edgecase is a good suggestion, although i'm unsure what pvp servers would change that

[Axionize]

I've added support for handling the gravity attribute in 1.20.5 and up.

#9

[TheobaldTheBird]

/tick rate is another interesting edge case, looks like Spigot API doesn't have anything for it yet though

[Axionize]

It wouldn't be that hard to add support for /tick rate but if we're going to do it we might as well do it properly, and doing it properly means accountings for tick skipping. This of course would be a nightmare until 1.21.2 is released with the changes in https://www.minecraft.net/en-us/article/minecraft-snapshot-24w36a and even then we would have to support older clients.

To properly account for that you need a latency compensated internal model of the world per player

This would also allows us to solve

• the delay in applying new gravity when the attribute changes, as noted in the comments in my PR
• account for https://bugs.mojang.com/browse/MC-69459 properly
• and the afformentioned /tick rate

But that point you might as well just make the entire plugin a GrimAC module...

[lowercasebtw]

Is the knockback code abstracted from the core of the plugin? If not, I think it should be allowing other plugins to change knockback if they want, because OldCombatMechanics has its own knockback (1.8 knockback) which could interfere with this code, but if it was abstracted, it would allow other plugins (OldCombatMechanics) to hook into it and modify the knockback logic, and or just addon plugins if (OldCombatMechanics) weren't to integrate it

[lowercasebtw]

^

[Axionize]

After some testing and digging around in code I can confirm OldCombatMechanics overwrites the effects of KnockBackSync.

We would need to reimplement OldCombatMechanic's legacy Knockback behaviour and add our corrections on top of it with a config option to enable old kb.

[lowercasebtw]

kernitus/BukkitOldCombatMechanics#771

[kernitus]

After some testing and digging around in code I can confirm OldCombatMechanics overwrites the effects of KnockBackSync.

We would need to reimplement OldCombatMechanic's legacy Knockback behaviour and add our corrections on top of it with a config option to enable old kb.

In what way is this the case? OCM listens to the PlayerVelocityEvent on lowest priority, meaning it comes in and sets the velocity first - it looks like it's knockback sync which is overwriting the velocity.

[Krymonota]

I just stumbled upon a similar problem and realized that KnockbackSync ignores all velocities set by previous event listeners of PlayerVelocityEvent. Instead of getting the velocity from the event (event.getVelocity()), which would take into account all velocities set by other plugins, KnockbackSync gets the velocity from the player (which is still the unchanged velocity as long as the event is not completed):

knockback-sync/bukkit/src/main/java/me/caseload/knockbacksync/listener/bukkit/BukkitPlayerKnockbackListener.java

Line 37 in f68d280

Vector vector = victim.getVelocity();

[Axionize]

I just stumbled upon a similar problem and realized that KnockbackSync ignores all velocities set by previous event listeners of PlayerVelocityEvent. Instead of getting the velocity from the event (event.getVelocity()), which would take into account all velocities set by other plugins, KnockbackSync gets the velocity from the player (which is still the unchanged velocity as long as the event is not completed):

knockback-sync/bukkit/src/main/java/me/caseload/knockbacksync/listener/bukkit/BukkitPlayerKnockbackListener.java

Line 37 in f68d280

Vector vector = victim.getVelocity();

I did notice this a few months ago but I didn't change it because the inclusion of this behavior predates me joining this project, and as I didn't know why this was done I saw no reason to change it. @CASELOAD7000 before is there any reason we're not grabbing the velocity from the event?

[lowercasebtw]

Any update?

[NotAlexNoyle]

Bump