Jump to content

[1.12.2] Packet problems when updating capability data


Alekseyev

Recommended Posts

Hi all,

 

after quite some time away I finally got back to fixing a long-standing issue within my mod in progress. (See here)

 

I was advised to send the capability data to the server via a packet, and therefore set out to that. I came across this tutorial: https://wiki.mcjty.eu/modding/index.php?title=Networking-1.12 and used that to test out how packets work. The example function that was provided works flawlessly. However, when I tried to use another packet to update the needed data, I received a crash.

 

Here's the crash log.

 

And here are my relevant classes:

 

Event Subscriber

	@SideOnly(Side.CLIENT)
	@SubscribeEvent(priority=EventPriority.NORMAL, receiveCanceled=true)
	public static void onKeyInput(InputEvent.KeyInputEvent event)
	{
		System.out.println("Key Input Event");
		KeyBinding[] keyBindings = ClientProxy.keyBindings;

		// crashes
		if (keyBindings[KEY_TEST].isPressed())
		{
			System.out.println("Key binding =" + keyBindings[KEY_TEST].getKeyDescription());
			PacketHandler.INSTANCE.sendToServer(new PacketUpdateElement(BioElements.WATER));
		}

      	//Works fine
		if (keyBindings[KEY_GET_BLOCK].isPressed())
		{
			System.out.println("Key binding =" + keyBindings[KEY_GET_BLOCK].getKeyDescription());
			PacketHandler.INSTANCE.sendToServer(new PacketSendKey()); 
		}
	}

 

Packet

public class PacketUpdateElement implements IMessage
{
	private BioElements element;

	@Override
	public void fromBytes(ByteBuf buf)
	{
		try
		{
			byte[] bytes = new byte[buf.readableBytes()];
			buf.readBytes(bytes);
			element = BioElements.valueOf(new String(bytes));
		}
		catch (IllegalArgumentException e) // If no matching String is found in the enum, an IllegalArgumentException will be thrown
		{
			System.out.println("An error occurred: " + e.getMessage());
			element = BioElements.NONE;
			System.out.println("Element was set to NONE.");
		}
	}

	@Override
	public void toBytes(ByteBuf buf)
	{
		buf.writeBytes(element.name().getBytes());
	}

	public PacketUpdateElement(BioElements element)
	{
		this.element = element;
	}

	public static class Handler implements IMessageHandler<PacketUpdateElement, IMessage>
	{
		@Override
		public IMessage onMessage(PacketUpdateElement message, MessageContext context)
		{
			FMLCommonHandler.instance().getWorldThread(context.netHandler).addScheduledTask(() -> handle(message, context));
			return null;
		}

		private void handle(PacketUpdateElement message, MessageContext context)
		{
			EntityPlayerMP playerEntity = context.getServerHandler().player;

			if (playerEntity.hasCapability(CAPABILITY_BIO_PLAYER_DATA, null))
			{
				playerEntity.getCapability(CAPABILITY_BIO_PLAYER_DATA, null).setElement(message.element);
			}

			playerEntity.sendStatusMessage(new TextComponentString(TextFormatting.GREEN + "Selected element from packet: " + message.element.name()), false);
		}
	}
}

 

Packet handler

public class PacketHandler
{
	private static int packetId = 0;

	public static SimpleNetworkWrapper INSTANCE = null;

	public PacketHandler()
	{
	}

	public static int nextID()
	{
		return packetId++;
	}

	public static void registerMessages(String channelName)
	{
		INSTANCE = NetworkRegistry.INSTANCE.newSimpleChannel(channelName);
		registerMessages();
	}

	public static void registerMessages()
	{
		// Register messages which are sent from the client to the server here:
		INSTANCE.registerMessage(PacketSendKey.Handler.class, PacketSendKey.class, nextID(), Side.SERVER);
		INSTANCE.registerMessage(PacketUpdateElement.Handler.class, PacketUpdateElement.class, nextID(), Side.SERVER);
	}
}

PacketHandler.registerMessages("biomod");  is fired in the FMLInitializationEvent event subscriber.

 

I tried figuring out the issue on my own, but had no luck so far.

Link to comment
Share on other sites

Your packet class MUST have an explicitly defined parameterless constructor.

 

15 minutes ago, Alekseyev said:

@SideOnly(Side.CLIENT)

Don't abuse SideOnly, modders are not even meant to use it in the first place. If your event handlers are client-side only then only register them on the client.

 

Don't send a packet to the server every time a key is pressed regardless of anything. Perform the checks as to whether the client should even send the packet first, receive the packet on the server, perform your checks for whatever it is you need to do, etc. Don't spam the server with packets, but don't trust the client either.

 

17 minutes ago, Alekseyev said:

byte[] bytes = new byte[buf.readableBytes()]; buf.readBytes(bytes); element = BioElements.valueOf(new String(bytes));

Why are you doing this? Just store the ordinal as a byte or something in the buffer, then read the enum constant through said enum's array.

If you don't like this approach then you can have an index field in your enums and get it by the index. There is no reason to send a string.

Also if you ever do need to send a string use ByteBufUtils.

 

 

Link to comment
Share on other sites

2 minutes ago, V0idWa1k3r said:

Your packet class MUST have an explicitly defined parameterless constructor.

How do I supply the info to the packet, then?

 

---------------------------------------------------

1 minute ago, V0idWa1k3r said:

Don't send a packet to the server every time a key is pressed regardless of anything. Perform the checks as to whether the client should even send the packet first, receive the packet on the server, perform your checks for whatever it is you need to do, etc. Don't spam the server with packets, but don't trust the client either.

Thanks for the heads up, I will keep it in mind for the future. This particular case was just for easy testing anyway and will be removed afterwards.

3 minutes ago, V0idWa1k3r said:

Why are you doing this? Just store the ordinal as a byte or something in the buffer, then read the enum constant through said enum's array.

If you don't like this approach then you can have an index field in your enums and get it by the index. There is no reason to send a string. 

Also if you ever do need to send a string use ByteBufUtils.

I thought about simply using an ordinal and a single byte, since the amount of enum values will never exceed 256 anyway. I will look into fixing this as soon as the packet sending itself works.

Link to comment
Share on other sites

5 minutes ago, Alekseyev said:

How do I supply the info to the packet, then?

 

I didn't say that it must ONLY have the parameterless constructor, i said that you must INCLUDE it. You can still keep your current constructor and use it just fine.

 

Also there are a ton of ways to pass data to an object without involving the constructor, like a setter for example. But this is basic java.

 

 

  • Thanks 1
Link to comment
Share on other sites

Just now, V0idWa1k3r said:

Also there are a ton of ways to pass data to an object without involving the constructor, like a setter for example. But this is basic java

Sorry, sometimes I'm dumb. I was assuming that if there has to be a parameterless constructor, that one would be used in the sending process somehow and you would be unable to use your pre-created object.

 

I would dare to say that I know basic java pretty well, it's using someone else's hooks and libraries and whatnot (in this case Forge) what gets me all the time. :s

Link to comment
Share on other sites

3 hours ago, Alekseyev said:

Sorry, sometimes I'm dumb. I was assuming that if there has to be a parameterless constructor, that one would be used in the sending process somehow and you would be unable to use your pre-created object.

Server creates the packet by calling new YourPacket(some, params), right?

 

What do you think the client does, it doesn't know what some, params are, but it still needs to create the packet instance so that it can take the buffer it does have and pipe it through fromBytes(buf). And the only way that even makes sense is to call a parameterless constructor.

Apparently I'm a complete and utter jerk and come to this forum just like to make fun of people, be confrontational, and make your personal life miserable.  If you think this is the case, JUST REPORT ME.  Otherwise you're just going to get reported when you reply to my posts and point it out, because odds are, I was trying to be nice.

 

Exception: If you do not understand Java, I WILL NOT HELP YOU and your thread will get locked.

 

DO NOT PM ME WITH PROBLEMS. No help will be given.

Link to comment
Share on other sites

8 hours ago, Draco18s said:

And the only way that even makes sense is to call a parameterless constructor.

A factory would be nicer if vanilla did it that way

About Me

Spoiler

My Discord - Cadiboo#8887

My WebsiteCadiboo.github.io

My ModsCadiboo.github.io/projects

My TutorialsCadiboo.github.io/tutorials

Versions below 1.14.4 are no longer supported on this forum. Use the latest version to receive support.

When asking support remember to include all relevant log files (logs are found in .minecraft/logs/), code if applicable and screenshots if possible.

Only download mods from trusted sites like CurseForge (minecraft.curseforge.com). A list of bad sites can be found here, with more information available at stopmodreposts.org

Edit your own signature at www.minecraftforge.net/forum/settings/signature/ (Make sure to check its compatibility with the Dark Theme)

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Unfortunately, your content contains terms that we do not allow. Please edit your content to remove the highlighted words below.
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Announcements



×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use.