[1.11.2] Capabilitys and Packets

[HalestormXV]

Quick question. I set up a capability and it works great. It is hooked up to an OnBlockBreak event. Basically, when you break a certain block there is a 15% change that the ability will activate and either increase or decrease the scale by 1 point. The capability is persistent and hooked up to the onCloned so it stays. What I want to do is set up a bar (guiOverlay, like an HP bar) that can increase or decreased based on the value of the capability. The capability can go negative which is fine. Here is a pic of the bar:

 

Spoiler

So the red is the "background color" and basically is what will show once you start going negative. The blue is what will start filling up as you go positive. Your value starts at 0 so essentially it will display as half blue and half red. I know to solve all that you need to math and thats fine but what I want to know is do you need to involve packets since it is a capability? Or is it possible to get the capaibility and just read it. My gut says you need a packet since it is client fetching data from server for display but I figured I'd ask anyway.

[Choonster]

You'll need to synchronise the value from the server to the client with a custom packet, yes.

 

If the capability is attached to a player, do this when the player logs in (PlayerEvent.PlayerLoggedInEvent) and when the value changes.

[HalestormXV]

And it being a single interger shouldn't pose any issue right. If anything itd make it easier right?

[Choonster]

2 minutes ago, HalestormXV said:

And it being a single interger shouldn't pose any issue right. If anything itd make it easier right?

 

No, there shouldn't be any issues regardless of what data you need to sync. A single integer should make things very simple, though.

[HalestormXV]

1 minute ago, Choonster said:

 

No, there shouldn't be any issues regardless of what data you need to sync. A single integer should make things very simple, though.

I actually have a question for you. I've followed a number of your tutorials and tehy are great. Perhaps you can give me some feedback on this.

This is my packetHandler: https://pastebin.com/K5vzgq9N

This is my NetworkWrapper: https://pastebin.com/wYLKay8N

 

This is my packet andler and network wrapper. This is from I think 1.8.2 or 1.9? I honestly can't remember sicne it has been so long. Is this still valid/effcient way to take care of this in 1.11.2?

[Choonster]

40 minutes ago, HalestormXV said:

I actually have a question for you. I've followed a number of your tutorials and tehy are great. Perhaps you can give me some feedback on this.

This is my packetHandler: https://pastebin.com/K5vzgq9N

This is my NetworkWrapper: https://pastebin.com/wYLKay8N

 

This is my packet andler and network wrapper. This is from I think 1.8.2 or 1.9? I honestly can't remember sicne it has been so long. Is this still valid/effcient way to take care of this in 1.11.2?

 

Packets haven't really changed in recent versions, so anything that worked in 1.8.x/1.9.x should still work in 1.11.2 or 1.12.

 

You can't safely reference the Minecraft class in an IMessageHandler#onMessage method that will be called on both sides, you need to move the code to get an IThreadListener to your proxies (like this, this and this).

 

That seems like a lot bolilerplate code for not a lot of gain. If you look at my mod's packets, you'll see that the first line of every packet handler uses the proxy methods to get the IThreadListener and schedule a lambda implementation of Runnable. This seems simpler than wrapping SimpleNetworkWrapper and IMessageHandler.

[HalestormXV]

17 hours ago, Choonster said:

 

Packets haven't really changed in recent versions, so anything that worked in 1.8.x/1.9.x should still work in 1.11.2 or 1.12.

 

You can't safely reference the Minecraft class in an IMessageHandler#onMessage method that will be called on both sides, you need to move the code to get an IThreadListener to your proxies (like this, this and this).

 

That seems like a lot bolilerplate code for not a lot of gain. If you look at my mod's packets, you'll see that the first line of every packet handler uses the proxy methods to get the IThreadListener and schedule a lambda implementation of Runnable. This seems simpler than wrapping SimpleNetworkWrapper and IMessageHandler.

 

 

I See, so the static class (since I implement the handler into the packe) should look something like this instead:

    public static class FetchMoralityHandler implements IMessageHandler<FetchMoralityPacket, IMessage>
    {
        // Do note that the default constructor is required, but implicitly defined in this case

        @Override
        public IMessage onMessage(FetchMoralityPacket message, MessageContext ctx)
        {
         EntityPlayerMP player = ctx.getServerHandler().playerEntity;
         player.mcServer.addScheduledTask(() -> {
                     IMorality morality = player.getCapability(moralityProvider.MORALITY_CAP, null);
                     int amount = message.toSend;
                     player.sendMessage(new TextComponentString("Your current morality is: " + morality.getMorality()));
                 });
            // No response packet
            return null;
        }
    }
}

Edited July 12, 2017 by HalestormXV

[Choonster]

5 hours ago, HalestormXV said:

I See, so the static class (since I implement the handler into the packe) should look something like this instead:

 

Is this a client-to-server or a server-to-client packet?

 

Why does the packet send an int (FetchMoralityPacket#toSend) if the handler ignores it?

 

Any string that you display to the user should be localised. Include a format placeholder (e.g. %d for an integer) in the translation and you can create a TextComponentTranslation with the translation key and the format argument instead of concatenating the text and the amount in a TextComponentString.

 

If the handler is a nested class of the packet, you don't need to include the packet's action (FetchMorality) in its name; since it's implied by being a nested class. This is just my code style, it's not required that you follow it.

 

Apart from these issues, it looks correct.

[HalestormXV]

3 hours ago, Choonster said:

 

Is this a client-to-server or a server-to-client packet?

 

Why does the packet send an int (FetchMoralityPacket#toSend) if the handler ignores it?

 

Any string that you display to the user should be localised. Include a format placeholder (e.g. %d for an integer) in the translation and you can create a TextComponentTranslation with the translation key and the format argument instead of concatenating the text and the amount in a TextComponentString.

 

If the handler is a nested class of the packet, you don't need to include the packet's action (FetchMorality) in its name; since it's implied by being a nested class. This is just my code style, it's not required that you follow it.

 

Apart from these issues, it looks correct.

 

I actually managed to fix this up and get it working as intended. The idea was to be able to press a keyBind and then you will receive a message with your Morality. That worked and is working wonderfully both on a server and on a client, so I managed to get my packets set up correctly. However I have run into a new issue with a similar situation. Now I am trying to Sync the server and the client. In the future I plan to make a GUIBar that will reflect your Morality levels, however  I want to lay the foundation. I followed a tutoiral that seems to have been updated to 1.11.2 and seems to work but I am getting a strange error and perhaps it is because I misunderstood the tutoiral but maybe reading it here will help me understand it better. Here is how it works currently. You break certain blocks the morality scales tip back and forth. I want to sync the client and the server everytime that happens. So here are my two packets. (Now please do mind you this was a tutoiral that I was following so perhaps I misunderstood or did something wrong).

 

SyncMorality - https://pastebin.com/bnNkaE9P

SyncMoralityReturn - https://pastebin.com/Kj7udWmR

 

Those are the two packets and here is where I make the call in my event handler: (attached to a blockbreak)

    if (chance < 15) {
        EntityPlayer player = event.getPlayer();
        IMorality morality = player.getCapability(moralityProvider.MORALITY_CAP, null);
        player.sendMessage(new TextComponentString("\u00A74" + "Your scales of morality have tipped to sin."));
        morality.addSin(1);
        eAngelusPacketHandler.sendToServer(new SyncMorality(morality.getMorality(), "halestormxv.eAngelus.main.handlers.EA_EventHandler",
                Reference.MODID +":morality"));
    }
}

 

Now all works fine. When you break the block and the chance procs your morality will increase. And as you can see I call my sentToServer which produces this error: https://pastebin.com/pNXDHaMq

 

I mean I understand the error (i think?), it is basically saying that the eangel:morality field doesnt exist. But it does, I am looking at it in the NBT data:

 

So like I said, maybe  I am misunderstanding something which is more than possible. So any help is apprecaited. Examples are excellent as well since I am one of those stare at it and study it learners (most of my learning comes from YouTube so I can pause and learn whats going on) I know i don't need to get the Sync system working IMMEDIATLY but I like to have the foundation laid out. If you need any of the other code just let me know and I will post it.

 

Here is the capability code:

https://pastebin.com/cKt9U5Be - moralityProvider

https://pastebin.com/KA47rFYJ - moralityStorage

https://pastebin.com/yJadAav3 - moralityScale

 

And my interface just has the functions in it.

Edited July 13, 2017 by HalestormXV

[Choonster]

That's not a field, that's an NBT key. morality is the name of the field in the moralityScale class.

 

This tutorial code is overly complex and fragile, since it relies on sending class and field names between sides and reflectively assigning the new value to the field. I suggest you delete it and start over.

 

The server is in charge of the game state, so any change to the morality value needs to be done on the server. When the morality value changes, the server can send a packet to the client to update its copy of the morality value. This should be handled in the moralityScale class itself, so the code that interacts with it doesn't need to know the details of how and when the value is synced.

 

The packet should only send the new morality value, you don't need to send a class/field name or determine whether or not the packet is valid.

[HalestormXV]

36 minutes ago, Choonster said:

That's not a field, that's an NBT key. morality is the name of the field in the moralityScale class.

 

This tutorial code is overly complex and fragile, since it relies on sending class and field names between sides and reflectively assigning the new value to the field. I suggest you delete it and start over.

 

The server is in charge of the game state, so any change to the morality value needs to be done on the server. When the morality value changes, the server can send a packet to the client to update its copy of the morality value. This should be handled in the moralityScale class itself, so the code that interacts with it doesn't need to know the details of how and when the value is synced.

 

The packet should only send the new morality value, you don't need to send a class/field name or determine whether or not the packet is valid.

You have a brief example of what it may look like? I am trying to work it out and I am guessing that in my moralityScale i will have a void syncDataClient(entityPlayer player) then it will fetch the moreality value of the player passed into it and send it to the client in a packet using MyPacektHandler.sentTo(new SyncMoralityToClient, (EntityMP) player.

 

Does it then need to write that using toBytes and fromBytes to a player NBT tag? I know it should not be this hard to pass one interger in a packet to the client but I just am having hte worst trouble with it for no reason.

Edited July 13, 2017 by HalestormXV

[Choonster]

2 minutes ago, HalestormXV said:

You have a brief example of what it may look like? I am trying to work it out and I am guessing that in my moralityScale i will have a void syncDataClient(entityPlayer player) then it will fetch the moreality value of the player passed into it and send it to the client in a packet using MyPacektHandler.sentTo(new SyncMoralityToClient, (EntityMP) player.

 

moralityScale should have an EntityPlayer field holding the player it's attached to.

 

Create a method in IMorality called something like synchronise and implement it in moralityScale to check that it's being called on the server (i.e. World#isRemote is false for the player's World) before sending a packet to the player with the current morality value.

 

In moralityScale#addSin, moralityScale#addVirtue and moralityScale#set (i.e. the methods that change the morality value), call the synchronise method after changing the morality value to sync the change to the client.

 

 

2 minutes ago, HalestormXV said:

Does it then need to write that using toBytes and fromBytes to a player NBT tag?

 

Byte buffers are for sending data between the client and server using packets. NBT is for storing data that will be saved to the disk.

 

You very rarely send NBT between the client and server and should never need to use byte buffers for saving data to the disk.

[HalestormXV]

Alright. And once again I apologize for my ignorance because I know once I figure it out I'm going to feel like an idiot because I know it isn't that difficult and I know this is likely just a mental block I'm not seeing past because I've done similar things to this. I think because it's I've just started remoding after a couple of years and trying to understand the whole new capabilities system I'm making this more difficult. 

 

So let me explain what I am likely going to have to do. Store the player field in the main class where I have my morality currently initialized to 0. Then on each function add in the player field (so then make changes accordingly to current spots the code is called). Add a new void SyncClient that has the Args for the morality value and the player. Create a packet that writes the passed value to the NBT (so later on if I do a bar or something the data is stored to disk client side and can be checked) am I looking st this correctly now?  Or skip NBT all together and just have the packet call my SetMoraloty function?

[Choonster]

59 minutes ago, HalestormXV said:

Store the player field in the main class where I have my morality currently initialized to 0.

In the moralityScale class, yes.

 

59 minutes ago, HalestormXV said:

Then on each function add in the player field (so then make changes accordingly to current spots the code is called).

Don't add the player as an argument to any of the functions, the moralityScale already knows which player it's attached to (the one stored in the field).

 

1 hour ago, HalestormXV said:

Add a new void SyncClient that has the Args for the morality value and the player.

Again, you don't need these arguments because the moralityScale already knows its current morality value and which player it's attached to.

 

1 hour ago, HalestormXV said:

Create a packet that writes the passed value to the NBT (so later on if I do a bar or something the data is stored to disk client side and can be checked) am I looking st this correctly now?

Data persistence and networking are completely separate. Only the server persists the game state (i.e. writes your capability to NBT and writes the NBT to the disk), the client doesn't need to persist anything.

 

1 hour ago, HalestormXV said:

Or skip NBT all together and just have the packet call my SetMoraloty function?

This is correct, just have the packet set the morality value directly; it doesn't need to do anything NBT-related.

 

 

Side note: I recommend following Java naming conventions by using PascalCase for class, enum and interface names and camelCase for field, parameter, local variable and method names.

[HalestormXV]

Spoiler

 

10 hours ago, Choonster said:

In the moralityScale class, yes.

 

Don't add the player as an argument to any of the functions, the moralityScale already knows which player it's attached to (the one stored in the field).

 

Again, you don't need these arguments because the moralityScale already knows its current morality value and which player it's attached to.

 

Data persistence and networking are completely separate. Only the server persists the game state (i.e. writes your capability to NBT and writes the NBT to the disk), the client doesn't need to persist anything.

 

This is correct, just have the packet set the morality value directly; it doesn't need to do anything NBT-related.

 

 

Side note: I recommend following Java naming conventions by using PascalCase for class, enum and interface names and camelCase for field, parameter, local variable and method names.

 

EDIT: Scratch my original post. My problems were originating from a change I made accidently and forgot about. (I've been working on this particular feature of the mod for too long, but I need to finish it for my sanity lol) All I can say is thank goodness for gitHub to track my changes. Anyway. Here is the real issue:

 

This happens on my blockbreak when morality.SyncToClient: https://pastebin.com/tWpkJBSk (Server Log)

This happens on my blockbreak wen morality:SyncToClient is called client: https://pastebin.com/K9S9B8xS (Client Log)

 

I also am getting this error: https://pastebin.com/CrzEsqNE (perhaps this is an error that is happening once it gets into the SyncMorality packet?

 

I know the client log one is not supposed to happen becasue if we are in singleplayer we don't need to sync anything to the client. What I dont know is why the !player.world.isRemote check in the actual moralityScale under this function isn't triggering. Or am i supposed to make the check before I call the actualy morality.SyncToClient in whatever class I am calling it from.

 

Here is the new moralityScale class: https://pastebin.com/FYMUNqqu

The this.syncToClient(); are all commented out under the functions that change the actual morality because I'm imagining you aren't going to call them from the scale, but you call them from where you need to sync, blockBreak, etc.? I have also tested with the syncClient function doing new SyncMorality() as opposed to new SyncMorality(this.getMorality)

 

This is the packet for the SyncMorality: (Maybe I am not supposed to return null? This is my first time using Lambda functions)

https://pastebin.com/CPEum3zq

(I also tried with runnable and the errors are the same)

 

So as you can see I followed basically all the directions which is why I don't think I am crashing anymore and am at least getting somewhere. I know I can see some light at the end of the tunnel. And I truely thank you for "teaching" me this. Not many people appreciate lessons learned like this and just want answers, answers, asnwers. I however genuinly am trying to learn this, so I greatly apprecaite what you are helping me with and I assure you every post you make that I am reading I am learning from. So nothing is falling on deaf ears.

 

 

Edited July 14, 2017 by HalestormXV

[HalestormXV]

I needed to bump this topic to mark it as what appears to be solved. (I couldn't seem to edit the topic title, but I don't want the thread locked either) After stepping away from the code for a while and working on something else it hit me like some bricks. I also took a look at the botania source code and managed to understand it because I know Vazki uses packets alot and even though our code style isn't similar I can understand it better. So in looking over the code there and then looking at my code I about pissed myself. Sending my packet to the client isn't hard once I stopped thinking so hard about it. So it seems my code works now but I would like another pair of eyes to view it to maybe confirm I am correct. Here is the code:

 

MoralityScale: https://pastebin.com/7HB7qR0x

SyncMoralityPacekt: https://pastebin.com/2V42WNPc

PacketHandler: https://pastebin.com/FRkWvCVy

Example of sending the packet in my event handler: https://pastebin.com/4T6BbZM7

 

Everything loads up fine and my debug message in the SyncMorality only shows up client side when testing. The message does not appear in the serverConsole, well because the server is sending it

 

So please if someone can do a look over and confirm with me that this is the correct way to handle this (maybe not the most efficient) but gets the job done for now at least, I'd apprecaite it. And @Choonster I CANNOT thank you enough and I apprecaite you putting up with my noobyness. But I do want you to know that I learned something from every single post and will take it with me going forward lol. Especially my naming conventions. So thanks for all that

 

And yes on my copy of the file I changed:

1.                     Minecraft mc = Minecraft.getMinecraft();
2.                     World world = mc.world;
3.                     EntityPlayer player = Minecraft.getMinecraft().player;

to 

1.                     Minecraft mc = Minecraft.getMinecraft();
2.                     EntityPlayer player = mc.getMinecraft().player;

 

Edited July 14, 2017 by HalestormXV

[Choonster]

That looks mostly correct, but I highly recommend moving the synchronisation to MoralityScale itself (like I described in this post) so the code that uses it gets the synchronisation automatically. This avoids repeating the synchronisation code in every place you change the morality value and prevents inconsistencies between places that change the value, e.g. forgetting to send the packet or sending the wrong packet.

 

Your addVirtue/addSin methods attempt to clamp the morality value between the maximum virtue and sin values but you only do this when the morality is already at the maximum, which means that any argument greater than or equal to 2 could push it over the maximum ((max - 1) + 2 = max + 1 ). I recommend using MathHelper.clamp(int, int, int) to clamp morality +/- points between the maximum virtue and sin values.

 

Don't create a new Random each time you need a random number, create one instance and store it.

 

You're still using hardcoded chat messages rather than specifying them in the lang files.

 

You should only send chat messages on one side (usually the server) to avoid sending the message twice.

 

I'm glad you've learned from this.

Edited July 14, 2017 by Choonster

[HalestormXV]

1 hour ago, Choonster said:

That looks mostly correct, but I highly recommend moving the synchronisation to MoralityScale itself (like I described in this post) so the code that uses it gets the synchronisation automatically. This avoids repeating the synchronisation code in every place you change the morality value and prevents inconsistencies between places that change the value, e.g. forgetting to send the packet or sending the wrong packet.

 

Your addVirtue/addSin methods attempt to clamp the morality value between the maximum virtue and sin values but you only do this when the morality is already at the maximum, which means that any argument greater than or equal to 2 could push it over the maximum ((max - 1) + 2 = max + 1 ). I recommend using MathHelper.clamp(int, int, int) to clamp morality +/- points between the maximum virtue and sin values.

 

Don't create a new Random each time you need a random number, create one instance and store it.

 

You're still using hardcoded chat messages rather than specifying them in the lang files.

 

You should only send chat messages on one side (usually the server) to avoid sending the message twice.

 

I'm glad you've learned from this.

 

 

I've never used the MathHelper functions in forge, maybe once in 1.7.10. I forgot all about them actually. So your saying that doing something like

public void addSin(int points) //Subtract Morality
{
    MathHelper.clamp(this.morality -= points, -400, 400)
    //this.syncToClient();
}

 

public void addVirtue(int points) //Add Morality
{
    MathHelper.clamp(this.morality += points, -400, 400);
   // this.syncToClient();
}

 

Would rectify that situation? I wasn't thinking about it currently becasue right now morality only goes up in values of 1 or -1 but I do plan to add items that will increase and decrease it by larger amounts in which case it could exceed the min and max at least intially on the item's use.

 

As for creating a random instance and storing it. Is that a matter of efficency? Or quality of the random number produced? What would the difference be between the two methods? I ask because I have always created random values this way, as a matter of fact I thought it was the best way to do it?

 

As for moving the  synchronisation  to the scale itself I am probably going to give it another shot becasue I kind of want it there also. I was just having so many problems with it being there and errors generating server crashing, etc it was absurd. (See my spoiler post). I know i was doing something wrong but I will proably give it another shot. 

 

Hardcoded messages, yes I will eventualy move away from them probably as I proceed through the mod and then ultimatey convert all my messages to my Lang. I don't use many messages as it is, it is mostly copy pasting my same messages over, so moving them to the Lang won't be so bad.

Edited July 14, 2017 by HalestormXV

[Choonster]

4 minutes ago, HalestormXV said:

I've never used the MathHelper functions in forge, maybe once in 1.7.10. I forgot all about them actually. So your saying that doing something like

 

No, you need to add/subtract points to/from morality without changing the morality field (i.e. use +/- rather than +=/-=) and then assign the return value of MathHelper.clamp to the morality field. Currently you're adding/subtracting and then completely ignoring the clamped value.

 

You should also use the existing max fields for the second and third arguments rather than using their values.

 

8 minutes ago, HalestormXV said:

As for creating a random instance and storing it. Is that a matter of efficency? Or quality of the random number produced? What would the difference be between the two methods? I ask because I have always created random values this way, as a matter of fact I thought it was the best way to do it?

 

A bit of both. Two Random objects with the same seed will always produce the same sequence of random numbers when the same methods are called on them.

 

The Random() constructor does use a unique seed each time you call it, so each of your Random objects should have a different seed and produce different random numbers.

[HalestormXV]

oh I gotcha, this.morality = this.morality - points

MathHelper.clamp(this.morality, this.maxSin, this.MaxVirute, and vice versa for the adding. This way the min and max are obeyed if the player uses an item that icnreases their virtue by 50 and are already at 380 or something like that. And yeah the fields for min and max should have been used since they are going to be configurable in the future but I was typing to quick and not paying attention.

 

that is indeed a nifty little piece of code. I gotta get more familar with that one.

Edited July 14, 2017 by HalestormXV

[Choonster]

23 minutes ago, HalestormXV said:

oh I gotcha, this.morality = this.morality - points

MathHelper.clamp(this.morality, this.maxSin, this.MaxVirute, and vice versa for the adding. This way the min and max are obeyed if the player uses an item that icnreases their virtue by 50 and are already at 380 or something like that. And yeah the fields for min and max should have been used since they are going to be configurable in the future but I was typing to quick and not paying attention.

 

You need to assign the return value of MathHelper.clamp to the morality field. You can pass the result of the addition/subtraction directly to MathHelper.clamp.

Edited July 14, 2017 by Choonster

[HalestormXV]

Got it, okay. It took me a minute. The first value is the number that is "clamped" (forced) to be within the second and third values and is returned. So by assining a new "temp" int just within that add or subtract function to be the result of the calculation of this.morality + points or - points I can then pass that "temp" int into the value area of .clamp then set value 1 and value 2 to the min and max and then return that result ("the temp" int) to = this.morality value.

[Choonster]

1 minute ago, HalestormXV said:

Got it, okay. It took me a minute. The first value is the number that is "clamped" (forced) to be within the second and third values and is returned. So by assining a new "temp" int just within that add or subtract function to be the result of the calculation of this.morality + points or - points I can then pass that "temp" int into the value area of .clamp then set value 1 and value 2 to the min and max and then return that result ("the temp" int) to = this.morality value.

 

Essentially, yes. The "temp" int doesn't need to be a variable, you can just pass it directly to MathHelper.clamp.

[HalestormXV]

1 hour ago, Choonster said:

 

Essentially, yes. The "temp" int doesn't need to be a variable, you can just pass it directly to MathHelper.clamp.

Got it and tested and it is working now. All that is left is trying to figure out how to sync within the scale itself and not error out all the time lol. 

I just cant understand why if this:

if (!player.world.isRemote) { eAngelusPacketHandler.sendTo(new SyncMorality(morality.getMorality()), (EntityPlayerMP) player);}

works in my eventHandler on the block break event, it doesn't work in the scale class, but rather than have morality.getMorality() I try this.getMorality since it is only fetching teh data from itself. It keeps spitting that entiyNBT is null. It even does it when i actually put in a null check. Thats why the whole making it sync in teh scale itself is so bothersome.

Edited July 14, 2017 by HalestormXV