Skip to content

Conversation

@NoelStephensUnity
Copy link
Collaborator

@NoelStephensUnity NoelStephensUnity commented Aug 10, 2024

Backport of #3004
When using collections with NetworkVariable, it would only detect a completely new assignment of a collection but would not detect a change to an already assigned collection (add, remove, insert, clear) nor was there a convenient way to check if a collection or the values of its items/elements were dirty.
(Requires a backport to NGO v1.x.x)

MTTB-305
MTTB-308

fix: #2993

Changelog

  • Added NetworkVariable.CheckDirtyState that is to be used in tandem with collections in order to detect whether the collection or an item within the collection has changed.
  • Fixed issue using collections within NetworkVariable where the collection would not detect changes to items or nested items.
  • Fixed issue where List, Dictionary, and HashSet collections would not uniquely duplicate nested collections.
  • Changed permissions exception thrown in NetworkList to exiting early with a logged error that is now a unified permissions message within NetworkVariableBase.
  • Changed permissions exception thrown in NetworkVariable.Value to exiting early with a logged error that is now a unified permissions message within NetworkVariableBase.

Testing and Documentation

  • Includes integration tests in NetworkVariableCollectionsTests
  • Requires documentation updates.

Fixing some of the collections issues.
Adding a unified permissions error.
Replacing throw exceptions with logging errors and returning.
adjustments for replacing throw exception and added internal unified permissions write error.
Adjustments made due to other issues with direct assignment.
back port typo.
removing using directive.
Mark that we have a previous value for disposing when destroyed.
This fixes an issue with duplicating nested lists, hashsets, and dictionaries.
This is the first set of tests for Lists, including nested, that test using base types and INetworkSerializable.
Still have HashSet and Dictionary to cover.
@NoelStephensUnity NoelStephensUnity changed the title fix: managed collections not detecting deltas backport to v1.x.x fix: collections not detecting deltas backport to v1.x.x Aug 12, 2024
Added Dictionary and HashSet to the tests.
Removing whitespaces
adding changelog entries
@NoelStephensUnity NoelStephensUnity marked this pull request as ready for review August 12, 2024 22:26
@NoelStephensUnity NoelStephensUnity requested a review from a team as a code owner August 12, 2024 22:26
// check write permissions
if (!CanClientWrite(m_NetworkBehaviour.NetworkManager.LocalClientId))
{
throw new InvalidOperationException("Client is not allowed to write to this NetworkList");
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious how come all of these exceptions have been removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Been making it a point to swap out throwing exceptions with just logging errors when I run across areas where an exception is a bit harsh for the issue (if it wouldn't normally cause some form of exception then there is no point in interrupting the call stack for some form of improper usage or the like). Also wanted the permissions error to be unified for both NetworkVariable and NetworkList. 👍

Adding missed change log entries.

- Fixed issue using collections within `NetworkVariable` where the collection would not detect changes to items or nested items. (#3005)
- Fixed issue where `List`, `Dictionary`, and `HashSet` collections would not uniquely duplicate nested collections. (#3005)
- Fixed Issue where a state with dual triggers, inbound and outbound, could cause a false layer to layer state transition message to be sent to non-authority `NetworkAnimator` instances and cause a warning message to be logged. (#2999)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These were missed changelog entries: 2999 and 2992. They are not in this PR.

@NoelStephensUnity NoelStephensUnity enabled auto-merge (squash) August 13, 2024 17:19
@NoelStephensUnity NoelStephensUnity merged commit 231b36d into develop Aug 13, 2024
@NoelStephensUnity NoelStephensUnity deleted the fix/managed-collections-not-detecting-deltas-backport-v1 branch August 13, 2024 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NetworkVariable<Dictionary> not syncing or firing OnValueChanged events

3 participants