Entitas-csharp: [CodeGenerator:] Generate component interfaces

[sschmid]

The problem

You are allowed to write this

entity.health.value = 42;

but you shouldn't. You always should replace components like this

entity.ReplaceHealth(42);

The solution

This

entity.health.value = 42;

should result in a compile error.

The Plan

Declare your components with the partial keyword and getters and setters

public partial class HealthComponent : IComponent {
    public int value { get; set; }
}

The code generator should generate

// Getter only
public interface IHealthComponent : IComponent {
    int value { get; }
}

// Make component sealed and implement interface
public sealed partial class HealthComponent : IHealthComponent {
}

All the generated API of other generators will use the interface instead of the component which means this

entity.health.value = 42;

will result in a compile error 👍

Make Entitas safe again!

[bddckr]

Some thoughts I had which resulted in me not suggesting this. I'll post in in case someone else comes up with that, too:

Q: Why not generate int value { get; set; } and in the setter do entity.ReplaceHealth(value)?
A: I can see that being stupid when you have multiple fields in your component and leading to confusion because it will eat your performance probably.

Q: Maybe only generate that for components with a single field?
A: Sure that works, but is a pain when adding more fields later and one API is simpler than two 😄

[IDNoise]

I'm not sure this is good feature. In some cases you don't need component to be trigger in reactive system and want to save performance on every value change. F.e. you have global component with counters that is affected by multiple systems almost every frame, but no one need to react on those changes.

As an option it's possible to add new component attribute like [ReadOnly] that will generate interface with getters only.

[sschmid]

@IDNoise I know what you mean, but not replacing the component circumvents all concepts of Entitas (Groups, ReactiveSystems, EntityIndex, VisualDebugging, etc will not work) Components should be seen as immutable data. The only reason why they are technically mutable in Entitas-CSharp is the garbage collector and performance optimizations. The whole api is designed to treat components as immutable data. I consider the fact that we were actually able to modify components as an exploit that I want to patch.
Entities and components are all object pooled and reused, so changing them multiple times each frame shouldn't be a problem

[sschmid]

In rare situations where performance is really critical, and none of the features mentioned above are needed, you can still modify the actual component. But as a default I think it's a safer way to enforce replacing components

[danielbe1]

The main motivations behind data immutability are to avoid side-effects which in turn give us thread-safety and data sharing and some more nice things. Replacing and setting are both side effects so personally, I don't think this has any real benefit.

[sschmid]

Replacing has a real benefit as it makes the whole state in the context observable and thus enables Groups, ReactiveSystems, EntityIndex, VisualDebugging, etc.

[danielbe1]

Yes, but that's assuming that we always want to cause those side effects, which might not be the case. I mean, not reacting to change is simply a design choice of the ECS, its just as valid as reacting to changes.

[sschmid]

Sure, there might be a case where you prefer not to have those effects. Personally, I never encountered such a case in my last 3 games with Entitas (which doesn't mean anything really, tbh :)). With Entitas I'd like to promote general best practises and insist on consistency as I'm convinced consistency is generally a big plus. As mentioned above, there will still remain a way to modify components directly, but it won't be as obvious anymore in order to prevent people from doing common mistakes.

[sschmid]

The goal is not to remove flexibility but to make it harder to write code with bugs

[trumpets]

I must add to this "accidental and hard to spot bugs" :) this is a good feature IMHO

[FNGgames]

I think this is a good idea - i think you should be enforcing these rules with your API. My main concern is that this will generate an inconsistency between the way you set flag components and the way you set other components. I think that if you implement your suggestion you should also change the API for the flag components to use methods instead of direct assignments too. So 'e.IsFlag = true;' should become e.SetFlag(true) or something. Does this make sense?

[danielbe1]

Well, technically there isn't an inconsistency since for flag components, setting is similar to replacing but yeah, this is a style inconsistency.

I think generating something like Scala's copy would be the best of both worlds, the ease of use of direct assignment together with consistent state transformations.

[sschmid]

just had another real-life issue from a user that called e.position.x = 42;
I think that feature really helps reducing bugs

[Bradamante3D]

Enforcing a Replace makes sense. However, are there other conceivable ways to do so, besides a compile error?

[sschmid]

After some thought, I will close. While I think it's nice to enforce, there are performance penalties that hurt everyone. Getters are just way slower than fields. In rare cases you want to actually circumvent all event for extreme performance.

[FNGgames]

how much slower Simon? Do you have test results (I know you do!)?

[sschmid]

Of course I have :D Everything I do is based on performance results. I even came up with a name
PDD - Profiler-Driven-Development ™

I try to recall, it was sth like 50-70ms vs 13ms