Skip to content

fix: avoid throwing exception when using NetworkList without a NetworkManager (#2539) #2540

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
Closed
wants to merge 1 commit into from
Closed

fix: avoid throwing exception when using NetworkList without a NetworkManager (#2539) #2540

wants to merge 1 commit into from

Conversation

@PitouGames
Copy link
Contributor

@PitouGames PitouGames commented Apr 30, 2023 •
edited
Loading

Resolve #2539

This PR add a check that m_NetworkBehaviour is not null before calling the CanClientWrite function.

I did it exactly the same way it's already done for NetworkVariable.

With this PR, operations on the list are done and throws the anyoning warning ( NetworkVariable<T> do the same)

image

Changelog

Testing and Documentation

  • No tests have been added.
  • No documentation changes or additions are necessary.

@PitouGames PitouGames requested a review from a team as a code owner April 30, 2023 21:41
@PitouGames PitouGames force-pushed the fix/offline-network-variable branch from 6081b6e to b1feb94 Compare May 2, 2023 20:57
@0xFA11 0xFA11 requested a review from jeffreyrainy May 2, 2023 21:10
@michalChrobot michalChrobot added stat:awaiting-triage Status - Awaiting triage from the Netcode team. and removed stat:awaiting-triage Status - Awaiting triage from the Netcode team. labels Jan 16, 2025
@PitouGames PitouGames force-pushed the fix/offline-network-variable branch from 8dc63f2 to 844acdb Compare April 19, 2025 09:25
EmandM
EmandM reviewed May 26, 2025
View reviewed changes
Copy link
Collaborator

@EmandM EmandM left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution and your patience on this review.

To be able to land this PR we will also need a forward-port PR for the develop-2.0.0 branch as we cannot have bug fixes in older versions that are not also in newer versions.

@PitouGames PitouGames force-pushed the fix/offline-network-variable branch from 844acdb to fd18093 Compare May 27, 2025 23:03
@EmandM
Copy link
Collaborator

EmandM commented Jun 2, 2025

Thanks for the update! Just to check that we're not landing an incomplete fix, have you used this branch with your project? Does it resolve the issue you're needing resolved?

@PitouGames PitouGames force-pushed the fix/offline-network-variable branch from fd18093 to 5e06bf1 Compare June 6, 2025 17:06
@PitouGames
Copy link
Contributor Author

PitouGames commented Jun 6, 2025

Yes, I do use this fix in my project for two years.
Yes, it resolve the issue.

This PR just adds the same check as you already do in the NetworkVariable here:

com.unity.netcode.gameobjects/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariable.cs

Line 148 in 7ed7a52

if (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId))

I also have rebased my branch.

@EmandM EmandM mentioned this pull request Jun 13, 2025
Merged
@EmandM
Copy link
Collaborator

EmandM commented Jun 13, 2025

This change has been imported into #3502 so we can run the change through our validation process. Thanks for your contribution! Feel free to review if I have missed something.

@EmandM EmandM closed this Jun 13, 2025
EmandM added a commit that referenced this pull request Jun 24, 2025
@EmandM @PitouGames
fix: Avoid throwing exception when using NetworkList without a Networ…
283493e 
…kManager (#3502)

<!-- Replace this block with what this PR does and why. Describe what
you'd like reviewers to know, how you applied the engineering
principles, and any interesting tradeoffs made. Delete bullet points
below that don't apply, and update the changelog section as appropriate.
-->

<!-- Add short version of the JIRA ticket to the PR title (e.g. "feat:
new shiny feature [MTT-123]") -->
Continues #2540 
Fixes #2539 


## Changelog

- Added: `LocalClientCannotWrite` function to co-locate the shared
functionality
- Fixed: Ensure that the `NetworkManager` exists before attempting to
access the `LocalClientId`

## Testing and Documentation

- No tests have been added.

<!-- Uncomment and mark items off with a * if this PR deprecates any
API:
### Deprecated API
- [ ] An `[Obsolete]` attribute was added along with a `(RemovedAfter
yyyy-mm-dd)` entry.
- [ ] An [api updater] was added.
- [ ] Deprecation of the API is explained in the CHANGELOG.
- [ ] The users can understand why this API was removed and what they
should use instead.
-->

## Backport

<!-- If this is a backport:
 - Add the following to the PR title: "\[Backport\] ..." .
 - Link to the original PR.
If this needs a backport - state this here
If a backport is not needed please provide the reason why.
If the "Backports" section is not present it will lead to a CI test
failure.
-->

Up-ported in #3503

---------

Co-authored-by: PitouGames <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@EmandM EmandM EmandM left review comments

@NoelStephensUnity NoelStephensUnity NoelStephensUnity left review comments

@jeffreyrainy jeffreyrainy Awaiting requested review from jeffreyrainy

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Can't do operations on NetworkList with no NetworkManager (offline)

4 participants

@PitouGames @EmandM @NoelStephensUnity @michalChrobot