Skip to content

Conversation

@pdeschain
Copy link
Contributor

@pdeschain pdeschain commented Apr 13, 2022

Description

Turned out to be a one-liner change that would allow us to also Usubscribe from ISubscribers.

Issue Number(s)

MTT-2765

Contribution checklist

  • 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
  • Added to changelog

@pdeschain pdeschain added 2-Easy This PR is trivial and can be reviewed quickly 1-Needs Review PR needs attention from the assignee and reviewers labels Apr 13, 2022
@SamuelBellomo
Copy link
Contributor

With this, I would have expected to see changes in callsites of ISubscriber no? What kinds of improvements can we do with this?

@SamuelBellomo
Copy link
Contributor

Keep Dispose for bundled dispose, for single dispose, can use symmetrical unsubscribe.

@SamuelBellomo SamuelBellomo added 2-Reviewed with Comments PR requires owner's attention and removed 1-Needs Review PR needs attention from the assignee and reviewers labels Apr 26, 2022
@SamuelBellomo
Copy link
Contributor

Adding this for all PRs that have been opened since this new flow has been added. Please add your changelog here project changelog file and/or package changelog file

@pdeschain pdeschain added 1-Needs Review PR needs attention from the assignee and reviewers and removed 2-Reviewed with Comments PR requires owner's attention labels Aug 11, 2022
fernando-cortez
fernando-cortez previously approved these changes Sep 7, 2022
@fernando-cortez fernando-cortez added 2-Reviewed with Comments PR requires owner's attention and removed 1-Needs Review PR needs attention from the assignee and reviewers labels Sep 7, 2022
SamuelBellomo
SamuelBellomo previously approved these changes Sep 9, 2022
@SamuelBellomo SamuelBellomo added 0-URGENT Blocker for a release and needs to be merged ASAP and removed 0-URGENT Blocker for a release and needs to be merged ASAP labels Sep 22, 2022
# Conflicts:
#	Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs
#	CHANGELOG.md
SamuelBellomo
SamuelBellomo previously approved these changes Sep 29, 2022
LPLafontaineB
LPLafontaineB previously approved these changes Sep 29, 2022
@LPLafontaineB LPLafontaineB added 3-Good to Merge and removed 2-Reviewed with Comments PR requires owner's attention labels Sep 29, 2022
@pdeschain pdeschain enabled auto-merge (squash) September 29, 2022 18:33
@fernando-cortez fernando-cortez added 2-Reviewed with Comments PR requires owner's attention and removed 3-Good to Merge labels Sep 29, 2022
@pdeschain pdeschain merged commit 5f1bccc into develop Sep 29, 2022
@pdeschain pdeschain deleted the pdeschain/isubscriber-unsub branch September 29, 2022 20:52
SamuelBellomo added a commit that referenced this pull request Oct 13, 2022
* develop: (60 commits)
  fix: removing FindObjectOfType for a serialized reference (#754)
  fix: can click through loading screen [MTT-4753] (#760)
  Update CHANGELOG.md (#765)
  feat: Architecture.md update (MTT-2637) (#763)
  Update README.md (#759)
  Updating utilities package for release (#764)
  Version bumps (#761)
  Updating broken PR links (shouldn't be linked to jira) and changelog fixes (#758)
  adding components refs to index (#757)
  Adding link to feedback form at the top (#756)
  chore: adding details to changelog (#746)
  Updating readme with Typeform for feedback (#755)
  made arrow trails smaller (#750)
  Fixed up some layer settings on some archer VFX so that they show up properly! (#739)
  fix: lobby UI unblocking before it should [MTT-4579] (#748)
  feat: PubSub improvement: ISubscriber unsub [MTT-2765] (#612)
  Fix: Reduction of SSAO Cost (Especially on Mac) [MTT-4558] (#753)
  fix: Moving utp to component instead of child GameObject (#752)
  Adding TOC and index to readme [MTT-4617] (#736)
  Feat: Char Portrait Mouse Hover Feedback [MTT-4754] (#751)
  ...

# Conflicts:
#	Assets/Prefabs/NetworkingManager.prefab
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2-Easy This PR is trivial and can be reviewed quickly 2-Reviewed with Comments PR requires owner's attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants