[1.8] [Partially Solved] RenderPlayerEvent is rendering to the wrong player

[TrashCaster]

So basically, what I'm doing is rendering accessories to the player (for testing purposes, these are just block items in my custom inventory). I have the inventory working properly, and the packets to synchronize the inventory to the clients (so they can display objects from the contents).

 

But for whatever reason, when I'm trying to render to a player, it renders to only the local player, and it renders items based on all other players in view.

 

For example, my current setup allows you to put block items into the inventory. When it renders the player, it will render the block item model at the player's location, with a vertical offset equal to 0.5 + the slot index, so as to make a "stack".

 

So to explain what is currently happening, here's a representative scenario.

• Server has players Bill, and Jack
  
• Bill has a dirt block in his first slot
  
• Jack has an oak log in his second slot
  
• On Jack's screen, while in third person, he sees a dirt block near his feet, and a log in his torso. He does not see the log on Bill anywhere
  
• On Bill's screen, while in third person, he sees a dirt block near his feet, and a log in his torso. He does not see the dirt on Jack anywhere
  
• Bill turns away so that Jack isn't visible on his screen, and while in third person, he now sees his dirt block near his feet
  
• Jack now takes the log out of his inventory, and Bill turns back
  
• On Jack's screen, while in third person, he sees a dirt block near his feet
  
• On Bill's screen, while in third person, he sees a dirt block near his feet
  

 

So what is happening? Well, simply put, each player on the server has their information in sync on the clients. The RenderPlayerEvent is supposed to fetch the correct inventory based on the player it is rendering. It does this. However, it never renders to that player. It simply renders to the local player.

 

Is this something I can fix/work around, or is this something within Forge that needs fixing?

 

Here's my GitHub repo:

https://github.com/TrashCaster/TheStuffMod-2.0

[coolAlias]

Why don't you just retrieve the inventory from the client player's IEEP when rendering, instead of the convoluted and unnecessary ClientProxy static Map business? Having that map in the ClientProxy means that each player really has their own instance of the Map, despite you marking it as static - each client is a separate application that can only share information with the server via packets, which shares that information with other clients. Static client data doesn't behave as one might expect.

 

As long as you send each player's IEEP data to any other players that begin tracking him/her, those other players will have the information they need to fetch that player's inventory from IEEP when rendering.

 

As far as the positioning of the items, I'm not really sure what's going on with that. Maybe it will sort itself out after cleaning up.

 

EDIT: Also, you should register your packets in a common area of the code, they don't need to be (and shouldn't be) segregated. Don't mark your message handler as @SideOnly anything, either. That is pointless and harmful. Furthermore, packets that set important data, such as your sync packet, should only ever be sent from the server to the client, never the other way around. I suggest you remove all the code in that packet class / registration that has to do with sending to the server.

[TrashCaster]

Thanks for the quick reply.

 

As far as packets from client to server, the only one I send from client to server is Message which tells the server that the player attempted to open the accessory inventory, which the server then submits to the GuiHandler.

 

And for storing the inventory in a client side IEEP versus the hashmap, I did do that before, and didn't have the results I was expecting, so I thought changing this may be better, but only found it to have the same end result. I will switch back.

 

I had followed your guides for all the inventory stuff, but the "sync packet" area was a little vague, so I put it into ContainerAccessories in the detectAndSendChanges method. I used the packet tutorial from diesieben07, so I may have to re-read that to see if there's something I missed, though it is already working perfectly fine for the messages.

 

The reason why I used the HashMap instead was also because when a client receives the sync packet, they get the UUID of the player to update, and the NBT tag of the data (whether the be the whole IEEP, or just the inventory). And the client handler isn't able to track players from other worlds (dimensions). So I just wanted to let them be in sync too. But I guess that'll get corrected when they step through a portal, and fire off another EntityJoinWorldEvent, right? I just thought as seamless as possible would be best.

 

So, correct me if I'm wrong when the client receives the sync packet, the handler should do the following:

    EntityPlayer player = Minecraft.getMinecraft().theWorld.getPlayerEntityByUUID(message.playerID);
    ExtendedPlayer.get(player).loadNBTData(message.tag);

[coolAlias]

Pretty much, yeah, but you shouldn't send the packets to everyone all the time - only players that are currently tracking each other need to know about this stuff, so you can get by using the EntityTracker#sendToAllTrackingEntity method. Get the EntityTracker instance for the player from WorldServer.

[TrashCaster]

Pretty much, yeah, but you shouldn't send the packets to everyone all the time - only players that are currently tracking each other need to know about this stuff, so you can get by using the EntityTracker#sendToAllTrackingEntity method. Get the EntityTracker instance for the player from WorldServer.

 

Alright, and then because I'm sending the packet with the inventory changes (detectAndSendChanges), it should be like this:

// New code
    	if (!this.player.worldObj.isRemote) {
    	    EntityTracker tracker = ((WorldServer)this.player.worldObj).getEntityTracker();
    	    tracker.sendToAllTrackingEntity(this.player, TSM.NETWORK.getPacketFrom(new SyncPlayerPropsMessage(this.player)));
    	}

 

as opposed to my previous

// Previous code
    	if (!this.player.worldObj.isRemote) {
            TSM.NETWORK.sendToAll(new SyncPlayerPropsMessage(this.player));
    	}

 

Is that right?

[TrashCaster]

Also, (and sorry for the double post, I sent the previous off without adding this bit, and don't want to get ninja'd), I moved the packet registration out of the proxies, and into the mod class, as you suggested.

 

It looks like this:

 

    @EventHandler
    public void preInit(FMLPreInitializationEvent event)
    {
        NETWORK = NetworkRegistry.INSTANCE.newSimpleChannel("TSMnet");
        NETWORK.registerMessage(Message.ServerHandler.class, Message.class, 1, Side.SERVER);
        NETWORK.registerMessage(Message.ClientHandler.class, Message.class, 1, Side.CLIENT);
NETWORK.registerMessage(SyncPlayerPropsMessage.ClientHandler.class, SyncPlayerPropsMessage.class, 2, Side.CLIENT);
    	PROXY.preInit();
    }

 

I took out the line:

        NETWORK.registerMessage(SyncPlayerPropsMessage.ServerHandler.class, SyncPlayerPropsMessage.class, 2, Side.SERVER);

 

cause the server doesn't need to handle the packet, right?

 

 

EDIT:

So that crashed, cause I derped and let the server try to load client side code. I moved those lines back into the ClientProxy. To be honest though, I see no problem with having the server lines in the CommonProxy, since the ClientProxy extends it, and calls the super preInit anyways.

 

 

EDIT 2:

And when I removed the aforementioned line, it caused the client to disconnect. I added it back, and the client can connect. So the server still needs to register the handler and message, even if it doesn't do anything with it.

 

 

 

And that wraps up the networking part. Back to square one, with potential optimizations. Now how about the RenderPlayerEvent issue?

[coolAlias]

Those network registrations should NOT have caused a crash - you're crashing because you have @SideOnly(Side.CLIENT) on your ClientHandler class, which I mentioned earlier you shouldn't. If that wasn't the cause of the crash, post the crash log.

 

Anyway, I'm not sure about the rendering. It could be something to do with the current render view entity, as every player posts the RenderPlayer event for every other player, meaning:

 

Player A w/ dirt block

Player B w/ log block

 

Every render tick:

Player A:

  renders self w/ dirt block

  renders player B, but current GL matrix is relative to current player A, so log shows up at A's feet

 

Player B:

  renders self w/ log block

  renders player A, but current GL matrix is relative to current player B, so log shows up at B's feet

 

Just a guess. I haven't messed with that kind of rendering too much, and what little I did was many months ago (probably over a year).

[TrashCaster]

Those network registrations should NOT have caused a crash - you're crashing because you have @SideOnly(Side.CLIENT) on your ClientHandler class, which I mentioned earlier you shouldn't. If that wasn't the cause of the crash, post the crash log.

 

-snip-

 

Ok, so maybe I should invert the transform, and then re-apply it using the other player's transform?

 

Like so:

EntityPlayer p1 = Minecraft.getMinecraft().thePlayer;
double x1 = p1.prevPosX + (p1.posX - p1.prevPosX)*event.partialTicks;
double y1 = p1.prevPosY + (p1.posY - p1.prevPosY)*event.partialTicks;
double z1 = p1.prevPosZ + (p1.posZ - p1.prevPosZ)*event.partialTicks;

EntityPlayer p2 = event.entityPlayer;
double x2 = p2.prevPosX + (p2.posX - p2.prevPosX)*event.partialTicks;
double y2 = p2.prevPosY + (p2.posY - p2.prevPosY)*event.partialTicks;
double z2 = p2.prevPosZ + (p2.posZ - p2.prevPosZ)*event.partialTicks;

GL11.glPushMatrix();
GL11.glTranslated(-x1,-y1,-z1);
GL11.glTranslated(x2,y2,z2);
// Render
GL11.glPopMatrix();

 

EDIT:

Just tried this, and it does work. However, one issue is if a player adjusts their inventory, the old information is merged with new information. So basically, anything that became null isn't turning null on the client. So maybe upon receiving the packet (on the client), the player inventory should be cleared, and then have the NBT applied, since the packet won't send null information.

 

 

EDIT 2:

Just tried clearing it client side before the NBT gets loaded, and that fixed it.

The only issue I have now is that upon joining the world, the player won't fetch the information from the server.

 

Does the JoinWorldEvent fire on just the server, or on the client too? Cause my thought is to make the client send a request packet to the server, and the server responds with a sync packet.

 

To elaborate on the above, here's another infamous representative scenario:

• Bob joins world. Doesn't see his items. He opens his accessory inventory, causing sync packet. Now he sees his items
  
• Jack joins world. Doesn't see his items, nor Bob's items. Jack opens his accessory inventory, causing a sync packet.
  
• Jack and Bob both see Jack's items.
  
• Bob opens accessory inventory again, and now Jack can see his items.
  

 

For the join world I used tracking entities, as mentioned for the inventory changes being sent.

 

Code for if you don't want to re-snoop my GitHub:

if (event.entity instanceof EntityPlayer) {
    if (!event.world.isRemote) {
        EntityTracker tracker = ((WorldServer)event.world).getEntityTracker();
        tracker.sendToAllTrackingEntity((EntityPlayer)event.entity, TSM.NETWORK.getPacketFrom(new SyncPlayerPropsMessage((EntityPlayer)event.entity)));
        Set<EntityPlayer> otherPlayers = tracker.getTrackingPlayers(event.entity);
        for (EntityPlayer p:otherPlayers) {
            TSM.NETWORK.sendTo(new SyncPlayerPropsMessage(p), (EntityPlayerMP)event.entity);
        }
    }
}

 

 

EDIT 3:

I think it is to do with the data saving/loading on the server being iffy, and not sending proper information to the clients. I tried scheduling a sync in the update event to see if it does sync it, but it never does unless the players open their inventories.

 

My GitHub is updated to the latest source, so if someone wouldn't mind looking through it to get me back on track with proper synchronizing, that'd be swell.

It feels as though I tried to cover as many bases as possible, and have sync calls that are redundant, or wrong. I'd like to minimize the calls, while maintaining accurate synchronization between client and server.

[coolAlias]

That's the problem with syncing only from #detectAndSendChanges - you should also send the inventory update packet to all tracking players from PlayerEvent.StartTracking, and the combination should cover all your bases.

 

As for your EDIT 2 about needing to clear the inventory, that's true because if the slot wasn't sent in the packet, that means there was nothing in it, but the client will not write to that slot in the basic NBTTagList-style loop.

[TrashCaster]

So to recap, resync on:

• Player join world
  
• Player start tracking
  
• Detect and send changes
  

 

Because currently it syncs from everything but the "start tracking". I had to use an ArrayList to contain the tracking players, as well as the active player, since I suppose they don't track themselves (I tried just the Set returned by the tracker, and the active player never received the sync).

 

I also unified the calls to a single method, just to reduce duplicate code that may stop working due to not changing all of the occurrences.

 

It is working really well right now too.

StartTracking event will basically be the occurrence that a player suddenly appears on a player's screen due to them being close enough, correct?

[coolAlias]

Yes, the event fires any time a client player starts receiving updates from the server about any entity, e.g. in order to render that entity on the screen.

 

You probably don't need to send anything when the player joins the world except to that 1 player (so they can render their items in 3rd person); other players will be handled by the StartTracking event.

 

You shouldn't need an ArrayList to store any players at all - that's the whole point of the EntityTracker: use the method that gives you all tracking players for an entity. That's all you need.

[TrashCaster]

Yes, the event fires any time a client player starts receiving updates from the server about any entity, e.g. in order to render that entity on the screen.

 

You probably don't need to send anything when the player joins the world except to that 1 player (so they can render their items in 3rd person); other players will be handled by the StartTracking event.

 

You shouldn't need an ArrayList to store any players at all - that's the whole point of the EntityTracker: use the method that gives you all tracking players for an entity. That's all you need.

 

No, I just meant in the one call as a temporary list, so that I could iterate through all of them, and not write another "send" line to the active player. More of a code-style preference. But I will keep the rest in mind, thanks a bunch. It's always nice to get help on even the basics.