player-use-pertick-interpolated#119
Conversation
silnarm
commented
Oct 14, 2023
- add player position/rotation interpolation to perTick use action
* add player position/rotation interpolation to perTick use action
|
perTick test 1_14_4 (read-only).zip
|
| IServerPlayerEntity iplayer = (IServerPlayerEntity)(Object)player; | ||
| EntityPlayerActionPackActionTypeAccessor typeAccessor = ((EntityPlayerActionPackActionTypeAccessor)(Object)type); | ||
| EntityPlayerActionPack.Action self = (EntityPlayerActionPack.Action)(Object)this; | ||
| IEntityPlayerActionPackActionTypeUse actionTypeUSE = (IEntityPlayerActionPackActionTypeUse)(Object)EntityPlayerActionPack.ActionType.USE; |
There was a problem hiding this comment.
Does it only support "USE" action pack? imo it's much better if all action packs support interpolation
at the same time, and them will have the same behavior
There was a problem hiding this comment.
You can use the global flag suggestion described in 2. of #119 (comment), so you don't need to mixin into every action implemetation that invokes EntityPlayerActionPack#getTarget
| { | ||
| private float tickPart = 1.0f; | ||
|
|
||
| @Redirect( |
There was a problem hiding this comment.
@Redirect is bad, try use @ModifyExpressionValue or @WrapOperation from mixin extra, or set require = 0 if you do need an @Redirect
There was a problem hiding this comment.
Yes, bad I know. It is carpet code at least, not MC. Added the require = 0, but I was unable to get rid of it, @ModifyExpressionValue gave an unhelpful error message, but I assume the static function is the problem.
|
Some suggestions:
|
|
|
||
| actionTypeUSE.setTickPart(1.0f); | ||
| iplayer.swapOldPosRot(false); | ||
| iplayer.pushOldPosRot(); |
There was a problem hiding this comment.
So here's where you record the rotation and position of the player:
- when the action starts
- when the action completes an execution
You're assuming that the player moves smoothly between 2 positions, but this assution might be wrong. The player might be teleported, and teleporation do not have interpolation
There was a problem hiding this comment.
This is true. I had largely discounted it as an issue originally because player reach meant the use attempts would do nothing anyway, but it will potentially load chunks which does suck.
Will have a think about it, seems like it will be a few more injects at least though, I'm not convinced it is worth handling this explicitly.
There was a problem hiding this comment.
imo, it's important to ensure interpolation only applies in the correct scenarios. As you mentioned, interpolating in unexpected scenarios can cause problems, especially since carpet doesn't provide fine-granular permission system for these commands
and that's why the interpolation feature is hard to implement
* remove static in ActionType.USE, move 'tickPart' to ActionPack. * move last tick pos/rot into Action * dupe USE mixin for ATTACK * change some names