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

fix: replace null-coalescencing and null-conditional operators [MTT-7125] #867

Merged
merged 10 commits into from
Sep 6, 2023

Conversation

LPLafontaineB
Copy link
Contributor

@LPLafontaineB LPLafontaineB commented Aug 29, 2023

Description

This PR replaces occurrences of the null-coalesing (??=) and null-conditional (?.) operators in the project. These operators can cause issues when used with types inheriting UnityEngine.Object because that type redefines the == operator to define when an object is null. This redefinition applies to regular null checks (if foo == null) but not to those operators, thus this could lead to unexpected behaviour.

While those operators were safely used within Boss Room, only with types that were not inheriting UnityEngine.Object, we decided to remove most usages for consistency. This will also help avoid accidental mistakes, such as a user reusing a part of this code, but modifying it so that one of those operators are used with a UnityEngine.Object.

The only cases where we kept the null-conditional operator were with events/delegates/actions, for which there is no risk of confusion.

Issue Number(s)

MTT-7125

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

@LPLafontaineB LPLafontaineB marked this pull request as ready for review August 29, 2023 15:46
@LPLafontaineB LPLafontaineB requested a review from a team as a code owner August 29, 2023 15:46
@LPLafontaineB LPLafontaineB added 2-Easy This PR is trivial and can be reviewed quickly 1-Needs Review PR needs attention from the assignee and reviewers labels Aug 29, 2023
MissCaligari
MissCaligari previously approved these changes Sep 5, 2023
@LPLafontaineB LPLafontaineB merged commit 5a1de63 into develop Sep 6, 2023
@LPLafontaineB LPLafontaineB deleted the fix/replace-null-coalescence-operators branch September 6, 2023 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1-Needs Review PR needs attention from the assignee and reviewers 2-Easy This PR is trivial and can be reviewed quickly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants