Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix: Added null reference check to ClientInputSender script [MTT-6672] #880

Merged
merged 9 commits into from
Mar 12, 2024

Conversation

Elfi0Kuhndorf
Copy link
Contributor

Description

Added null reference checks to ClientInputSender script in order to remove null reference exemption when using an unavailable ability.

Validation:

  1. Open Boss room and press play
  2. Start a game and choose Tank or Mage class in character selection
  3. Press 3 on keyboard (notice in skill UI no ability 3 available for these classes)
  4. Notice no null reference coming up

Issue Number(s)

MTT-6672

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

@Elfi0Kuhndorf Elfi0Kuhndorf added the 2-Easy This PR is trivial and can be reviewed quickly label Feb 29, 2024
@Elfi0Kuhndorf Elfi0Kuhndorf requested a review from a team as a code owner February 29, 2024 13:20
@Elfi0Kuhndorf Elfi0Kuhndorf self-assigned this Feb 29, 2024
@Elfi0Kuhndorf Elfi0Kuhndorf requested a review from a team February 29, 2024 14:46
fernando-cortez
fernando-cortez previously approved these changes Mar 1, 2024
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.

Approving, but marking this as On Hold until this PR lands first.

@fernando-cortez fernando-cortez added 4-On Hold PR can't proceed because it's blocked or is otherwise waiting on something. and removed 4-On Hold PR can't proceed because it's blocked or is otherwise waiting on something. labels Mar 1, 2024
Copy link

@RikuTheFuffs RikuTheFuffs left a comment

Choose a reason for hiding this comment

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

lots of unneeded formatting changes that reduce readability, can you please remove them?

Assets/Scripts/Gameplay/UserInput/ClientInputSender.cs Outdated Show resolved Hide resolved
@unity-cla-assistant
Copy link

unity-cla-assistant commented Mar 7, 2024

CLA assistant check
All committers have signed the CLA.

RikuTheFuffs
RikuTheFuffs previously approved these changes Mar 7, 2024
Copy link

@RikuTheFuffs RikuTheFuffs 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 now!

@Elfi0Kuhndorf
Copy link
Contributor Author

Elfi0Kuhndorf commented Mar 11, 2024

looks good now!

Thanks! I would need an approving review (due to develop being merged into it I think), then I can merge it :)

RikuTheFuffs
RikuTheFuffs previously approved these changes Mar 11, 2024
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Fernando Cortez <fernando.cortez@unity3d.com>
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.

Looks good!

@Elfi0Kuhndorf Elfi0Kuhndorf merged commit 384cfc7 into develop Mar 12, 2024
13 checks passed
@Elfi0Kuhndorf Elfi0Kuhndorf deleted the fix-unavailable-abilities-null-reference branch March 12, 2024 14:32
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
2-Easy This PR is trivial and can be reviewed quickly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants