Dispatch a Vanilla Packet?

[Reika]

I have the need to send a vanilla-type packet (S35PacketUpdateTileEntity) to clients from the server. Previous to 1.7, the PacketDispatcher was used, but now the new system is only capable of accepting custom packet objects. Short of writing a wrapper for the S35, which I find to be a sloppy solution, how do I send the packet to a given player now? As soon as I know that, functions like sendToAllOnServer and sendToAllAround are easy to replicate.

[coolAlias]

// Client->Server
((EntityClientPlayerMP) player).sendQueue.addToSendQueue(new S35PacketUpdateTileEntity(params));

// Server->Client
((EntityPlayerMP) player).playerNetServerHandler.sendPacket(new S35PacketUpdateTileEntity(params));

You could also consider sending the packet returned from the TileEntity's getDescriptionPacket method.

[Reika]

// Client->Server
((EntityClientPlayerMP) player).sendQueue.addToSendQueue(new S35PacketUpdateTileEntity(params));

// Server->Client
((EntityPlayerMP) player).playerNetServerHandler.sendPacket(new S35PacketUpdateTileEntity(params));

You could also consider sending the packet returned from the TileEntity's getDescriptionPacket method.

 

This crashes with a message "Unexpected Change in protocol!".

[coolAlias]

Interesting. Well, I've only used the server->client way of sending vanilla messages with S12PacketEntityVelocity, so perhaps it doesn't like the TE update packet. It would help if you posted your code and crash report.

[Reika]

Interesting. Well, I've only used the server->client way of sending vanilla messages with S12PacketEntityVelocity, so perhaps it doesn't like the TE update packet. It would help if you posted your code and crash report.

 

private void sendPacketToAllAround(S35PacketUpdateTileEntity p, int r) {
	if (!worldObj.isRemote) {
		AxisAlignedBB box = ReikaAABBHelper.getBlockAABB(xCoord, yCoord, zCoord).expand(r, r, r);
		List<EntityPlayerMP> li = worldObj.getEntitiesWithinAABB(EntityPlayerMP.class, box);
		for (int i = 0; i < li.size(); i++)  {
			EntityPlayerMP entityplayermp = li.get(i);
			entityplayermp.playerNetServerHandler.sendPacket(p);
		}
	}
}

 

java.lang.IllegalStateException: Unexpected change in protocol!
at net.minecraft.network.NetHandlerPlayServer.onConnectionStateTransition(NetHandlerPlayServer.java:1390)
at net.minecraft.network.NetworkManager.processReceivedPackets(NetworkManager.java:236)
at net.minecraft.network.NetworkSystem.networkTick(NetworkSystem.java:182)
at net.minecraft.server.MinecraftServer.updateTimeLightAndEntities(MinecraftServer.java:736)
at net.minecraft.server.MinecraftServer.tick(MinecraftServer.java:624)
at net.minecraft.server.integrated.IntegratedServer.tick(IntegratedServer.java:118)
at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:495)
at net.minecraft.server.MinecraftServer$2.run(MinecraftServer.java:762)

 

 

[coolAlias]

I'm not too sure why you are getting that message, but I must ask, why do you need to send a TileEntity update packet in this way? Whenever a TE block is marked for an update (which you can do manually, if needed), the TileEntity's description packet is sent automatically to all nearby players - you can override getDescriptionPacket to add any extra information you need.

// this will force the block to update, and all nearby clients will get the new description packet
// any information you send will be available for rendering, GUIs, etc.
worldObj.markBlockForUpdate(xCoord, yCoord, zCoord);


@Override
public Packet getDescriptionPacket() {
NBTTagCompound tag = new NBTTagCompound();
this.writeToNBT(tag);
return new S35PacketUpdateTileEntity(xCoord, yCoord, zCoord, 1, tag);
}

@Override
public void onDataPacket(NetworkManager net, S35PacketUpdateTileEntity packet) {
readFromNBT(packet.func_148857_g());
}

[Reika]

I'm not too sure why you are getting that message, but I must ask, why do you need to send a TileEntity update packet in this way? Whenever a TE block is marked for an update (which you can do manually, if needed), the TileEntity's description packet is sent automatically to all nearby players - you can override getDescriptionPacket to add any extra information you need.

// this will force the block to update, and all nearby clients will get the new description packet
// any information you send will be available for rendering, GUIs, etc.
worldObj.markBlockForUpdate(xCoord, yCoord, zCoord);


@Override
public Packet getDescriptionPacket() {
NBTTagCompound tag = new NBTTagCompound();
this.writeToNBT(tag);
return new S35PacketUpdateTileEntity(xCoord, yCoord, zCoord, 1, tag);
}

@Override
public void onDataPacket(NetworkManager net, S35PacketUpdateTileEntity packet) {
readFromNBT(packet.func_148857_g());
}

This is true, but I in fact have a special subclass of S35 which only writes the "changed" data, as sending all the data at any rate fast enough to provide acceptable sync is terrible on network load. Because getDescriptionPacket() is what is used to sync data to clients on logging into servers, it must still return a vanilla S35, because a fresh client does need to have all the data synced, whether it just changed serverside or not.

 

Additionally, markBlockForUpdate triggers block and lighting updates, which is very bad for performance.

[coolAlias]

Ah, well without knowing anything about what you're trying to do, that was the best I could come up with. Why don't you create a custom packet then and send it using your mod network rather than trying to use the vanilla network? That way you can send as much or as little information as you want without worrying about illegal stances (probably because the vanilla system does not recognize your subclass).

[Reika]

Ah, well without knowing anything about what you're trying to do, that was the best I could come up with. Why don't you create a custom packet then and send it using your mod network rather than trying to use the vanilla network? That way you can send as much or as little information as you want without worrying about illegal stances (probably because the vanilla system does not recognize your subclass).

I could write a wrapper for the S35 whose read methods are essentially a super() call, but that is a messy solution I would like to avoid.

Additionally, it would require some rewrites to my base classes that I would like to avoid if possible.

[sequituri]

Have you tried using the simplimpl package and the SimpleChannelHandlerWrapper and SimpleNetworkWrapper classes?

[Reika]

Have you tried using the simplimpl package and the SimpleChannelHandlerWrapper and SimpleNetworkWrapper classes?

I have no idea what the "simplimpl" package is, but the Wrapper classes are for the Forge packet system, not vanilla.

[sequituri]

Have you tried using the simplimpl package and the SimpleChannelHandlerWrapper and SimpleNetworkWrapper classes?

I have no idea what the "simplimpl" package is, but the Wrapper classes are for the Forge packet system, not vanilla.

I take it from that answer that you have no desire whatsoever to use Forge netty support and will only work on a vanilla packet solution.

That's cool by me and good luck with it.

[Reika]

Have you tried using the simplimpl package and the SimpleChannelHandlerWrapper and SimpleNetworkWrapper classes?

I have no idea what the "simplimpl" package is, but the Wrapper classes are for the Forge packet system, not vanilla.

I take it from that answer that you have no desire whatsoever to use Forge netty support and will only work on a vanilla packet solution.

That's cool by me and good luck with it.

Not for a vanilla packet subclass, no. It is to me both ugly and probably worse for performance, both in terms of network and CPU load.

[coolAlias]

The question is, though, why does it absolutely have to subclass a vanilla packet? Why not write an IMessage on SimpleNetworkWrapper that sends the exact data you want, rather than trying to force a non-vanilla packet through the vanilla system? It seems to me that that would be the simplest and cleanest solution, since your attempts so far have not gone anywhere.

 

The code I gave earlier does work, though, with vanilla packets. Interesting that sub-classed versions don't work.

[Reika]

The question is, though, why does it absolutely have to subclass a vanilla packet? Why not write an IMessage on SimpleNetworkWrapper that sends the exact data you want, rather than trying to force a non-vanilla packet through the vanilla system? It seems to me that that would be the simplest and cleanest solution, since your attempts so far have not gone anywhere.

I could do it, but it seems largely superfluous, because it will have to basically be lots of copy-paste from the S35 and Packet classes.

 

The code I gave earlier does work, though, with vanilla packets. Interesting that sub-classed versions don't work.

That is good to know, and I will use that in the future if needed.

[coolAlias]

Hardly lots - from the tile entity packet, you would only need the x/y/z coordinates; whatever data you want to send wouldn't have been there anyway, and you need nothing at all from the Packet class itself.

 

This is an entire message + handler that could be used to update specific info in a tile entity - all you would have to do is fill in whatever info you want to send, register it to Side.CLIENT, and use the SimpleNetworkWrapper to sendToAllNearby.

 

In the end, I'd wager it will be less code than you are using right now and it won't have the overhead of inheriting from the vanilla TE update packet.

 

public class TEUpdatemessage implements IMessage
{
// tile entity coordinates:
private int xCoord, yCoord, zCoord;

private Something otherInfo;

public TEUpdatemessage() {}

public TEUpdatemessage(TileEntity te) {
	this.xCoord = te.xCoord;
	this.yCoord = te.yCoord;
	this.zCoord = te.zCoord;
	this.otherInfo = te.someInfo;
}

@Override
public void fromBytes(ByteBuf buffer) {
	xCoord = buffer.readInt();
	yCoord = buffer.readInt();
	zCoord = buffer.readInt();
	someInfo = buffer.readSomething();
}

@Override
public void toBytes(ByteBuf buffer) {
	buffer.writeInt(xCoord);
	buffer.writeInt(yCoord);
	buffer.writeInt(zCoord);
	buffer.writeSomething(someInfo);
}

public static class TEUpdatemessageHandler implements IMessageHandler<TEUpdatemessage, IMessage> {
	@Override
	public IMessage onMessage(TEUpdatemessage message, MessageContext ctx) {
		EntityPlayer player = YourMod.proxy.getPlayerEntity(ctx);
		TileEntity te = player.worldObj.getTileEntity(message.xCoord, message.yCoord, message.zCoord);
		if (te instanceof YourTileEntity) {
			((YourTileEntity) te).setSomething(message.someInfo);
		}
		return null;
	}
}
}

 

 

If you still consider that superfluous, well that's your call, but it seems pretty straightforward to me.

[Reika]

Hardly lots - from the tile entity packet, you would only need the x/y/z coordinates; whatever data you want to send wouldn't have been there anyway, and you need nothing at all from the Packet class itself.

 

This is an entire message + handler that could be used to update specific info in a tile entity - all you would have to do is fill in whatever info you want to send, register it to Side.CLIENT, and use the SimpleNetworkWrapper to sendToAllNearby.

 

In the end, I'd wager it will be less code than you are using right now and it won't have the overhead of inheriting from the vanilla TE update packet.

 

public class TEUpdatemessage implements IMessage
{
// tile entity coordinates:
private int xCoord, yCoord, zCoord;

private Something otherInfo;

public TEUpdatemessage() {}

public TEUpdatemessage(TileEntity te) {
	this.xCoord = te.xCoord;
	this.yCoord = te.yCoord;
	this.zCoord = te.zCoord;
	this.otherInfo = te.someInfo;
}

@Override
public void fromBytes(ByteBuf buffer) {
	xCoord = buffer.readInt();
	yCoord = buffer.readInt();
	zCoord = buffer.readInt();
	someInfo = buffer.readSomething();
}

@Override
public void toBytes(ByteBuf buffer) {
	buffer.writeInt(xCoord);
	buffer.writeInt(yCoord);
	buffer.writeInt(zCoord);
	buffer.writeSomething(someInfo);
}

public static class TEUpdatemessageHandler implements IMessageHandler<TEUpdatemessage, IMessage> {
	@Override
	public IMessage onMessage(TEUpdatemessage message, MessageContext ctx) {
		EntityPlayer player = YourMod.proxy.getPlayerEntity(ctx);
		TileEntity te = player.worldObj.getTileEntity(message.xCoord, message.yCoord, message.zCoord);
		if (te instanceof YourTileEntity) {
			((YourTileEntity) te).setSomething(message.someInfo);
		}
		return null;
	}
}
}

 

 

If you still consider that superfluous, well that's your call, but it seems pretty straightforward to me.

 

I would also need to copy the entire NBT serializer/deserializer.

[sequituri]

 

 

I would also need to copy the entire NBT serializer/deserializer.

 

Or... you could implement a transaction log for each TE and when changing NBT data also copy the path "I:Rieka/TEUsage/suchAndSo" to the log (Set<String> tlog).

 

Then when you send update package, use the tlog to send only changed data and then clear it.

[coolAlias]

I would also need to copy the entire NBT serializer/deserializer.

Or simply use ByteBufUtils.writeTag(buffer) and ByteBufUtils.readTag(buffer)...

[Reika]

I would also need to copy the entire NBT serializer/deserializer.

Or simply use ByteBufUtils.writeTag(buffer) and ByteBufUtils.readTag(buffer)...

If there is a prefab utility, then using Forge packets becomes a viable solution. I will look into this.

[Reika]

Writing a wrapper ultimately failed, but I have found a solution to my initial inquiry. It turns out the problem was that you can easily dispatch a vanilla packet using the

entityplayermp.playerNetServerHandler.sendPacket(p);

method, but any unique classes require some special code.

 

Basically, the vMC packet system has a few internal maps of <Class<?extends Packet>, EnumConnectionState> and <Integer, Class<?extends Packet>, and these are drawn upon to fetch the EnumConnectionState from a given packet. With my subclass of S35, it was returning null from those maps (which is obviously not equal to PLAY) and immediately crashed with a mismatch.

 

So here is the solution, which makes everything work wonderfully. What you need to do to register a custom type of vanilla packet is to manually add the mapping for your packet type. To do this, you need to add an entry to two maps. One is a map of <Integer, Class<?extends Packet>; this one is an instance field of the EnumConnectionState object. Simply register your packet (with a unique ID!) there. Now you need to add to the master <Class<?extends Packet>, EnumConnectionState> map. This one is static to the EnumConnectionState class, so you can simply add it directly.

 

Two things to note:

One, the nonstatic map in fact has two copies, one for clientsourced/serverbound packets and one for serversourced/clientbound packets. For vanilla packets, the class name begins with either "C" or "S" respectively.

Two, all these maps are private. However, the nonstatic maps each have a public function to return the map instance itself, allowing for a direct .put() call. The static map must be accessed either reflectively or via an access transformer.

 

Sample code:

//Nonstatic map
EnumConnectionState.PLAY.func_150755_b().put(182, SyncPacket.class);

//Static map
Field f = EnumConnectionState.class.getDeclaredField("field_150761_f");
f.setAccessible(true);
Map map = (Map)f.get(null);
map.put(SyncPacket.class, state);

[coolAlias]

Good work finding that out, though adding your own discriminators into the map has some huge potential for conflicting with not only other mods that may think of doing the same thing, but also vanilla if they ever decide to use those values for their packets.

 

I would still recommend using SimpleNetworkWrapper and the ByteBufUtils for your packet needs, as the implementation is probably pretty identical in terms of lines of code and simplicity, but ensures compatibility. Anyway, that's still neat that you figured out how to register your own packets to the vanilla system - you could do that with any packet, I would guess, not just sub-classes of vanilla ones.

[Reika]

Good work finding that out, though adding your own discriminators into the map has some huge potential for conflicting with not only other mods that may think of doing the same thing, but also vanilla if they ever decide to use those values for their packets.

That is why you make the IDs configurable, and then the problem only arises with inept pack makers or if vMC decides to add 200 new packet types. 

 

you could do that with any packet, I would guess, not just sub-classes of vanilla ones.

No, the ultimate parent class must be Packet.