Skip to content

feat: PubSub improvement: ISubscriber unsub [MTT-2765] #612

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
Merged
merged 15 commits into from
Sep 29, 2022
Merged

feat: PubSub improvement: ISubscriber unsub [MTT-2765] #612

merged 15 commits into from
Sep 29, 2022

Conversation

pdeschain
Copy link
Contributor

@pdeschain pdeschain commented Apr 13, 2022 •
edited
Loading

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
View reviewed changes
@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
View reviewed changes
@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
@pdeschain
Addressing comments
5844c0c
@pdeschain
Merge branch 'develop' into pdeschain/isubscriber-unsub
5ba581e 
# Conflicts:
#	Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs
#	CHANGELOG.md
@pdeschain
removed unnecessary assert in MessageChannel Unsubscribe and replaced…
9ffda69 
… it with conditional
SamuelBellomo
SamuelBellomo previously approved these changes Sep 29, 2022
View reviewed changes
LPLafontaineB
LPLafontaineB previously approved these changes Sep 29, 2022
View reviewed changes
@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 previously approved these changes Sep 29, 2022
View reviewed changes
@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
@SamuelBellomo
Merge branch 'develop' into sam/feat/bump-NGO-to-1.1
c13ba78 
* 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
Reviewers

@fernando-cortez fernando-cortez fernando-cortez approved these changes

@LPLafontaineB LPLafontaineB LPLafontaineB approved these changes

@SamuelBellomo SamuelBellomo Awaiting requested review from SamuelBellomo

Assignees
No one assigned
Labels
2-Easy This PR is trivial and can be reviewed quickly 2-Reviewed with Comments PR requires owner's attention
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pdeschain @SamuelBellomo @fernando-cortez @LPLafontaineB @netcode-ci-service