-
Notifications
You must be signed in to change notification settings - Fork 650
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Position packet 0x05B and rename some entity update packets #6902
base: base
Are you sure you want to change the base?
Conversation
These functions are practically identical except for some GM handling.
This will allow renaming CCharPacket to CCharUpdatePacket, to match CEntityUpdatePacket.
Functionally similar to CEntityUpdatePacket so rename to reflect that.
d501691
to
acf355b
Compare
odd. 0x05B and 0x065 are apparently the same per atom0s decomp, but sometimes we sent both? curious... |
Yeah I'm not sure when one is used over another, and I didn't update 0x065 yet, figured I'd wait on review for this one since it should just be a copy/paste job. |
when you tested (such as drawin/setpos) did you use two chars? |
Yep, everything looked right from both perspectives. That's actually how I came about adding |
Just for reference, I think the only changes here that might have any visible effect are that NPCs no longer send an entity update packet when drawn in, instead both players and NPCs use a position packet, and SmallPacket0x05C (CS position update) no longer sends a position packet or CS position packet (updatePosition looks like it's always false). SmallPacket0x05C looks like it's in an incomplete state right now anyway and positional updates are likely all handled with setPos and the entity update packet so this shouldn't change anything either. Everything else is mostly just renaming or currently unused. I didn't notice anything breaking, but those would be the places to keep an eye on if there are any issues. |
yeah, I was vaguely aware of this when I rewrote the char update packets to better match the decomps |
I affirm:
What does this pull request do?
Couple of things in here. Best to look at each commit individually.
First, I've updated updateEntityPacket to handle all entities, removing the need for a separate updateCharPacket. I believe the plan is to eventually move this all into pushPacket but this keeps things in one place for now.
Second requires a little background. Incoming packets 0x000D and 0x000E handle entity updates for PCs and NPCs respectively. Currently, the naming of these packets is confusing. There is CCharPacket for PC updates and CEntityUpdatePacket for NPCs. CCharUpdatePacket, which you might think is related to CEntityUpdatePacket, is a packet used only to update the local player (0x0037). So I've changed the naming so that CCharUpdatePacket and CEntityUpdatePacket serve the same purpose for updating PC and NPC entities respectively, and changed the 0x0037 packet to CCharStatusPacket. I didn't really know what to go with with 0x0037 (XiPackets I guess would refer to this as ServerStatus, but I didn't think that was very clear for our use here) so I'm open to suggestions if you don't like this.
Finally, this reworks the Position packet (0x005B) to be closer to retail. I don't think this packet is being used correctly at all right now. It looks like it's only sending for the local player, and we try to be efficient with it by updating the pointer, but this doesn't seem to be how it's used. It appears that this is sent to all players in range to update an entity's position for certain conditions, possibly to save space from doing a full entity update packet? For example, if your trust is drawn in, you receive this packet with the trust as the target entity. Similarly this packet seems to be used in some way when popping or defeating an entity. In WotG zones this packet seems to be spammed about 10 times per second for some invisible NPC... This PR doesn't attempt to update when/where this packet is used except in a few places where it's already being used, it mostly just makes it available for use.
The client automatically moves the player when it receives this packet, so I've also moved the server position setting logic into the packet constructor to somewhat enforce correct usage. The CCSPositionPacket seems to be just a copy of the position packet, but I haven't updated that one yet.
There's also a small update to the draw in messaging packet to use CHAR_INRANGE_SELF instead of CHAR_INRANGE that should fix #6738.
Steps to test these changes
I tested Divine Might to ensure CLuaBaseEntity::teleport works, LB10 Beyond Infinity and homepoint warps to test setPos, draw in.
Position packet modes can be tested with the positionSpecial binding:
Can adjust the pos and mode to try different things. Careful setting mode 8 (lock) as it will lock your client (including text input) until you get a mode 5 or 9 packet to unlock.