Arch.Extended
                                
                                
                                
                                    Arch.Extended copied to clipboard
                            
                            
                            
                        Unity compatibility adjustments
Hello, this PR is aimed at several different areas to make the features available in Arch.Extended more compatible with Unity and easier for developers to build and use.
I've based this PR on https://github.com/genaray/Arch.Extended/pull/67 so I'd hold off on merging it if there are any concerns/issues with that one.
Publish Profiles
I've created publish profiles for all the non-tests/sample projects with the correct settings such that a developer has a straightforward path to publish these projects and use them in Unity without compiler error issues. The steps are to:
- Publish each project using the provided profile. This will result in all assemblies being published to two folders underneath 
UnityPublish. 
UnityPublishPlugins=> Contains assemblies that may need to be directly referenced.SourceGenerators=> Contains source generator assemblies only. These require a few special steps after being imported into Unity to use properly.
- Import the contents of 
UnityPublishinto a folder in Unity. This will cause compiler errors temporarily due to the source generators. - Select all assemblies underneath 
SourceGeneratorsand make sure the following settings are configured.- Go to Select platforms for plugin and disable Any Platform.
 - Go to Include Platforms and disable Editor and Standalone.
 - Go to Asset Labels and open the Asset Labels sub-menu (this is present in the lower-right corner of the inspector).
 - Assign a custom 
RoslynAnalyzerslabel to all of them. 
 
- This should result in all compiler errors going away and all features of Arch.Extended can be used in Unity.
 
If useful these instructions could be added to the Wiki.
AOT.SourceGenerator
Unity's C# level and capabilities do not currently include [ModuleInitializer] and so this source generator did not work in Unity as a result as well as causing compiler errors. I've modified this to conditionally use the Unity equivalent [RuntimeInitializeOnLoadMethod] instead when this source generator is used in Unity projects.
I also added a small QOL debug change to sort the component types by name; this makes it easier to visually inspect the generated code in the case of any issues.
Systems
This is a helpful feature that almost seems like it could be a default for Arch, but has one issue that currently makes that impossible to use in Unity with MonoBehavior and ScriptableObject types.
Update() is a special method in Unity for anything derived from MonoBehavior or ScriptableObject, even if it includes parameters. Attempting to use overloads of Update() on these types will result in Unity crashing, which prevents developers from being able to implement ISystem<T> on these types.
This PR changes these methods to instead use Execute in place of Update which fixes this issue and updates the sample project as a result. This enabled me as a developer to make a few Unity-centric base classes for systems so that I could create ScriptableObject, MonoBehavior, and even third-party MonoBehavior derived systems and does not result in Unity Editor crashes.
ScriptableObject Examples of Systems
MonoBehavior Examples of Systems
@genaray The only issue with this change from Update to Execute is that it would be a breaking API change and perhaps not necessarily one we'd want to force on everybody for the sake of just Unity developers. I could also instead rework this to conditionally use Execute over Update if something like a Unity constant were added to both System projects, but then this would result in the locally included sample project to fail to compile when a Unity developer locally made those changes. Any thoughts on how you would like to accept changes like these that aim for compatibility with specific engines?
Thanks that looks great! :)
Just one small question, is the .idea folder required? Normally it is not recommended to include IDE specific directorys.
This is great PR, but as a developer using Arch in Unity, there are a few things I don't agree with.
First, the RuntimeInitializeOnLoad attribute used in the AOT Source Generator works by default with RuntimeInitializeLoadType.AfterSceneLoad. Perhaps this should be done earlier (e.g. RuntimeInitializeLoadType.SubsystemRegistration)?
Also, in my opinion ISystem<T> should not be implemented by MonoBehaviour or ScriptableObject, the implementation of System should be delegated to pure C# classes. I also don't think it is appropriate to make such a breaking change as to change the name of the method because of it.
Also, although this is a rare case, it may be better to use UnityEditor.InitializeOnLoadMethodAttribute in consideration of running it on the editor. (This can be implemented with #if UNITY_EDITOR.)
Additionally, there are better solutions such as NugetForUnity regarding how to distribute the dll. (This allows automatic dependency resolution and automatically adds RoslynAnalyzer tag to SourceGenerator.)
This is great PR, but as a developer using Arch in Unity, there are a few things I don't agree with.
First, the RuntimeInitializeOnLoad attribute used in the AOT Source Generator works by default with RuntimeInitializeLoadType.AfterSceneLoad. Perhaps this should be done earlier (e.g. RuntimeInitializeLoadType.SubsystemRegistration)?
This is a good point, this should probably not be using AfterSceneLoad. I'm not sure if SubsystemRegistration would be better though than AfterAssembliesLoaded; if there were any components attempting to be registered from external assemblies I'm not sure those types would be resolved. Do you have any strong information that pushes towards using one over the other?
I've pushed up a fix commit that adds a constructor param for AfterAssembliesLoaded.
Also, although this is a rare case, it may be better to use
UnityEditor.InitializeOnLoadMethodAttributein consideration of running it on the editor. (This can be implemented with#if UNITY_EDITOR.)
I'm not sure what your intent would be on making this method call editor only. Since the AOT protection is for types that il2cpp player builds that might strip out unless explicitly referenced, making this editor-only would not result in that outcome. Is there any additional context you can give for why this would need to be editor-only?
Also, in my opinion ISystem should not be implemented by MonoBehaviour or ScriptableObject, the implementation of System should be delegated to pure C# classes.
That's your opinion, but just because thats how you choose to work doesn't necessarily mean it should prevent or make it impossible for other developers to use Arch differently. There are a few good reasons why systems are easier to work with in a Unity way as ScriptableObjects or MonoBehaviors (usually for me SO's are the way to go, but there are times where MBs are impossible to avoid or very difficult).
Serialization Sometimes it is desirable to provide a developer or designer in Unity the ability to edit fields on a system directly, at edit-time or runtime. By making it possible to build systems as SO's or MB's it's easy to do that.
It also comes with the added advantage that they are very modular when composing and ordering them into a World and this can be done from the Unity editor by anyone (even a designer) rather than only by a programmer in code. I can now build prototype scenes that only use a subset of the overall systems for testing a particular feature where the production scene would reference the ones only needed for the live game.
It's possible to do the serialization part as POCO systems where those systems are serialized directly inline on a Unity SO or MB, but then the grouping/ordering is all done from code only (less ideal).
Third-Party Plugins and/or Implementation Constraints
While I would rather not ever write my systems as MonoBehaviors due to unneeded extra baggage, sometimes it is unavoidable due to third-party plugins or other potential implementation constraints (a system needs access to APIs that are only available to MonoBehaviors).
For example, to create systems using the Shapes library that I use for part of the in-game UX/UI it was best if my systems inherited from their ImmediateModeShapeDrawer type which embeds well in their library and is derived ultimately from MonoBehaviors. Working around this constraint with a POCO class would have been much more complex and onerous.
I also don't think it is appropriate to make such a breaking change as to change the name of the method because of it.
That was also my thought as well (if you read the bottom of the PR, I had been looking for reasonable alternatives to a breaking API change). See the below comment I'd listed there, curious to hear if there are any alternatives that would enable both solutions outside of conditional compilation.
@genaray The only issue with this change from
UpdatetoExecuteis that it would be a breaking API change and perhaps not necessarily one we'd want to force on everybody for the sake of just Unity developers. I could also instead rework this to conditionally useExecuteoverUpdateif something like aUnityconstant were added to both System projects, but then this would result in the locally included sample project to fail to compile when a Unity developer locally made those changes. Any thoughts on how you would like to accept changes like these that aim for compatibility with specific engines?
Additionally, there are better solutions such as NugetForUnity regarding how to distribute the dll. (This allows automatic dependency resolution and automatically adds RoslynAnalyzer tag to SourceGenerator.)
I'm not sure what your point here is; providing an easy path for any developer to be able to publish the runtime assemblies locally for easy inclusion in a project doesn't prevent someone else from using something like NugetForUnity. For a developer like myself that is often modifying or augmenting the source for a project (perhaps even for client projects where the changes might not be PR'ed back to the repo), building the assemblies locally is the only solution to enabling that workflow. Using NugetForUnity or even Unity's package manager would not be viable options in that scenario.
I've seen several times on the Discord where users have asked how to build and use Arch or Arch.Extended projects in Unity and these publish profiles would enable them to easily do that (for the latter at least).
I'm not sure what your intent would be on making this method call editor only. Since the AOT protection is for types that il2cpp player builds that might strip out unless explicitly referenced, making this editor-only would not result in that outcome. Is there any additional context you can give for why this would need to be editor-only?
You're right. I was concerned that it would be registered twice in the ComponentRegistry, but by reading the implementation I realized that it was checked.
It's possible to do the serialization part as POCO systems where those systems are serialized directly inline on a Unity SO or MB, but then the grouping/ordering is all done from code only (less ideal).
In ECS, system should not maintain state in the first place. The state should be linked to the entity as a component, and the fact that system serialization is necessary is itself a code smell. Regarding execution, instead of implementing ISystem with MonoBehaviour, it would be ideal to schedule a pure C# class System on PlayerLoop like Unity ECS.
Also, all system usage should be visible from the code. Although solutions using SO are easy, they become more complex to manage as development progresses. (I have seen projects that failed in the final stage of development as a result of designing with SO as the main focus.)
While I would rather not ever write my systems as MonoBehaviors due to unneeded extra baggage, sometimes it is unavoidable due to third-party plugins or other potential implementation constraints (a system needs access to APIs that are only available to MonoBehaviors).
I don't have Shapes, so I can only guess from information such as documents, but ImmediateModeShapeDrawer is just a wrapper for OnPreRender, and it seems impossible to trigger it at any timing. Is it necessary to forcefully implement ISystem under these constraints? It should be enough to call World.Query() whenever needed.
I'm not sure what your point here is; providing an easy path for any developer to be able to publish the runtime assemblies locally for easy inclusion in a project doesn't prevent someone else from using something like NugetForUnity. For a developer like myself that is often modifying or augmenting the source for a project (perhaps even for client projects where the changes might not be PR'ed back to the repo), building the assemblies locally is the only solution to enabling that workflow. Using NugetForUnity or even Unity's package manager would not be viable options in that scenario.
There was insufficient explanation on this point. What I'm trying to say here is that it's probably more beneficial for most users to use NugetForUnity. Of course, there are times when this PR solution is necessary. However, when describing the installation method on the wiki, it is better to include both.
It's possible to do the serialization part as POCO systems where those systems are serialized directly inline on a Unity SO or MB, but then the grouping/ordering is all done from code only (less ideal).
In ECS, system should not maintain state in the first place. The state should be linked to the entity as a component, and the fact that system serialization is necessary is itself a code smell.
It seems like you're conflating Unity editor serialization (being able to edit fields of an object in the editor) as runtime game state or something like save data serialization (wholly different concepts). In the example screenshot I provided below of one of my selection systems, making it an SO (or even in the example where it's inline as a POCO class) makes it possible for me to provide it what is essentially global game data or even external systems that do not need to live as an ECS-oriented entity or component.
Regarding execution, instead of implementing ISystem with MonoBehaviour, it would be ideal to schedule a pure C# class System on PlayerLoop like Unity ECS.
Also, all system usage should be visible from the code. Although solutions using SO are easy, they become more complex to manage as development progresses. (I have seen projects that failed in the final stage of development as a result of designing with SO as the main focus.)
With all due respect, this seems more like a personal preference or team style than a Unity-wide best practice or requirement for shipping games. We've shipped and maintain several different games/apps with heavy SO usage for context. There are many resources out there about Scriptable Architecture. While I don't agree with your sentiment that everything needs to be in code, this PR isn't trying to prevent you from doing that. It's making it more possible for developers who use a different architectural approach to be able to use Arch more easily.
While I would rather not ever write my systems as MonoBehaviors due to unneeded extra baggage, sometimes it is unavoidable due to third-party plugins or other potential implementation constraints (a system needs access to APIs that are only available to MonoBehaviors).
I don't have Shapes, so I can only guess from information such as documents, but
ImmediateModeShapeDraweris just a wrapper forOnPreRender, and it seems impossible to trigger it at any timing. Is it necessary to forcefully implement ISystem under these constraints? It should be enough to callWorld.Query()whenever needed.
You're kind of missing the point. If a system has requirements or constraints that make it attractive to be implemented as a  MonoBehavior, this PR is aiming to make that possible whereas attempting to do that now results in the editor crashing. I'd like to try to make changes to make this more easily possible for a Unity developer short of needing to maintain a local fork with these or similar changes and am soliciting for preference/options from @genaray and others interested on how best to do that.
I'm not sure what your point here is; providing an easy path for any developer to be able to publish the runtime assemblies locally for easy inclusion in a project doesn't prevent someone else from using something like NugetForUnity. For a developer like myself that is often modifying or augmenting the source for a project (perhaps even for client projects where the changes might not be PR'ed back to the repo), building the assemblies locally is the only solution to enabling that workflow. Using NugetForUnity or even Unity's package manager would not be viable options in that scenario.
There was insufficient explanation on this point. What I'm trying to say here is that it's probably more beneficial for most users to use NugetForUnity. Of course, there are times when this PR solution is necessary. However, when describing the installation method on the wiki, it is better to include both.
That's what I'm proposing; an easier way for a developer using Rider (pretty common on macOS for Unity development) to be able to publish these assemblies locally with the right settings for them to work in Unity that can live alongside other methods, like Nuget or even a Unity Package Manager distro (I don't think there is one for Arch at the moment).
@AnnulusGames I definitely appreciate your interest and feedback (I've starred a few of your repos!), but wish that it was more open-minded and less reductive towards there being only one way to make games. I'm definitely cognizant that some developers might really dislike ScriptableObjects, but this PR isn't attempting to force you to use them ;)
It seems like you're conflating Unity editor serialization (being able to edit fields of an object in the editor) as runtime game state or something like save data serialization (wholly different concepts). In the example screenshot I provided below of one of my selection systems, making it an SO (or even in the example where it's inline as a POCO class) makes it possible for me to provide it what is essentially global game data or even external systems that do not need to live as an ECS-oriented entity or component.
Ideally, in the context of ECS, all of these should be accessed via components. For example, in Unity ECS, you construct a BlobAsset in the authoring component and pass the BlobAssetReference to the component. However, I am skeptical that the benefits of this restriction are worth the effort, and it would be easier to use SerializeField directly. (Although this is sufficient for practical purposes, please note that strictly speaking, this is against the philosophy of ECS.)
(For those reading this, I would like to add one thing: Serialization of the editor and serialization of save data are, in concept, exactly the same. The only difference is whether it is done by the user or by Unity in the editor. )
With all due respect, this seems more like a personal preference or team style than a Unity-wide best practice or requirement for shipping games. We've shipped and maintain several different games/apps with heavy SO usage for context. There are many resources out there about Scriptable Architecture. While I don't agree with your sentiment that everything needs to be in code, this PR isn't trying to prevent you from doing that. It's making it more possible for developers who use a different architectural approach to be able to use Arch more easily.
The concept of ScriptableArchitecture is now considered a bad practice by many developers. (The argument here is a bit extreme, but I generally agree with it.) As an alternative, my advice based on experience is to use Rx for messaging and a DI container for dependency resolution. However, if it can be maintained, it may be sufficient.
In any case, as you say, it may be in the developer's interest to be able to take multiple approaches. If you want to change the method name, I recommend OnUpdate rather than Execute. (This also matches Unity ECS System.)
Ideally, in the context of ECS, all of these should be accessed via components. For example, in Unity ECS, you construct a BlobAsset in the authoring component and pass the BlobAssetReference to the component.
You kind of have to do it this way with Unity's first-party ECS framework; there aren't other ways provided. Their framework is perhaps not the best to use as an example seeing how it's now considered production-ready, but doesn't have support for animation and other essential features without using third-party paid or open-source plugins.
Just to take this sound example further though, in the event I did want to have specific selection sounds per object I likely would use some kind of flyweight-like pattern where an index or enum value on a component would be used by a system to retrieve the appropriate AudioClip from an external factory. Right now at this early stage of development that's not important.
However, I am skeptical that the benefits of this restriction are worth the effort, and it would be easier to use SerializeField directly. (Although this is sufficient for practical purposes, please note that strictly speaking, this is against the philosophy of ECS.)
You're exactly right here though in that sometimes it's not beneficial for practical reasons to deconstruct every piece of data into components. It's better to not lock yourself into needing to implement everything into an ECS.
Where people get tripped up on ECS as a design pattern is thinking that using it means everything now has to be implemented in that paradigm, even features that don't benefit from it. This is kind of silly when you think about it; it limits the ability to integrate any sort of external functionality outside of the ECS in the game if it can't be boiled down to entities, components, or systems (should localization be implemented as an ECS system for example or is it better served as a non-ECS API?).
ECS is best-used as it benefits a project to be integrated into it and nothing more. It's really great for the simulation and core-game logic portion of my game, but there are other gameplay systems which maybe only have a tiny representation (like Cinemachine for camera movement/behavior).
**(For those reading this, I would like to add one thing: Serialization of the editor and serialization of save data are, in concept, exactly the same. The only difference is whether it is done by the user or by Unity in the editor. )
They use similar technologies, but thats about it. Making fields on a Unity object serializable enables developers to edit them in the Editor for development purposes. This isn't seen outside of the editor by users much in the way that private fields aren't seen outside of the class by consumers of an object's API.
The concept of ScriptableArchitecture is now considered a bad practice by many developers. (The argument here is a bit extreme, but I generally agree with it.) As an alternative, my advice based on experience is to use Rx for messaging and a DI container for dependency resolution. However, if it can be maintained, it may be sufficient.
Again, this seems like a very black and white, binary hot-take on a technology when we live in a world of gray. I feel like I've said my piece on different development practices and this PR isn't forcing an approach on others as much as it is giving reasonable options.
I definitely agree with shades of the arguments that developer states (we use a lot of plain C# events alongside scriptable events for example), but I know other people who feel like a lot of DI containers and RX libraries are also unnecessarily complex. It's a great big melting pot of opinions out there!
In any case, as you say, it may be in the developer's interest to be able to take multiple approaches. If you want to change the method name, I recommend
OnUpdaterather thanExecute. (This also matches Unity ECS System.)
Alternate method names like OnUpdate is a reasonable solution to achieving the same outcome as Execute though it also is a breaking API change. If we were to follow this tract I'd be changing the system/group method names like so:
BeforeUpdate=>OnBeforeUpdateUpdate=>OnUpdateAfterUpdate=>OnAfterUpdate
@genaray is this a reasonable change to you?
In any case, we probably shouldn't discuss design any further here. Again, I would like to emphasize my concern that changing method names is a breaking change and many projects and libraries will be affected accordingly. (In my personal opinion, it's unlikely that it would be profitable enough to make the change, but I'll leave it up to the collaborators to decide whether to merge or not.)
What a discussion, it took me a long time to read through all this but I admire your dedication ^^ The only thing that logically worries me are the breaking changes... because arch is now used by many people and that could make some things incompatible. Especially the source generators have to be adapted.
Does Unity really crash if you add more update methods with a different signature? I can hardly imagine it, but I have never tested it. Would it be an alternative to replace the method names with flags? That would only be a breaking change for Unity users who use the source.
Would it be an alternative to replace the method names with flags? That would only be a breaking change for Unity users who use the source.
I don't really agree with the idea of changing method names with flags. The method name mismatch is confusing and if it is going to be changed, it should be changed in its entirety.
What a discussion, it took me a long time to read through all this but I admire your dedication ^^ The only thing that logically worries me are the breaking changes... because arch is now used by many people and that could make some things incompatible. Especially the source generators have to be adapted.
I wouldn't be afraid of breaking changes so much as making sure whatever the new method name is makes sense and doesn't change again for a similar reason. It took me about half an hour to go through and make this example change originally for both System projects, will not take long to adapt to a different name.
Does Unity really crash if you add more update methods with a different signature? I can hardly imagine it, but I have never tested it. Would it be an alternative to replace the method names with flags? That would only be a breaking change for Unity users who use the source.
There are pros and cons. Like @AnnulusGames mentioned it does make for an inconsistency between method names which can be confusing to document in API versus a one-time method name change that applies to everyone. It would also mean the Arch.Extended.Sample wouldn't compile when a user had set the flag to build the system projects for Unity, aka the solution would fail to build while that flag was set (all projects other than the sample would succeed in building).
How sure are we about that crash stuff? Im pretty sure that a monobehaviour can have a e.g. Update(MyCustomClass) method besides the default update method.
How sure are we about that crash stuff? Im pretty sure that a monobehaviour can have a e.g.
Update(MyCustomClass)method besides the default update method.
This is an example MB with an overload of Update.
 using System.Collections;
using System.Collections.Generic;
using UnityEngine;
public class ExampleBadMonoBehaviour : MonoBehaviour
{
	public void Update(float timeDelta)
	{
	}
}
Rider itself will warn that the method overload is wrong for Unity.
On Unity 2022.3.16f1 Unity will show script compiling errors.
Script error (ExampleBadMonoBehaviour): Update() can not take parameters.
In the test project I just ran this in it didn't cause a crash, but in the one I was using Arch in changing these to a non-Unity protected method for the Arch systems I was creating did.
Either way, this just points to it not being an ideal method name choice with Unity in mind.