Skip to content
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

Core: Remove CCharEntity::pushPacket(raw packet) #6713

Merged
merged 12 commits into from
Jan 15, 2025
Merged

Conversation

zach2good
Copy link
Contributor

@zach2good zach2good commented Jan 14, 2025

I affirm:

  • I understand that if I do not agree to the following points by completing the checkboxes my PR will be ignored.
  • I understand I should leave resolving conversations to the LandSandBoat team so that reviewers won't miss what was said.
  • I have read and understood the Contributing Guide and the Code of Conduct.
  • I have tested my code and the things my code has changed since the last commit in the PR and will test after any later commits.

What does this pull request do?

Closes #6545

After looking here: I realised how easy it still is to leak packets: #6711

A big step towards: #5171

The scope of this blew up, as always. Now, CBasicPacket is ONLY a wrapper around std::array<uint8, PACKET_SIZE> buffer_;. It doesn't do non-owning, I've audited how they're constructed, etc. We also now NEVER new a packet - it's always constructed as a unique_ptr, and we are careful to pass around references or move them as needed. This also means that we have to be obvious when we wither std::move() or ->copy() packets in places.

But now, there's no question about what happens where - we can see what our data is doing.

Testing

  • Log in
  • /wave
  • Zone into {Dynamis - Bastok}
  • Spells -> targeting failure
  • RA -> targeting failure
  • Action packet
  • Mounts
  • Chocobo raising menu
  • Train everything around
  • Crafting
  • Interacting with NPCs
  • Moghouse
  • Party
  • Trusts
  • Linkshell
  • IPC/ZMQ
  • Tell/Party/Shout/Yell
  • Events with updates (BCNM entry, Instance entry)
  • BCNMs
  • Instances
  • Trading
  • AH
  • Monstrosity

@zach2good zach2good force-pushed the remove_push_packet_raw branch from f7d060b to d7b3bc4 Compare January 15, 2025 10:42
@zach2good zach2good force-pushed the remove_push_packet_raw branch from f8e58eb to 80afa51 Compare January 15, 2025 11:56
@zach2good zach2good marked this pull request as ready for review January 15, 2025 12:25
@zach2good
Copy link
Contributor Author

This is now very well tested and ready for review

@zach2good zach2good force-pushed the remove_push_packet_raw branch from e18f699 to 98ae9d1 Compare January 15, 2025 12:42
@zach2good zach2good merged commit 72170d4 into base Jan 15, 2025
13 checks passed
@zach2good zach2good deleted the remove_push_packet_raw branch January 15, 2025 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Outgoing synth packet is always "your" synth result even though its from "others"
3 participants