[Forge 1.18.1] Prevent sending of packets too quickly from client to server

[cyanog3n]

I have a block that allows you to store XP in the form of a fluid and retrieve it when needed via a GUI. When the buttons are clicked, I send a packet to the server updating the player's XP level as well as the fluid level in the block.

It works perfectly most of the time, but I've noticed an case in which the XP is double-counted when the buttons are clicked very rapidly or under very laggy conditions. For instance, supposing you have 10000 mB stored, clicking the '-10' button too fast to retrieve your xp would you getting more than you started with:

with 1mB corresponding to 1 experience point. This only seems to happen with the retrieve 1 and 10 buttons while the store buttons work fine.

I suspect this is due to the client sending overlapping packets before the server can update the fluid level in the block, allowing you to retrieve more xp than actually exists. However, I have no idea how to prevent this. I've tried making it such that at most one request could sent per tick, as well as updating the client to see if that would help anything but nothing seems to have solved the issue.

package com.cyanogen.experienceobelisk.gui;

import com.cyanogen.experienceobelisk.block_entities.XPObeliskEntity;
import com.cyanogen.experienceobelisk.network.PacketHandler;
import com.cyanogen.experienceobelisk.network.UpdateXPToServer;
import net.minecraft.world.entity.player.Player;

public class XPManager {

    public static int levelsToXP(int levels){
        if (levels <= 16) {
            return (int) (Math.pow(levels, 2) + 6 * levels);
        } else if (levels >= 17 && levels <= 31) {
            return (int) (2.5 * Math.pow(levels, 2) - 40.5 * levels + 360);
        } else if (levels >= 32) {
            return (int) (4.5 * Math.pow(levels, 2) - 162.5 * levels + 2220);
        }
        return 0;
    }

    public static int xpToLevels(int xp){
        if (xp < 394) {
            return (int) (Math.sqrt(xp + 9) - 3);
        } else if (xp >= 394 && xp < 1628) {
            return (int) ((Math.sqrt(40 * xp - 7839) + 81) * 0.1);
        } else if (xp >= 1628) {
            return (int) ((Math.sqrt(72 * xp - 54215) + 325) / 18);
        }
        return 0;
    }

    public static int playerXP;
    public static int finalXP;
    public static int tickCount = 0;

    public static void storeXP(int levels, Player player, XPObeliskEntity xpobelisk){

        if(player.tickCount != tickCount){

            //total amount of experience points the player currently has
            playerXP = levelsToXP(player.experienceLevel) + Math.round(player.experienceProgress * player.getXpNeededForNextLevel());

            if(player.experienceLevel >= levels){

                finalXP = levelsToXP(player.experienceLevel - levels) + Math.round(player.experienceProgress *
                        (levelsToXP(player.experienceLevel - levels + 1) - levelsToXP(player.experienceLevel - levels)));

                player.giveExperienceLevels(-levels);
                PacketHandler.INSTANCE.sendToServer(new UpdateXPToServer(0,-levels));

                xpobelisk.fillFromClient(playerXP - finalXP);
            }
            else if (playerXP >= 1){
                xpobelisk.fillFromClient(playerXP);
                PacketHandler.INSTANCE.sendToServer(new UpdateXPToServer(-2147483647,-2147483647));
            }
        }

        tickCount = player.tickCount;
    }

    public static void retrieveXP(int levels, Player player, XPObeliskEntity xpobelisk){

        if(player.tickCount != tickCount){

            playerXP = levelsToXP(player.experienceLevel) + Math.round(player.experienceProgress * player.getXpNeededForNextLevel());

            finalXP = levelsToXP(player.experienceLevel + levels) + Math.round(player.experienceProgress *
                    (levelsToXP(player.experienceLevel + levels + 1) - levelsToXP(player.experienceLevel + levels)));

            if(xpobelisk.getFluidAmount() >= finalXP - playerXP){
                xpobelisk.drain(finalXP - playerXP);
                xpobelisk.drainFromClient(finalXP - playerXP);

                PacketHandler.INSTANCE.sendToServer(new UpdateXPToServer(0,levels));
            }
            else if(xpobelisk.getFluidAmount() >= 1){
                PacketHandler.INSTANCE.sendToServer(new UpdateXPToServer(xpobelisk.getFluidAmount(),0));

                xpobelisk.setFluid(0);
                xpobelisk.emptyFromClient();
            }
        }

        tickCount = player.tickCount;
    }
}

https://github.com/cyanog3n/experienceobelisk-1.18/blob/master/src/main/java/com/cyanogen/experienceobelisk/gui/XPManager.java

Any help would be appreciated!

[Luis_ST]

4 hours ago, cyanog3n said:

buttons are clicked very rapidly

you can use a delay so it's not possible to click the button very fast

5 hours ago, cyanog3n said:

under very laggy conditions

please elaborate

[Alpvax]

Don't EVER trust the client to report the correct amounts. Do all calculation of amount on the server, and just send "client requested X amount" to the server. The server should then verify that there is that amount in the block before giving it to the player.

The client should never tell the server what the new amount in the block is.

[cyanog3n]

So If I'm understanding you correctly, the packet should only contain 1. the number of levels i want to store or withdraw and 2. the position of the block i'm targeting?

3 hours ago, Luis_ST said:

please elaborate

It seems to happen most easily when facing something that causes a large frame drop on the client, like a huge stack of enchantment tables for example. It doesn't happen as easily on low tps situations but I haven't tested this rigorously. During normal gameplay you would have to click the button in excess of ~15cps to get the glitch to happen so I suppose a delay would work well

[Alpvax]

23 minutes ago, cyanog3n said:

So If I'm understanding you correctly, the packet should only contain 1. the number of levels i want to store or withdraw and 2. the position of the block i'm targeting?

Yes, although if you have a server side container (which I assume you do for a BlockEntity based GUI) then you should only need the first part, as you already know the block entity.

[Draco18s]

The client is a lying, cheating bastard. Do not trust the client to report the correct amounts.

[cyanog3n]

Certainly checks out. I moved everything serverside and it worked like a charm. Thanks for the help!