Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

player-use-pertick-interpolated #119

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* This file is part of the Carpet TIS Addition project, licensed under the
* GNU Lesser General Public License v3.0
*
* Copyright (C) 2023 Fallen_Breath and contributors
*
* Carpet TIS Addition is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Carpet TIS Addition is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with Carpet TIS Addition. If not, see <https://www.gnu.org/licenses/>.
*/

package carpettisaddition.helpers.carpet.playerActionEnhanced;

public interface IEntityPlayerActionPackActionTypeUse
{
void setTickPart(float f);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* This file is part of the Carpet TIS Addition project, licensed under the
* GNU Lesser General Public License v3.0
*
* Copyright (C) 2023 Fallen_Breath and contributors
*
* Carpet TIS Addition is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Carpet TIS Addition is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with Carpet TIS Addition. If not, see <https://www.gnu.org/licenses/>.
*/

package carpettisaddition.helpers.carpet.playerActionEnhanced;

public interface IServerPlayerEntity
{
void pushOldPosRot();

void swapOldPosRot(boolean lastTickValues);

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

import carpet.helpers.EntityPlayerActionPack;
import carpettisaddition.helpers.carpet.playerActionEnhanced.IEntityPlayerActionPackAction;
import carpettisaddition.helpers.carpet.playerActionEnhanced.IEntityPlayerActionPackActionTypeUse;
import carpettisaddition.helpers.carpet.playerActionEnhanced.IServerPlayerEntity;
import carpettisaddition.helpers.carpet.playerActionEnhanced.randomly.gen.RandomGen;
import net.minecraft.server.network.ServerPlayerEntity;
import org.spongepowered.asm.mixin.Final;
Expand Down Expand Up @@ -90,15 +92,24 @@ private void perTickMultiplier(EntityPlayerActionPack actionPack, EntityPlayerAc
{
if (this.perTick != null)
{
ServerPlayerEntity player = ((EntityPlayerActionPackAccessor)actionPack).getPlayer();
IServerPlayerEntity iplayer = (IServerPlayerEntity)(Object)player;
EntityPlayerActionPackActionTypeAccessor typeAccessor = ((EntityPlayerActionPackActionTypeAccessor)(Object)type);
EntityPlayerActionPack.Action self = (EntityPlayerActionPack.Action)(Object)this;
IEntityPlayerActionPackActionTypeUse actionTypeUSE = (IEntityPlayerActionPackActionTypeUse)(Object)EntityPlayerActionPack.ActionType.USE;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added to ATTACK as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


iplayer.swapOldPosRot(true);

for (int i = 0; i < this.perTick - 1; i++)
{
EntityPlayerActionPackActionTypeAccessor typeAccessor = ((EntityPlayerActionPackActionTypeAccessor)(Object)type);
ServerPlayerEntity player = ((EntityPlayerActionPackAccessor)actionPack).getPlayer();
EntityPlayerActionPack.Action self = (EntityPlayerActionPack.Action)(Object)this;

actionTypeUSE.setTickPart((i + 1) / (float)perTick);
silnarm marked this conversation as resolved.
Show resolved Hide resolved
typeAccessor.invokeExecute(player, self);
typeAccessor.invokeInactiveTick(player, self);
}

actionTypeUSE.setTickPart(1.0f);
iplayer.swapOldPosRot(false);
iplayer.pushOldPosRot();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@Fallen-Breath Fallen-Breath Oct 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* This file is part of the Carpet TIS Addition project, licensed under the
* GNU Lesser General Public License v3.0
*
* Copyright (C) 2023 Fallen_Breath and contributors
*
* Carpet TIS Addition is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Carpet TIS Addition is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with Carpet TIS Addition. If not, see <https://www.gnu.org/licenses/>.
*/

package carpettisaddition.mixins.carpet.tweaks.command.playerActionEnhanced;

import carpet.helpers.EntityPlayerActionPack;
import carpettisaddition.helpers.carpet.playerActionEnhanced.IServerPlayerEntity;
import net.minecraft.server.network.ServerPlayerEntity;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;

@Mixin(EntityPlayerActionPack.ActionType.class)
public class EntityPlayerActionPackActionTypeMixin
{
@Inject(method = "start", at = @At("HEAD"), remap = false)
private void onStart(ServerPlayerEntity player, EntityPlayerActionPack.Action action, CallbackInfo info)
{
((IServerPlayerEntity)(Object)player).pushOldPosRot();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* This file is part of the Carpet TIS Addition project, licensed under the
* GNU Lesser General Public License v3.0
*
* Copyright (C) 2023 Fallen_Breath and contributors
*
* Carpet TIS Addition is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Carpet TIS Addition is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with Carpet TIS Addition. If not, see <https://www.gnu.org/licenses/>.
*/

package carpettisaddition.mixins.carpet.tweaks.command.playerActionEnhanced;

//#if MC > 11903
//$$ import carpet.script.utils.Tracer;
//#else
import carpet.helpers.Tracer;
//#endif
import carpettisaddition.helpers.carpet.playerActionEnhanced.IEntityPlayerActionPackActionTypeUse;
import net.minecraft.server.network.ServerPlayerEntity;
import net.minecraft.util.hit.HitResult;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Redirect;

@Mixin(targets = "carpet.helpers.EntityPlayerActionPack$ActionType$1")
public class EntityPlayerActionPackActionTypeUseMixin implements IEntityPlayerActionPackActionTypeUse
{
private float tickPart = 1.0f;

@Redirect(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Redirect is bad, try use @ModifyExpressionValue or @WrapOperation from mixin extra, or set require = 0 if you do need an @Redirect

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

method = "execute",
at = @At(
value = "INVOKE",
target = "Lcarpet/helpers/EntityPlayerActionPack;getTarget(Lnet/minecraft/server/network/ServerPlayerEntity;)Lnet/minecraft/util/hit/HitResult;",
remap = false
),
remap = false
)
private HitResult doRayTrace(ServerPlayerEntity player)
{
double reach = player.interactionManager.isCreative() ? 5 : 4.5f;
return Tracer.rayTrace(player, tickPart, reach, false);
}

@Override
public void setTickPart(float f)
{
tickPart = f;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* This file is part of the Carpet TIS Addition project, licensed under the
* GNU Lesser General Public License v3.0
*
* Copyright (C) 2023 Fallen_Breath and contributors
*
* Carpet TIS Addition is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Carpet TIS Addition is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with Carpet TIS Addition. If not, see <https://www.gnu.org/licenses/>.
*/

package carpettisaddition.mixins.command.player.pertick;

import carpettisaddition.helpers.carpet.playerActionEnhanced.IServerPlayerEntity;
import net.minecraft.server.network.ServerPlayerEntity;
import net.minecraft.util.math.Vec2f;
import net.minecraft.util.math.Vec3d;
import org.spongepowered.asm.mixin.Mixin;

@Mixin(ServerPlayerEntity.class)
public abstract class ServerPlayerEntityMixin implements IServerPlayerEntity
{
private Vec3d posLastTick;
private Vec2f rotLastTick;

public void pushOldPosRot()
{
ServerPlayerEntity self = (ServerPlayerEntity)(Object)this;
posLastTick = self.getPos();
rotLastTick = self.getRotationClient();
}

public void swapOldPosRot(boolean lastTickValues)
silnarm marked this conversation as resolved.
Show resolved Hide resolved
{
ServerPlayerEntity self = (ServerPlayerEntity)(Object)this;
if (lastTickValues) {
self.prevX = posLastTick.x;
self.prevY = posLastTick.y;
self.prevZ = posLastTick.z;
self.prevPitch = rotLastTick.x;
self.prevYaw = rotLastTick.y;
} else {
self.prevX = self.getPos().x;
self.prevY = self.getPos().y;
self.prevZ = self.getPos().z;
self.prevPitch = self.getPitch(1.0f);
self.prevYaw = self.getYaw(1.0f);
}
}
}
3 changes: 3 additions & 0 deletions src/main/resources/carpet-tis-addition.mixins.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
"carpet.tweaks.command.playerActionEnhanced.EntityPlayerActionPackActionAccessor",
"carpet.tweaks.command.playerActionEnhanced.EntityPlayerActionPackActionMixin",
"carpet.tweaks.command.playerActionEnhanced.EntityPlayerActionPackActionTypeAccessor",
"carpet.tweaks.command.playerActionEnhanced.EntityPlayerActionPackActionTypeMixin",
"carpet.tweaks.command.playerActionEnhanced.EntityPlayerActionPackActionTypeUseMixin",
"carpet.tweaks.command.playerActionEnhanced.PlayerCommandMixin",
"carpet.tweaks.command.spawnTrackingRestart.SpawnCommandMixin",
"carpet.tweaks.command.tickWarpMaximumDuration.TickCommandMixin",
Expand Down Expand Up @@ -150,6 +152,7 @@
"command.manipulate.entity.MobEntityAccessor",
"command.mobcapsLocal.SpawnCommandAccessor",
"command.mobcapsLocal.SpawnCommandMixin",
"command.player.pertick.ServerPlayerEntityMixin",
"command.raid.RaidAccessor",
"command.raid.RaidManagerAccessor",
"command.raid.RaidMixin",
Expand Down