Custom NetServerHandler ?

Hi, Needspeed and me made a client mod removing the 15 character limitation when writing on a sign. When we tried it in SMP, more than 15 chars resulted in te string "!?".
We searched the craftbukkit code and found following:

From the NetServerHandler code (open) From the NetServerHandler code (close)

Code:

public void a(Packet130UpdateSign packet130updatesign) {
        // CraftBukkit start
        if (this.e.world.f(packet130updatesign.a, packet130updatesign.b, packet130updatesign.c)) {
            TileEntity tileentity = this.e.world.getTileEntity(packet130updatesign.a, packet130updatesign.b, packet130updatesign.c);
            if (tileentity instanceof TileEntitySign) {
                TileEntitySign tileentitysign = (TileEntitySign)tileentity;
                if (!tileentitysign.a()) {
                    this.d.c("Player " + this.e.name + " just tried to change non-editable sign");
                    return;
                }
            }
            // CraftBukkit end

            int i;
            int j;

            for (j = 0; j < 4; ++j) {
                boolean flag = true;

/*-------->*/ if (packet130updatesign.d[j].length() > 15) {
                    flag = false;
                } else {
                    for (i = 0; i < packet130updatesign.d[j].length(); ++i) {
                        if (FontAllowedCharacters.a.indexOf(packet130updatesign.d[j].charAt(i)) < 0) {
                            flag = false;
                        }
                    }
                }

                if (!flag) {
                    packet130updatesign.d[j] = "!?";
                }
            }

            if (tileentity instanceof TileEntitySign) {
                j = packet130updatesign.a;
                int k = packet130updatesign.b;

                i = packet130updatesign.c;
                TileEntitySign tileentitysign1 = (TileEntitySign) tileentity;

                // CraftBukkit start - SIGN_CHANGE hook
                Player player = server.getPlayer(this.e);
                SignChangeEvent event = new SignChangeEvent((CraftBlock) player.getWorld().getBlockAt(j, k, i), server.getPlayer(this.e), packet130updatesign.d);
                server.getPluginManager().callEvent(event);

                if (event.isCancelled()) {
                    // Normally we would return here, but we have to update the sign with blank text if it's been cancelled
                    // Otherwise the client will have bad text on their sign (client shows text changes as they type)
                    for (int l = 0; l < 4; ++l) {
                        event.setLine(l, "");
                    }
                }
                // CraftBukkit end

                for (int l = 0; l < 4; ++l) {
                    tileentitysign1.a[l] = event.getLine(l);
                    // CraftBukkit
                }

                tileentitysign1.i();
                // CraftBukkit
                this.e.world.g(j, k, i);
            }
        }
    }

To solve it we did this:

Solution (open) Solution (close)

Code:

public class CNetServerHandler extends NetServerHandler
{
 private final CraftServer server;
 private MinecraftServer d;
 public CNetServerHandler(MinecraftServer minecraftserver,
   NetworkManager networkmanager, EntityPlayer entityplayer)
 {
  super(minecraftserver, networkmanager, entityplayer);
  this.d = minecraftserver;
  this.server = minecraftserver.server;
 }

@Override
public void a(Packet130UpdateSign packet130updatesign) {
    System.out.println("--test--");
 // CraftBukkit start
    if (this.e.world.f(packet130updatesign.a, packet130updatesign.b, packet130updatesign.c)) {
        TileEntity tileentity = this.e.world.getTileEntity(packet130updatesign.a, packet130updatesign.b, packet130updatesign.c);
        if (tileentity instanceof TileEntitySign) {
            TileEntitySign tileentitysign = (TileEntitySign)tileentity;
            if (!tileentitysign.a()) {
                this.d.c("Player " + this.e.name + " just tried to change non-editable sign");
                return;
            }
        }
        // CraftBukkit end

        int i;
        int j;

        for (j = 0; j < 4; ++j) {
            boolean flag = true;

            if (packet130updatesign.d[j].length() > 15) {
                //flag = false;

            }
            for (i = 0; i < packet130updatesign.d[j].length(); ++i) {
             if (FontAllowedCharacters.a.indexOf(packet130updatesign.d[j].charAt(i)) < 0) {
              flag = false;
                }
            }


            if (!flag) {
                packet130updatesign.d[j] = "!?";
            }
        }

        if (tileentity instanceof TileEntitySign) {
            j = packet130updatesign.a;
            int k = packet130updatesign.b;

            i = packet130updatesign.c;
            TileEntitySign tileentitysign1 = (TileEntitySign) tileentity;

            // CraftBukkit start - SIGN_CHANGE hook
            Player player = server.getPlayer(this.e);
            SignChangeEvent event = new SignChangeEvent((CraftBlock) player.getWorld().getBlockAt(j, k, i), server.getPlayer(this.e), packet130updatesign.d);
            server.getPluginManager().callEvent(event);

            if (event.isCancelled()) {
                // Normally we would return here, but we have to update the sign with blank text if it's been cancelled
                // Otherwise the client will have bad text on their sign (client shows text changes as they type)
                for (int l = 0; l < 4; ++l) {
                    event.setLine(l, "");
                }
            }
            // CraftBukkit end

            for (int l = 0; l < 4; ++l) {
                tileentitysign1.a[l] = event.getLine(l);
                // CraftBukkit
            }

            tileentitysign1.i();
            // CraftBukkit
            this.e.world.g(j, k, i);
        }
    }
}
}

And in onCommand(...):

Code:

CraftPlayer cp = (CraftPlayer) player;
  EntityPlayer ep = (EntityPlayer) cp.getHandle();
  ep.a = new CNetServerHandler(ep.b,ep.a.b,ep);

• Is this a good solution? - Is there a better one?
• Which errors might occur?
• Should we make a pull request?
  • How should it look like? - Something like Player.SetIsAllowedToOverloadSign(...) ?

 

Nobody an idea?