Skip to content
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.

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

Feature: v3 namespaces and organization #129

Closed
jzapdot opened this issue Dec 7, 2023 · 4 comments
Closed

Feature: v3 namespaces and organization #129

jzapdot opened this issue Dec 7, 2023 · 4 comments
Milestone

Comments

@jzapdot
Copy link

jzapdot commented Dec 7, 2023

I had taken a slew of notes for the previous v1 of GOAP and after looking through the code for v3 there would definitely be some benefits from simplifying the project namespaces and assembly definitions.

Is your feature request related to a problem? Please describe.

When implementing my example use-case using GOAP one area of friction I had was that there seemed to be a lot of nested namespaces, sometimes up to 4-5 deep. If attempting to use these classes outside of that specific namespace it requires using a using statement for each , leading to many of my files needing to import in 4-5 several using statements for the same library.

Taking a closer look at the library it seems like the primary culprit is that the namespaces directly match the folder structure. This kind of namespace structure can easily lead to breaking API changes if files/objects move around to different locations in the folder hierarchy which can add friction for users when updating versions.

Describe the solution you'd like

One approach we've practiced and that I've recommended to others is to limit the number of namespaces to be minimal, represent only the information that users would need, and generally align to the assembly/assembly definition they are contained in. For a typical Unity library (using GOAP as an example), I might create namespaces/assembly definitions like:

  • CrashKonijn.Goap or CrashKonijn.Goap.Runtime => Contains all runtime functionality for the library.
  • CrashKonijn.Goap.Editor => Contains all editor functionality for the library.
  • CrashKonijn.Goap.Editor.Tests => Contains all edit-mode unit test functionality for the library.
  • CrashKonijn.Goap.PlayModeTests => Contains all play-mode unit tests for the library.

The benefits of this approach is that:
* A developer immediately knows what assemblies they need to reference for runtime vs editor code without needing to know the inner working semantics (do I need to reference goap.core vs goap vs goap.resolver for example).
* As the files in an assembly all share the same namespace, if a type moves to a different folder location in an assembly this does not result in a breaking API change as the fully qualified type name has not changed.
* Less namespaces are needed to be referenced in a file for using the library in general. For example...

Instead of needing to specify something like this for runtime behavior (from one of the v1 sensors I had written):

using CrashKonijn.Goap.Behaviours;
using CrashKonijn.Goap.Classes;
using CrashKonijn.Goap.Enums;
using CrashKonijn.Goap.Interfaces;

you would instead need to use this:

using CrashKonijn.Goap;

Many IDEs offer the ability to remove a folder as being a namespace provider by right-clicking on a folder, selecting properties, and unchecking Namespace Provider.

image

Any new files created in that folder or subfolders will exclude that folder from contributing to the namespace and refactor actions in the IDE that adjust namespaces will also take it into account.

Additional context
Since there are already a lot of breaking API changes in v3 this would be the perfect time to consider doing this kind of simplification as there is already that expectation that the API/namespaces are breaking from v1.

@crashkonijn
Copy link
Owner

Hi there!

Awesome idea! I would never do this for something I'd use myself, but for a project like this that might be the right call!

Do you think it would be fine to differentiate between things you're meant to use (ie Interfaces, Enums, base classes), and internal classes you're not supposed to reference?

@crashkonijn crashkonijn added this to the v3 milestone Dec 7, 2023
@jzapdot
Copy link
Author

jzapdot commented Dec 7, 2023

Hi there!

Awesome idea! I would never do this for something I'd use myself, but for a project like this that might be the right call!

It definitely helps for libraries used by external users or teams for sure and helps to abstract away needing to think too hard about how types are organized since they are usually in the same namespace.

Do you think it would be fine to differentiate between things you're meant to use (ie Interfaces, Enums, base classes), and internal classes you're not supposed to reference?

"How to differentiate things that are meant for use by external consumers of the library vs internal use?"

As far as file/folder organization I use this less as a mechanism for differentiating. I usually organize files by function/relation and rely on the access modifier to distinguish whats for public consumption vs internal use. I would generally effect this by marking types as being internal (public inside the assembly, but private without) or private if they were meant for internal use only.

Sometimes there might be a need to expose a type from one assembly to another like public (like exposing an internal type from a runtime assembly to an editor or unit test assembly), but want to have it be private for users. In this case using the [assembly attribute with the InternalsVisibleTo field can make that possible. I'd usually make an AssemblyInfo.cs file at the root of the assembly (for organization) and add the attribute like so.

AssemblyInfo.cs in CrashKonijn.Goap assembly

[assembly: InternalsVisibleTo("CrashKonijn.Goap.Editor")]

@crashkonijn
Copy link
Owner

I was talking more about the fact that everything (mostly) has interfaces for it, in general when working with the system you don't generally use concrete implementations.

Since it's unity, there are case where you'd like to reference any of the XBehaviour classes, but most of them contain a class within them that actually do the work. For example the GoapRunnerBehaviour which internally uses the GoapRunner.

In theory you would 'never' need to use the GoapRunner. I could indeed make some of it internal instead of public, but I really don't like hiding things if I don't have to.

I was thinking about separating between 'you NEED these interfaces and classes' and as an advanced user you 'might need these to implement your own'.

Anyway, as someone that works as a .NET web developer I like to separate things into separate assemblies with contract interfaces between them to keep things interchangeable. Back when I started building V2 (the current version) you could see different assembly definitions between layers that where general C# and then unity on top. The Goap.Resolver assembly is still something leftover from that.

Anyway, I'll do this at some point for V3. Right now I've still got a branch where I'm halfway through some large changes :)

Thanks for you suggestion!

crashkonijn added a commit that referenced this issue Jun 28, 2024
@crashkonijn
Copy link
Owner

Fixed for v3! 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants