Closed sschmid closed 1 year ago
Personally, I would rename ReplaceComponent()
to SetComponent()
. To me, replace implied I should not use it if the component is not already added. Set doesn't have that connotation.
I don't feel like there is a huge downside to keeping AddComponent()
around.
It can help uncover some issues that would otherwise be hard to diagnose.
While being part of #1046, I feel similarly about generating additional TryGetComponent(out component)
methods.
I don't see a huge downside to having them - and situationally it might be the nicest solution.
Of course, there is some bloat that can slow down the compiler a bit. But it may also be plausible to have some of these things be generated based on settings? I haven't tested this yet, especially not in Unity. They haven't upgraded their build system and switched to user-defined MSBuild projects yet, but afaik that's still part of their plan for the future. But for now, some things (like AdditionalFiles) seem to be broken, so I'm not sure if this part is viable.
@rubenwe Right, using settings sounds good. I tried adding a test value using .editorconfig
, but it doesn't seem to work. https://github.com/sschmid/Entitas/commit/fb7f45fe41a68864ccd57200b1e08e7ee1c75785
I updated the TestHelper.cs to feed the unit tests with a test value, and that worked well and the tests successfully reads the value. But generated code in the integration test project doesn't seem to read the .editorconfig
from the root folder.
Any ideas why?
Ok, I found a solution, but not sure if I'm happy.
Instead of converting the global options to a custom options struct like this:
var options = initContext.AnalyzerConfigOptionsProvider
.Select((provider, _) => new EntitasAnalyzerConfigOptions(provider.GlobalOptions));
and later trying to retrieve the value from it (which did not work), I'm now using a slightly different approach, by using the AnalyzerConfigOptionsProvider without Select()
.
var options = initContext.AnalyzerConfigOptionsProvider;
and later before generating the code, I retrieve options like this by providing the component SyntaxTree
, which I had to add to the ComponentDeclaration
.
var entitasOptions = new EntitasAnalyzerConfigOptions(options.GetOptions(component.Syntax!.SyntaxTree));
Why do I need to provide the SyntaxTree?!? This should not affect global options, or does it? I'm slightly confused.
see commit: https://github.com/sschmid/Entitas/commit/9f06a26208bac5057d5e55e3a502879b250f51c9
Hey @sschmid !
Since you're rewriting this, I have two points that I think you might be interested in. I use this in my project and it makes my life much easier.
I rewrote the methods so that they return an entity so that I can call the next method as a chain. Here's how I use it:
entity
.SetPlayer(true)
.SetMovable(true)
.AddView(backing.gameObject)
.AddPosition(backing.transform.position)
.AddMoveComplete(true)
.AddPositionListener(backing);
This reduces the number of code and helps to group it better.
I use LanguageExt library in my project and I also added it to my Entitas generator.
It has the type Option<T>
, and it is an alternative to nullable types. But this forces devs to handle None (null) values, and also has many useful methods, as in LINQ.
For example this component, will generate two new properties for me: maybePosition
and maybePosition_value
public class PositionComponent : IComponent
{
public Vector2 value;
}
public Option<PositionComponent> maybePosition;
public Option<Vector2> maybePosition_value;
Thanks to this, I can safely use position later in my code
entity.maybePosition_value.IfSome(position =>
{
// Its guaranteed here that this entity have position
});
Or we can do some calculations, as in this fictional example
entity.maybePosition_value
.Map(position => position.normalized)
.Filter(normal => normal.x > 0)
.IfSome(positiveNormals =>
{
// ...
}
);
Or like here:
var hasFinished = entity.maybePosition_value.Match(
Some: position => position.x > 10,
None: () => false
);
And a lot more :)
I will be happy to help if you find any of this useful for Entitas.
I will resist and try not to change too much :D I will keep the familiar api with Add / Replace / Remove for now. I would also research if I can overwrite default analyzer behaviour for nullable checks before I would attempt to migrate to nullable. E.g. in reactive system + Filter() we're guaranteed that entities have components, but the compiler doesn't know that and warns about possible null.
@OctopBP Yes! I'll definitely do the fluent api!
Thanks for your feedback! Will close
Overview
vs
The code generator currently produces methods like this:
As we know in Entitas exceptions are thrown when
You will get nice exception description like
While working on the new code generator using C# source generators #1005, I started using C# nullables and I was wondering if we should get rid of the exceptions and move the responsibility to the dev.
This would mean adding components even if they already exists will result in overwriting them, basically exactly what
ReplaceComponent()
does right now. Attempting to remove a component that doesn't exist would also NOT fail.To communicate this fact, we can rename the methods to
Set
andUnset
, see examples in unit tests: https://github.com/sschmid/Entitas/blob/set-unset/tests/Entitas.Generators.IntegrationTests/ComponentTests.cs#L18-L55Entitas won't throw exception and communicates that using nullables.
Example:
Keep in mind that removing exceptions also mean no more
Cannot add component 'Health' to Entity_123!
What do you think?