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.

Sign up for GitHub

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

Jump to bottom

Companions - WIP #312

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Companions - WIP #312

wants to merge 4 commits into from

Conversation

SalmanTKhan
Copy link
Contributor

@SalmanTKhan SalmanTKhan commented Aug 24, 2024 •
edited
Loading

  • Base Implementation
  • Loading/Saving both in Barracks and Zone Server
  • Able to mount companions with proper class (Falcon/Flying pets don't work yet).
  • Known Issues
  • Changing maps while riding, you have to remount the companion.
    Companion list in barracks works for one companion; after that, it breaks.
    Companion Buff Logic has to be refactored.

@SalmanTKhan SalmanTKhan linked an issue Aug 24, 2024 that may be closed by this pull request
Companions #305
Open
@SalmanTKhan SalmanTKhan requested a review from exectails August 24, 2024 06:00
@exectails exectails marked this pull request as draft August 27, 2024 21:43
@exectails
Copy link
Member

Thanks for the PR! I'm going to convert this to a draft for now, because of the known issues, which sound like this isn't quite ready to be pulled into master. The barracks issue in particular sounds like the packet(s) aren't correct yet. Did you base your implementation on logs or is the structure guessed?

@SalmanTKhan
Copy link
Contributor Author

Thanks for the PR! I'm going to convert this to a draft for now, because of the known issues, which sound like this isn't quite ready to be pulled into master. The barracks issue in particular sounds like the packet(s) aren't correct yet. Did you base your implementation on logs or is the structure guessed?

Sounds good.

As always based-off packet logs and then guessed, because you know there are certain nuances like a byte + 3 gap/filler bytes, that are common in ToS packet structure, that I tend to miss. There used to be Pet Weapon/Armor slots in the Barrack PetInfo packet but as you know IMC removed them and now I have no clue what's in there. The structure seems fine in the packet template but when applying it to the actual in-game, it doesn't work.

@exectails exectails mentioned this pull request Sep 13, 2024
Draft
@SalmanTKhan
Companions - WIP
14f87df 
* Base Implementation
* Known Issues
 - Changing map while riding, you have to remount the companion.
 - Companion list in barracks works for 1 companion, after that it breaks.
 - Companion Buff Logic has to be refactored.
@SalmanTKhan
Fixed Buff Logic
86b6ff3 
* Added buff handler, so we don't end up with a permanent BM bonuses.
@SalmanTKhan
Official pet chat commands
f08ea63
@exectails
Copy link
Member

Based on the discussion on Discord, it seems like the second known issue is fixed now, and I also see a commit addressing buff issues. Does that mean only the first known issue (remounting) is left?

@SalmanTKhan SalmanTKhan marked this pull request as ready for review October 13, 2024 15:40
@SalmanTKhan
Copy link
Contributor Author

Based on the discussion on Discord, it seems like the second known issue is fixed now, and I also see a commit addressing buff issues. Does that mean only the first known issue (remounting) is left?

That is correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@exectails exectails Awaiting requested review from exectails

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Companions
2 participants
@SalmanTKhan @exectails