chore!: per-asmdef namespaces instead of per-folder#1009
Conversation
0xFA11
requested review from
NoelStephensUnity,
Rosme,
ShadauxCat,
becksebenius-unity,
jeffreyrainy and
mattwalsh-unity
| using Unity.Multiplayer.Netcode.Logging; | ||
| using Unity.Multiplayer.Netcode.Configuration; | ||
| using Unity.Multiplayer.Netcode.Profiling; | ||
| using Unity.Multiplayer.Netcode.Serialization; | ||
| using Unity.Multiplayer.Netcode.Transports; | ||
| using Unity.Multiplayer.Netcode.Connection; | ||
| using Unity.Multiplayer.Netcode.Messaging; | ||
| using Unity.Multiplayer.Netcode.SceneManagement; | ||
| using Unity.Multiplayer.Netcode.Spawning; | ||
| using Unity.Multiplayer.Netcode.Exceptions; | ||
| using Unity.Multiplayer.Netcode.Serialization.Pooled; | ||
| using Unity.Multiplayer.Netcode.Transports.Tasks; | ||
| using Unity.Multiplayer.Netcode.Timing; |
com.unity.multiplayer.mlapi/Runtime/NetworkVariable/Collections/NetworkDictionary.cs
Show resolved
Hide resolved
| using System.Collections.Generic; | ||
|
|
||
| namespace Unity.Multiplayer.Netcode.Profiling | ||
| namespace Unity.Multiplayer.Netcode |
There was a problem hiding this comment.
we might still want to keep profiling-related types behind a namespace like Unity.Multiplayer.Netcode.Profiling, what would you guys think? @becksebenius-unity @Rosme
also, should any of these types be public at all? I though they all could be internal types, no?
There was a problem hiding this comment.
okay, I see that NetworkManager implements IProfilableTransportProvider interface which has a property returning ITransportProfilerData type. I guess, if we were to keep these two public and make everything internal, that'd work fine and we'd still keep them under Unity.Multiplayer.Netcode namespace — how that sounds to you guys? 👀
There was a problem hiding this comment.
ITransportProfilerData will ultimately become deprecated, it's not something that we're using for Profiler V2. In general I'd say we should make as much of this internal as possible, although this was public because it needs to be used in other assemblies
ShadauxCat
left a comment
ShadauxCat
left a comment
There was a problem hiding this comment.
1,000% on board with this.
No... 10,000%.
| using System; | ||
| using System.Collections.Generic; | ||
| using System.IO; | ||
| using Unity.Multiplayer.Netcode.Messaging; |
There was a problem hiding this comment.
glad to see there's nothing further than that in this file! :-)
…nsform * develop: (32 commits) refactor: calling networkShow(NetworkObject) code in networkshow(List<NetworkObject>) (#1028) feat: snapshot. MTT-685 MTT-822 (#1021) test: adding a multi-instance test checking NetworkShow and NetworkHide on lists of objects (#1036) fix: corrected NetworkVariable WriteField/WriteDelta/ReadField/ReadDelta dropping the last byte if unaligned. (#1008) chore: run standards check over solution files (#1027) chore: replace MLAPI with Netcode in Markdown files (#1025) fix!: added plainly-callable Add() method to NetworkSet [MTT-1005] (#1022) fix: fixing incorrect merge done as part of commit 85842ee (#1023) chore: cleanup/upgrade serialized scenes (#1020) chore: replace MLAPI with Netcode in C# source files (#1019) test: add network collections, struct and class tests MTT-936 (#1000) test: add buildtests to test build pipeline on target platforms (#1018) chore: rename MLAPI types to Netcode (#1017) chore!: rename asmdefs, change top-level namespaces (#1015) Replacing community NetworkManagerHUD with a simpler implementation (#993) test: network prefab pools and INetworkPrefabInstanceHandler (#1004) fix: do not expose Runtime internals to TestProject.ManualTests asmdef (#1014) refactor: snapshot. merge preparation. Removing old acks, removing unused varia… (#1013) chore!: per-asmdef namespaces instead of per-folder (#1009) feat: snapshot. ground work, preparing depedencies. No impact on code behaviour (#1012) ... # Conflicts: # com.unity.multiplayer.mlapi/Prototyping/NetworkTransform.cs # com.unity.multiplayer.mlapi/Runtime/Messaging/InternalMessageHandler.cs
6 participants
it's a common practice in Unity packages to use the same top-level namespace for types including sub-folders (e.g. Unity Transport, DOTS Netcode etc.).
sticking with the same namespace would make discovery much better.
for instance, you don't have to remember
using Unity.Multiplayer.Netcode.NetworkVariabledirective to define yourNetworkVariable<int> m_Health;anymore, yay!sticking with top-level namespace would also get rid of long list of using statements subsetting folders.
(take a look at PR diffs for examples)
this PR proposes following top-level namespaces (hence
rootNamespacefields in asmdefs):nspaceUnity.Multiplayer.NetcodeasmdefUnity.Multiplayer.MLAPI.RuntimenspaceUnity.Multiplayer.Netcode.PrototypingasmdefUnity.Multiplayer.MLAPI.PrototypingnspaceUnity.Multiplayer.Netcode.EditorasmdefUnity.Multiplayer.MLAPI.EditornspaceUnity.Multiplayer.Netcode.RuntimeTestsasmdefUnity.Multiplayer.MLAPI.RuntimeTestsnspaceUnity.Multiplayer.Netcode.EditorTestsasmdefUnity.Multiplayer.MLAPI.EditorTestsnspaceTestProject.RuntimeTestsasmdefTestProject.RuntimeTestsnspaceTestProject.ManualTestsasmdefTestProject.ManualTestsI think we should be deliberate when hiding types behind namespaces, such as
Unity.Multiplayer.Netcode.Transports.UNET(which is the only subset namespace under Runtime asmdef atm).Hopefully, this'd make UX & optics much better and frictionless.
(note: you could also expect asmdef name changes coming soon!)