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

[MTTB-409] Fix a Healer ability doesn't work (#893) #893

Merged
merged 3 commits into from
Dec 12, 2024

Conversation

nanho-lee
Copy link

Description

This is the PR that fixed the following issue.
To solve the above issue, I did three things.

1.

Fixed a bug where the character's orientation was flipped if the target and current location were almost the same when the skill was triggered. In that case, the character's orientation is used.
ClientInputSender.cs

//there is a bug where the direction is flipped if the hitPos and current position are almost the same,
//so we use the character's direction instead.
float directionLength = offset.magnitude;
Vector3 direction = float.Epsilon >= directionLength ? (offset / directionLength) : m_PhysicsWrapper.Transform.forward;

2.

mttb_00
mttb_01
Despite being close enough, the Box cast would often fail depending on the angle. To compensate for this, I added the Sphere cast.
I considered creating a new Action for this, but since the logic is almost identical to the MeleeAction, I decided to only use the Sphere cast when the Radius value is set.

3.

If there are multiple targets that are checked for collisions, I've added the GetTotalDamage API to IDamagable to select the most damaged target. I felt that this prioritization was a good one, whether the MeleeAction is attack or recovery.
Note that I kept the ability to prioritize targets with hint targets for targeting.

Issue Number(s)

MTTB-409
https://jira.unity3d.com/browse/MTTB-409

Contribution checklist

  • Tests have been added for boss room and/or utilities pack
  • Release notes have been added to the project changelog file and/or package changelog file
  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • JIRA ticket ID is in the PR title or at least one commit message
  • Include the ticket ID number within the body message of the PR to create a hyperlink
  • An Index entry has been added in readme.md if applicable

@nanho-lee nanho-lee requested a review from a team as a code owner December 4, 2024 02:39
@unity-cla-assistant
Copy link

unity-cla-assistant commented Dec 4, 2024

CLA assistant check
All committers have signed the CLA.

Elfi0Kuhndorf
Elfi0Kuhndorf previously approved these changes Dec 9, 2024
Copy link
Contributor

@Elfi0Kuhndorf Elfi0Kuhndorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Works now as expected. Thank you for fixing this!

Copy link
Collaborator

@fernando-cortez fernando-cortez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All that's left is to merge develop, and add a note in our changelog with this fix. Make sure to add the commit number to the changelog line -- the changelog should have examples of this too.
Looks great!

@nanho-lee nanho-lee closed this Dec 10, 2024
@nanho-lee nanho-lee reopened this Dec 10, 2024
@nanho-lee nanho-lee changed the title Fix a Healer ability doesn't work [MTTB-409] Fix a Healer ability doesn't work (#893) Dec 10, 2024
@nanho-lee
Copy link
Author

Thank you both for your reviews.
I have modified the log and CHANGELOG files as you said.

Elfi0Kuhndorf
Elfi0Kuhndorf previously approved these changes Dec 11, 2024
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@fernando-cortez fernando-cortez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing!

@fernando-cortez fernando-cortez merged commit 31c3c3f into develop Dec 12, 2024
13 checks passed
@fernando-cortez fernando-cortez deleted the bugfix/mttb-585 branch December 12, 2024 15:46
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.

4 participants