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.

Sign up for GitHub

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

Jump to bottom

ReplayMod Compat #317

Merged
merged 1 commit into from
Jan 21, 2024
Merged

ReplayMod Compat #317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 6 additions & 4 deletions common/src/main/java/net/bettercombat/client/ClientNetwork.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@ public static void initializeHandlers() {
final var packet = Packets.AttackAnimation.read(buf);
client.execute(() -> {
var entity = client.world.getEntityById(packet.playerId());
if (entity instanceof PlayerEntity) {
//This addition check is not required, ive tested without it,
//but it probably retriggers the animation and could lead to issues on servers with higher pings
if (entity instanceof PlayerEntity player && player != client.player) {
if (packet.animationName().equals(Packets.AttackAnimation.StopSymbol)) {
((PlayerAttackAnimatable)entity).stopAttackAnimation(packet.length());
((PlayerAttackAnimatable) entity).stopAttackAnimation(packet.length());
} else {
((PlayerAttackAnimatable)entity).playAttackAnimation(packet.animationName(), packet.animatedHand(), packet.length(), packet.upswing());
((PlayerAttackAnimatable) entity).playAttackAnimation(packet.animationName(), packet.animatedHand(), packet.length(), packet.upswing());
}
}
});
Expand All @@ -36,7 +38,7 @@ public static void initializeHandlers() {

var soundEvent = Registries.SOUND_EVENT.get(new Identifier(packet.soundId()));
var configVolume = BetterCombatClient.config.weaponSwingSoundVolume;
var volume = packet.volume() * ((float)Math.min(Math.max(configVolume, 0), 100) / 100F);
var volume = packet.volume() * ((float) Math.min(Math.max(configVolume, 0), 100) / 100F);
client.world.playSound(
packet.x(),
packet.y(),
Expand Down
12 changes: 11 additions & 1 deletion common/src/main/java/net/bettercombat/network/ServerNetwork.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import net.minecraft.entity.attribute.EntityAttributeModifier;
import net.minecraft.entity.attribute.EntityAttributes;
import net.minecraft.entity.decoration.ArmorStandEntity;
import net.minecraft.entity.player.PlayerEntity;
import net.minecraft.entity.projectile.PersistentProjectileEntity;
import net.minecraft.item.SwordItem;
import net.minecraft.network.PacketByteBuf;
Expand All @@ -37,6 +38,7 @@
import net.minecraft.text.Text;
import org.slf4j.Logger;

import java.util.Collection;
import java.util.UUID;

public class ServerNetwork {
Expand All @@ -63,9 +65,17 @@ public static void initializeHandlers() {
}
final var packet = Packets.AttackAnimation.read(buf);
final var forwardBuffer = new Packets.AttackAnimation(player.getId(), packet.animatedHand(), packet.animationName(), packet.length(), packet.upswing()).write();
try {
//send info back for Replaymod Compat
if (ServerPlayNetworking.canSend(player, Packets.AttackAnimation.ID)) {
ServerPlayNetworking.send(player, Packets.AttackAnimation.ID, forwardBuffer);
}
} catch (Exception e){
e.printStackTrace();
}
Copy link
Owner

@ZsoltMolnarrr ZsoltMolnarrr Jan 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

As far as I understand, the changes in this file are the ones to truly fix the problem. Specifically Line 68-75.
Would it be possible to put this into an mod install check, and still have it work in every scenario?
Like

if(Platform.isModInstalled("replay-mod-id") {
   ...
}

Copy link
Author

Choose a reason for hiding this comment

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

No. the way Replay mod works is by rolling back the packets send to the client by the server.
Checking if Replay Mod is installed on the server is kinda pointless, it would only fix the issue in singleplayer.

Copy link
Owner

@ZsoltMolnarrr ZsoltMolnarrr Jan 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Okay.
What I am worried about is: network delay ping pong causing the attacking player's attack animation to played twice, with ping pong delay, resulting in visual glitch.

I understand that this packet sending cannot be conditional.

Can we make the client side interpretation of this packet conditional (based on ReplayMod installed)?

Copy link
Author

@Smartin-b Smartin-b Jan 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

we could, but i see no real purpose.
I changed the logic from the server refuses to send the client the packet about the animations
to the client ignoring the packet that regard themselfs so the animation is not played twice
line 19-21 in ClientNetwork

Copy link
Owner

Choose a reason for hiding this comment

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

That seems like a good change, is the compatibility still satisfied with this code?

Copy link
Author

Choose a reason for hiding this comment

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

it is, sorry for the late response, i didnt change anyhting, that was in the PR from the start

Copy link
Author

Choose a reason for hiding this comment

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

i should also mention #323 does pretty much exactly the same thing, this PR just has a comment and surrounds the netcode with a try/catch block since BC does that aswell in the loop

PlayerLookup.tracking(player).forEach(serverPlayer -> {
try {
if (serverPlayer.getId() != player.getId() && ServerPlayNetworking.canSend(serverPlayer, Packets.AttackAnimation.ID)) {
if (ServerPlayNetworking.canSend(serverPlayer, Packets.AttackAnimation.ID)) {
ServerPlayNetworking.send(serverPlayer, Packets.AttackAnimation.ID, forwardBuffer);
}
} catch (Exception e){
Expand Down